Re: [PATCH 1/3] database: Add notmuch_database_compact_close

Subject:Re: [PATCH 1/3] database: Add notmuch_database_compact_close

Date:Sat, 02 Nov 2013 11:30:43 -0700

To:Ben Gamari ,notmuch@notmuchmail.org

Cc:

From:Jameson Graef Rollins


On Wed, Oct 02 2013, Ben Gamari <bgamari.foss@gmail.com> wrote:
> +/* Compacts the given database, optionally saving the original database
> + * in backup_path. Additionally, a callback function can be provided to
> + * give the user feedback on the progress of the (likely long-lived)
> + * compaction process.
> + *
> + * The backup path must point to a directory on the same volume as the
> + * original database. Passing a NULL backup_path will result in the
> + * uncompacted database being deleted after compaction has finished.
> + * Note that the database write lock will be held during the
> + * compaction process to protect data integrity.
> + */
> +notmuch_status_t
> +notmuch_database_compact (const char* path,
> +			  const char* backup_path,
> +			  notmuch_compact_status_cb_t status_cb)
> +{
> +    void *local = talloc_new (NULL);
> +    NotmuchCompactor compactor(status_cb);
> +    char *notmuch_path, *xapian_path, *compact_xapian_path;
> +    char *old_xapian_path = NULL;
> +    notmuch_status_t ret = NOTMUCH_STATUS_SUCCESS;
> +    notmuch_database_t *notmuch = NULL;
> +    struct stat statbuf;
> +
> +    ret = notmuch_database_open(path, NOTMUCH_DATABASE_MODE_READ_WRITE, &notmuch);
> +    if (ret) {
> +	goto DONE;
> +    }
> +
> +    if (! (notmuch_path = talloc_asprintf (local, "%s/%s", path, ".notmuch"))) {
> +	ret = NOTMUCH_STATUS_OUT_OF_MEMORY;
> +	goto DONE;
> +    }
> +
> +    if (! (xapian_path = talloc_asprintf (local, "%s/%s", notmuch_path, "xapian"))) {
> +	ret = NOTMUCH_STATUS_OUT_OF_MEMORY;
> +	goto DONE;
> +    }
> +
> +    if (! (compact_xapian_path = talloc_asprintf (local, "%s.compact", xapian_path))) {
> +	ret = NOTMUCH_STATUS_OUT_OF_MEMORY;
> +	goto DONE;
> +    }
> +
> +    if (backup_path != NULL) {
> +	if (! (old_xapian_path = talloc_asprintf (local, "%s/xapian.old", backup_path))) {
> +	    ret = NOTMUCH_STATUS_OUT_OF_MEMORY;
> +	    goto DONE;
> +	}

Hey, folks.  I'm obviously late for this, but I just got around to
testing the new compact functionality now and I wanted to comment on the
path for the old xapian directory.  It seems to me that

  <notmuch_path>/xapian.old

isn't the right place for it.  I would think that

  <xapian_path>.old

would be a much better place.  I'm not such a fan of dumping internal
notmuch stuff into the main mail directory.  Keeping all notmuch stuff
in <notmuch_path> seems more reasonable and polite.

jamie.
part-000.sig (application/pgp-signature)

Thread: