--- /dev/null
+Return-Path: <tomi.ollila@iki.fi>\r
+X-Original-To: notmuch@notmuchmail.org\r
+Delivered-To: notmuch@notmuchmail.org\r
+Received: from localhost (localhost [127.0.0.1])\r
+ by olra.theworths.org (Postfix) with ESMTP id 3FBE8431FAF\r
+ for <notmuch@notmuchmail.org>; Thu, 10 Oct 2013 08:09:22 -0700 (PDT)\r
+X-Virus-Scanned: Debian amavisd-new at olra.theworths.org\r
+X-Spam-Flag: NO\r
+X-Spam-Score: 0\r
+X-Spam-Level: \r
+X-Spam-Status: No, score=0 tagged_above=-999 required=5 tests=[none]\r
+ autolearn=disabled\r
+Received: from olra.theworths.org ([127.0.0.1])\r
+ by localhost (olra.theworths.org [127.0.0.1]) (amavisd-new, port 10024)\r
+ with ESMTP id tXKBosMVaM0u for <notmuch@notmuchmail.org>;\r
+ Thu, 10 Oct 2013 08:09:16 -0700 (PDT)\r
+Received: from guru.guru-group.fi (guru.guru-group.fi [46.183.73.34])\r
+ by olra.theworths.org (Postfix) with ESMTP id 9ECBE431FAE\r
+ for <notmuch@notmuchmail.org>; Thu, 10 Oct 2013 08:09:15 -0700 (PDT)\r
+Received: from guru.guru-group.fi (localhost [IPv6:::1])\r
+ by guru.guru-group.fi (Postfix) with ESMTP id 1E49D100030;\r
+ Thu, 10 Oct 2013 18:09:08 +0300 (EEST)\r
+From: Tomi Ollila <tomi.ollila@iki.fi>\r
+To: Ben Gamari <bgamari.foss@gmail.com>, notmuch@notmuchmail.org\r
+Subject: Re: [PATCH 1/3] database: Add notmuch_database_compact_close\r
+In-Reply-To: <1380745848-4972-2-git-send-email-bgamari.foss@gmail.com>\r
+References: <1380745848-4972-1-git-send-email-bgamari.foss@gmail.com>\r
+ <1380745848-4972-2-git-send-email-bgamari.foss@gmail.com>\r
+User-Agent: Notmuch/0.16+87~g451fefe (http://notmuchmail.org) Emacs/24.3.1\r
+ (x86_64-unknown-linux-gnu)\r
+X-Face: HhBM'cA~<r"^Xv\KRN0P{vn'Y"Kd;zg_y3S[4)KSN~s?O\"QPoL\r
+ $[Xv_BD:i/F$WiEWax}R(MPS`^UaptOGD`*/=@\1lKoVa9tnrg0TW?"r7aRtgk[F\r
+ !)g;OY^,BjTbr)Np:%c_o'jj,Z\r
+Date: Thu, 10 Oct 2013 18:09:07 +0300\r
+Message-ID: <m2ob6xw4bw.fsf@guru.guru-group.fi>\r
+MIME-Version: 1.0\r
+Content-Type: text/plain\r
+X-BeenThere: notmuch@notmuchmail.org\r
+X-Mailman-Version: 2.1.13\r
+Precedence: list\r
+List-Id: "Use and development of the notmuch mail system."\r
+ <notmuch.notmuchmail.org>\r
+List-Unsubscribe: <http://notmuchmail.org/mailman/options/notmuch>,\r
+ <mailto:notmuch-request@notmuchmail.org?subject=unsubscribe>\r
+List-Archive: <http://notmuchmail.org/pipermail/notmuch>\r
+List-Post: <mailto:notmuch@notmuchmail.org>\r
+List-Help: <mailto:notmuch-request@notmuchmail.org?subject=help>\r
+List-Subscribe: <http://notmuchmail.org/mailman/listinfo/notmuch>,\r
+ <mailto:notmuch-request@notmuchmail.org?subject=subscribe>\r
+X-List-Received-Date: Thu, 10 Oct 2013 15:09:22 -0000\r
+\r
+On Wed, Oct 02 2013, Ben Gamari <bgamari.foss@gmail.com> wrote:\r
+\r
+> This function uses Xapian's Compactor machinery to compact the notmuch\r
+> database. The compacted database is built in a temporary directory and\r
+> later moved into place while the original uncompacted database is\r
+> preserved.\r
+>\r
+> Signed-off-by: Ben Gamari <bgamari.foss@gmail.com>\r
+> ---\r
+> configure | 27 ++++++++--\r
+> lib/database.cc | 151 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++\r
+> lib/notmuch.h | 21 +++++++-\r
+> 3 files changed, 195 insertions(+), 4 deletions(-)\r
+>\r
+> diff --git a/configure b/configure\r
+> index 6166917..1a8e939 100755\r
+> --- a/configure\r
+> +++ b/configure\r
+> @@ -277,7 +277,8 @@ printf "Checking for Xapian development files... "\r
+> have_xapian=0\r
+> for xapian_config in ${XAPIAN_CONFIG}; do\r
+> if ${xapian_config} --version > /dev/null 2>&1; then\r
+> - printf "Yes (%s).\n" $(${xapian_config} --version | sed -e 's/.* //')\r
+> + xapian_version=$(${xapian_config} --version | sed -e 's/.* //')\r
+> + printf "Yes (%s).\n" ${xapian_version}\r
+> have_xapian=1\r
+> xapian_cxxflags=$(${xapian_config} --cxxflags)\r
+> xapian_ldflags=$(${xapian_config} --libs)\r
+> @@ -289,6 +290,21 @@ if [ ${have_xapian} = "0" ]; then\r
+> errors=$((errors + 1))\r
+> fi\r
+> \r
+> +# Compaction is only supported on Xapian > 1.2.6\r
+> +have_xapian_compact=0\r
+> +if [ ${have_xapian} = "1" ]; then\r
+> + printf "Checking for Xapian compaction support... "\r
+> + case "${xapian_version}" in\r
+> + 0.*|1.[01].*|1.2.[0-5])\r
+> + printf "No (only available with Xapian > 1.2.6).\n" ;;\r
+> + [1-9]*.[0-9]*.[0-9]*)\r
+> + have_xapian_compact=1\r
+> + printf "Yes.\n" ;;\r
+> + *)\r
+> + printf "Unknown version.\n" ;;\r
+> + esac\r
+> +fi\r
+> +\r
+> printf "Checking for GMime development files... "\r
+> have_gmime=0\r
+> IFS=';'\r
+> @@ -729,6 +745,9 @@ HAVE_STRCASESTR = ${have_strcasestr}\r
+> # build its own version)\r
+> HAVE_STRSEP = ${have_strsep}\r
+> \r
+> +# Whether the Xapian version in use supports compaction\r
+> +HAVE_XAPIAN_COMPACT = ${have_xapian_compact}\r
+> +\r
+> # Whether the getpwuid_r function is standards-compliant\r
+> # (if not, then notmuch will #define _POSIX_PTHREAD_SEMANTICS\r
+> # to enable the standards-compliant version -- needed for Solaris)\r
+> @@ -787,13 +806,15 @@ CONFIGURE_CFLAGS = -DHAVE_GETLINE=\$(HAVE_GETLINE) \$(GMIME_CFLAGS) \\\r
+> -DHAVE_STRCASESTR=\$(HAVE_STRCASESTR) \\\r
+> -DHAVE_STRSEP=\$(HAVE_STRSEP) \\\r
+> -DSTD_GETPWUID=\$(STD_GETPWUID) \\\r
+> - -DSTD_ASCTIME=\$(STD_ASCTIME)\r
+> + -DSTD_ASCTIME=\$(STD_ASCTIME) \\\r
+> + -DHAVE_XAPIAN_COMPACT=\$(HAVE_XAPIAN_COMPACT)\r
+> CONFIGURE_CXXFLAGS = -DHAVE_GETLINE=\$(HAVE_GETLINE) \$(GMIME_CFLAGS) \\\r
+> \$(TALLOC_CFLAGS) -DHAVE_VALGRIND=\$(HAVE_VALGRIND) \\\r
+> \$(VALGRIND_CFLAGS) \$(XAPIAN_CXXFLAGS) \\\r
+> -DHAVE_STRCASESTR=\$(HAVE_STRCASESTR) \\\r
+> -DHAVE_STRSEP=\$(HAVE_STRSEP) \\\r
+> -DSTD_GETPWUID=\$(STD_GETPWUID) \\\r
+> - -DSTD_ASCTIME=\$(STD_ASCTIME)\r
+> + -DSTD_ASCTIME=\$(STD_ASCTIME) \\\r
+> + -DHAVE_XAPIAN_COMPACT=\$(HAVE_XAPIAN_COMPACT)\r
+> CONFIGURE_LDFLAGS = \$(GMIME_LDFLAGS) \$(TALLOC_LDFLAGS) \$(XAPIAN_LDFLAGS)\r
+> EOF\r
+> diff --git a/lib/database.cc b/lib/database.cc\r
+> index bb4f180..06f1c0a 100644\r
+> --- a/lib/database.cc\r
+> +++ b/lib/database.cc\r
+> @@ -24,7 +24,9 @@\r
+> #include <iostream>\r
+> \r
+> #include <sys/time.h>\r
+> +#include <sys/stat.h>\r
+> #include <signal.h>\r
+> +#include <ftw.h>\r
+> \r
+> #include <glib.h> /* g_free, GPtrArray, GHashTable */\r
+> #include <glib-object.h> /* g_type_init */\r
+> @@ -268,6 +270,8 @@ notmuch_status_to_string (notmuch_status_t status)\r
+> return "Unbalanced number of calls to notmuch_message_freeze/thaw";\r
+> case NOTMUCH_STATUS_UNBALANCED_ATOMIC:\r
+> return "Unbalanced number of calls to notmuch_database_begin_atomic/end_atomic";\r
+> + case NOTMUCH_STATUS_UNSUPPORTED_OPERATION:\r
+> + return "Unsupported operation";\r
+> default:\r
+> case NOTMUCH_STATUS_LAST_STATUS:\r
+> return "Unknown error status value";\r
+> @@ -800,6 +804,153 @@ notmuch_database_close (notmuch_database_t *notmuch)\r
+> notmuch->date_range_processor = NULL;\r
+> }\r
+> \r
+> +#if HAVE_XAPIAN_COMPACT\r
+> +static int unlink_cb (const char *path,\r
+> + unused (const struct stat *sb),\r
+> + unused (int type),\r
+> + unused (struct FTW *ftw))\r
+> +{\r
+> + return remove(path);\r
+> +}\r
+> +\r
+> +static int rmtree (const char *path)\r
+> +{\r
+> + return nftw(path, unlink_cb, 64, FTW_DEPTH | FTW_PHYS);\r
+> +}\r
+> +\r
+> +class NotmuchCompactor : public Xapian::Compactor\r
+> +{\r
+> + notmuch_compact_status_cb_t status_cb;\r
+> +\r
+> +public:\r
+> + NotmuchCompactor(notmuch_compact_status_cb_t cb) : status_cb(cb) { }\r
+> +\r
+> + virtual void\r
+> + set_status (const std::string &table, const std::string &status)\r
+> + {\r
+> + char* msg;\r
+> +\r
+> + if (status_cb == NULL)\r
+> + return;\r
+> +\r
+> + if (status.length() == 0)\r
+> + msg = talloc_asprintf (NULL, "compacting table %s", table.c_str());\r
+> + else\r
+> + msg = talloc_asprintf (NULL, " %s", status.c_str());\r
+> +\r
+> + if (msg == NULL) {\r
+> + return;\r
+> + }\r
+> +\r
+> + status_cb(msg);\r
+> + talloc_free(msg);\r
+> + }\r
+> +};\r
+> +\r
+> +/* Compacts the given database, optionally saving the original database\r
+> + * in backup_path. Additionally, a callback function can be provided to\r
+> + * give the user feedback on the progress of the (likely long-lived)\r
+> + * compaction process.\r
+> + *\r
+> + * The backup path must point to a directory on the same volume as the\r
+> + * original database. Passing a NULL backup_path will result in the\r
+> + * uncompacted database being deleted after compaction has finished.\r
+> + * Note that the database write lock will be held during the\r
+> + * compaction process to protect data integrity.\r
+> + */\r
+> +notmuch_status_t\r
+> +notmuch_database_compact (const char* path,\r
+> + const char* backup_path,\r
+> + notmuch_compact_status_cb_t status_cb)\r
+> +{\r
+> + void *local = talloc_new (NULL);\r
+> + NotmuchCompactor compactor(status_cb);\r
+> + char *notmuch_path, *xapian_path, *compact_xapian_path;\r
+> + char *old_xapian_path = NULL;\r
+> + notmuch_status_t ret = NOTMUCH_STATUS_SUCCESS;\r
+> + notmuch_database_t *notmuch = NULL;\r
+> + struct stat statbuf;\r
+> +\r
+> + ret = notmuch_database_open(path, NOTMUCH_DATABASE_MODE_READ_WRITE, ¬much);\r
+> + if (ret) {\r
+> + goto DONE;\r
+> + }\r
+> +\r
+> + if (! (notmuch_path = talloc_asprintf (local, "%s/%s", path, ".notmuch"))) {\r
+> + ret = NOTMUCH_STATUS_OUT_OF_MEMORY;\r
+> + goto DONE;\r
+> + }\r
+> +\r
+> + if (! (xapian_path = talloc_asprintf (local, "%s/%s", notmuch_path, "xapian"))) {\r
+> + ret = NOTMUCH_STATUS_OUT_OF_MEMORY;\r
+> + goto DONE;\r
+> + }\r
+> +\r
+> + if (! (compact_xapian_path = talloc_asprintf (local, "%s.compact", xapian_path))) {\r
+> + ret = NOTMUCH_STATUS_OUT_OF_MEMORY;\r
+> + goto DONE;\r
+> + }\r
+> +\r
+> + if (backup_path != NULL) {\r
+> + if (! (old_xapian_path = talloc_asprintf (local, "%s/xapian.old", backup_path))) {\r
+> + ret = NOTMUCH_STATUS_OUT_OF_MEMORY;\r
+> + goto DONE;\r
+> + }\r
+> +\r
+> + if (stat(old_xapian_path, &statbuf) != -1) {\r
+> + fprintf (stderr, "Backup path already exists: %s\n", old_xapian_path);\r
+> + ret = NOTMUCH_STATUS_FILE_ERROR;\r
+> + goto DONE;\r
+> + }\r
+> + if (errno != ENOENT) {\r
+> + fprintf (stderr, "Unknown error while stat()ing backup path: %s\n",\r
+> + strerror(errno));\r
+> + goto DONE;\r
+> + }\r
+> + }\r
+> +\r
+> + try {\r
+> + compactor.set_renumber(false);\r
+> + compactor.add_source(xapian_path);\r
+> + compactor.set_destdir(compact_xapian_path);\r
+> + compactor.compact();\r
+\r
+>From functionality point if view this looks safe to me. \r
+A followup patch could provide more information to the user\r
+is any of the following attemts fail, e.g. \r
+\r
+- if removing old database out of the way how to remove the new\r
+ compacted database which can be considered as garbage now -- or\r
+ how to rename it (which is a bit dangerous due to potential races)\r
+\r
+- if moving compacted database fails how to restore backup database...\r
+ ... or how to move compacted database to where it was supposed to be\r
+ moved so that database is usable...\r
+\r
+... if the database is missing is new created from scratch, also in case\r
+there already is .notmuch directory ?\r
+\r
+... should the database open try to open database from these alternative\r
+names in case opening from original name fails ?\r
+\r
+another, small change:\r
+\r
+ case "${xapian_version}" in\r
+- 0.*|1.[01].*|1.2.[0-5])\r
++ 0.*|1.[01].*|1.2.[0-5]|1.2.[0-5][^0-9]*)\r
+ printf "No (only available with Xapian > 1.2.6).\n" ;;\r
+\r
+someone might do version like 1.2.4-abc but probably not 1.2.04 (nor 1.2a.4)\r
+\r
+(side note: case $x in [^...]) works with bash (and ksh&zsh, but not with dash)\r
+\r
+\r
+> + } catch (Xapian::InvalidArgumentError e) {\r
+> + fprintf (stderr, "Error while compacting: %s\n", e.get_msg().c_str());\r
+> + ret = NOTMUCH_STATUS_XAPIAN_EXCEPTION;\r
+> + goto DONE;\r
+> + }\r
+> +\r
+> + if (old_xapian_path != NULL) {\r
+> + if (rename(xapian_path, old_xapian_path)) {\r
+> + fprintf (stderr, "Error moving old database out of the way\n");\r
+> + ret = NOTMUCH_STATUS_FILE_ERROR;\r
+> + goto DONE;\r
+> + }\r
+> + } else {\r
+> + rmtree(xapian_path);\r
+> + }\r
+> +\r
+> + if (rename(compact_xapian_path, xapian_path)) {\r
+> + fprintf (stderr, "Error moving compacted database\n");\r
+> + ret = NOTMUCH_STATUS_FILE_ERROR;\r
+> + goto DONE;\r
+> + }\r
+> +\r
+> + notmuch_database_close(notmuch);\r
+> +\r
+> +DONE:\r
+> + talloc_free(local);\r
+> + return ret;\r
+> +}\r
+> +#else\r
+> +notmuch_status_t\r
+> +notmuch_database_compact_close (unused (notmuch_database_t *notmuch))\r
+> +{\r
+> + fprintf (stderr, "notmuch was compiled against a xapian version lacking compaction support.\n");\r
+> + return NOTMUCH_STATUS_UNSUPPORTED_OPERATION;\r
+> +}\r
+> +#endif\r
+> +\r
+> void\r
+> notmuch_database_destroy (notmuch_database_t *notmuch)\r
+> {\r
+> diff --git a/lib/notmuch.h b/lib/notmuch.h\r
+> index 998a4ae..9dab555 100644\r
+> --- a/lib/notmuch.h\r
+> +++ b/lib/notmuch.h\r
+> @@ -101,6 +101,7 @@ typedef enum _notmuch_status {\r
+> NOTMUCH_STATUS_TAG_TOO_LONG,\r
+> NOTMUCH_STATUS_UNBALANCED_FREEZE_THAW,\r
+> NOTMUCH_STATUS_UNBALANCED_ATOMIC,\r
+> + NOTMUCH_STATUS_UNSUPPORTED_OPERATION,\r
+> \r
+> NOTMUCH_STATUS_LAST_STATUS\r
+> } notmuch_status_t;\r
+> @@ -215,8 +216,26 @@ notmuch_database_open (const char *path,\r
+> void\r
+> notmuch_database_close (notmuch_database_t *database);\r
+> \r
+> +/* A callback invoked by notmuch_database_compact to notify the user\r
+> + * of the progress of the compaction process.\r
+> + */\r
+> +typedef void (*notmuch_compact_status_cb_t)(const char*);\r
+> +\r
+> +/* Compact a notmuch database, backing up the original database to the\r
+> + * given path.\r
+> + *\r
+> + * The database will be opened with NOTMUCH_DATABASE_MODE_READ_WRITE\r
+> + * during the compaction process to ensure no writes are made.\r
+> + *\r
+> + */\r
+> +notmuch_status_t\r
+> +notmuch_database_compact (const char* path,\r
+> + const char* backup_path,\r
+> + notmuch_compact_status_cb_t status_cb);\r
+> +\r
+> /* Destroy the notmuch database, closing it if necessary and freeing\r
+> -* all associated resources. */\r
+> + * all associated resources.\r
+> + */\r
+> void\r
+> notmuch_database_destroy (notmuch_database_t *database);\r
+> \r
+> -- \r
+> 1.8.1.2\r
+>\r
+> _______________________________________________\r
+> notmuch mailing list\r
+> notmuch@notmuchmail.org\r
+> http://notmuchmail.org/mailman/listinfo/notmuch\r