Re: [PATCH 1/3] database: Add notmuch_database_compact_close
authorTomi Ollila <tomi.ollila@iki.fi>
Thu, 10 Oct 2013 15:09:07 +0000 (18:09 +0300)
committerW. Trevor King <wking@tremily.us>
Fri, 7 Nov 2014 17:57:23 +0000 (09:57 -0800)
bc/8da4cc384e9541e637f964885d3013b8d4d32c [new file with mode: 0644]

diff --git a/bc/8da4cc384e9541e637f964885d3013b8d4d32c b/bc/8da4cc384e9541e637f964885d3013b8d4d32c
new file mode 100644 (file)
index 0000000..399c46f
--- /dev/null
@@ -0,0 +1,384 @@
+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, &notmuch);\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