lib-index: When flushing cache changes to disk, try to also write offsets to transact...

This works as long as there aren't any newly created non-committed mails.

diff --git a/src/lib-index/mail-cache-transaction.c b/src/lib-index/mail-cache-transaction.c
index c6f2e0680a5e6d66993a0b6942c60806d3a79e78..d75de363a8cd6f3fec8a1fde7b90e41aaf033a4a 100644 (file)
--- a/src/lib-index/mail-cache-transaction.c
+++ b/src/lib-index/mail-cache-transaction.c
@@ -49,6 +49,7 @@ struct mail_cache_transaction_ctx {
 
        bool tried_purging:1;
        bool decisions_refreshed:1;
+       bool have_noncommited_mails:1;
        bool changes:1;
 };
 
@@ -293,14 +294,31 @@ mail_cache_transaction_lookup_rec(struct mail_cache_transaction_ctx *ctx,
 
 static void
 mail_cache_transaction_update_index(struct mail_cache_transaction_ctx *ctx,
-                                   uint32_t write_offset)
+                                   uint32_t write_offset, bool committing)
 {
        struct mail_cache *cache = ctx->cache;
+       struct mail_index_transaction *trans;
        const struct mail_cache_record *rec = ctx->cache_data->data;
        const struct mail_cache_transaction_rec *recs;
        uint32_t i, seq_count;
 
-       mail_index_ext_using_reset_id(ctx->trans, ctx->cache->ext_id,
+       if (committing) {
+               /* The transaction is being committed now. Use it. */
+               trans = ctx->trans;
+       } else if (ctx->have_noncommited_mails) {
+               /* Some of the mails haven't been committed yet. We must use
+                  the provided transaction to update the cache records. */
+               trans = ctx->trans;
+       } else {
+               /* We can commit these changes immediately. This way even if
+                  the provided transaction runs for a very long time, we
+                  still once in a while commit the cache changes so they
+                  become visible to other processes as well. */
+               trans = mail_index_transaction_begin(ctx->view->trans_view,
+                       MAIL_INDEX_TRANSACTION_FLAG_EXTERNAL);
+       }
+
+       mail_index_ext_using_reset_id(trans, ctx->cache->ext_id,
                                      ctx->cache_file_seq);
 
        /* write the cache_offsets to index file. records' prev_offset
@@ -308,13 +326,20 @@ mail_cache_transaction_update_index(struct mail_cache_transaction_ctx *ctx,
           synced. */
        recs = array_get(&ctx->cache_data_seq, &seq_count);
        for (i = 0; i < seq_count; i++) {
-               mail_index_update_ext(ctx->trans, recs[i].seq, cache->ext_id,
+               mail_index_update_ext(trans, recs[i].seq, cache->ext_id,
                                      &write_offset, NULL);
 
                write_offset += rec->size;
                rec = CONST_PTR_OFFSET(rec, rec->size);
                ctx->records_written++;
        }
+       if (trans != ctx->trans) {
+               if (mail_index_transaction_commit(&trans) < 0) {
+                       /* failed, but can't really do anything */
+               } else {
+                       ctx->records_written = 0;
+               }
+       }
 }
 
 static int
@@ -474,7 +499,8 @@ mail_cache_transaction_drop_last_flush(struct mail_cache_transaction_ctx *ctx)
 }
 
 static int
-mail_cache_transaction_flush(struct mail_cache_transaction_ctx *ctx)
+mail_cache_transaction_flush(struct mail_cache_transaction_ctx *ctx,
+                            bool committing)
 {
        struct stat st;
        uint32_t write_offset = 0;
@@ -521,7 +547,8 @@ mail_cache_transaction_flush(struct mail_cache_transaction_ctx *ctx)
                ret = -1;
        else {
                /* update records' cache offsets to index */
-               mail_cache_transaction_update_index(ctx, write_offset);
+               mail_cache_transaction_update_index(ctx, write_offset,
+                                                   committing);
        }
        if (mail_cache_flush_and_unlock(ctx->cache) < 0)
                ret = -1;
@@ -628,7 +655,7 @@ int mail_cache_transaction_commit(struct mail_cache_transaction_ctx **_ctx)
        if (ctx->changes) {
                if (ctx->prev_seq != 0)
                        mail_cache_transaction_update_last_rec(ctx);
-               if (mail_cache_transaction_flush(ctx) < 0)
+               if (mail_cache_transaction_flush(ctx, TRUE) < 0)
                        ret = -1;
                else {
                        /* successfully wrote everything */
@@ -733,6 +760,9 @@ void mail_cache_add(struct mail_cache_transaction_ctx *ctx, uint32_t seq,
            (MAIL_CACHE_DECISION_NO | MAIL_CACHE_DECISION_FORCED))
                return;
 
+       if (seq >= ctx->trans->first_new_seq)
+               ctx->have_noncommited_mails = TRUE;
+
        /* If the cache file exists, make sure the caching decisions have been
           read. */
        mail_cache_transaction_refresh_decisions(ctx);
@@ -789,7 +819,7 @@ void mail_cache_add(struct mail_cache_transaction_ctx *ctx, uint32_t seq,
                                full_size - MAIL_CACHE_MAX_WRITE_BUFFER;
                        mail_cache_transaction_drop_unwanted(ctx, space_needed);
                } else {
-                       if (mail_cache_transaction_flush(ctx) < 0) {
+                       if (mail_cache_transaction_flush(ctx, FALSE) < 0) {
                                /* If this is a syscall failure, the already
                                   flushed changes could still be finished by
                                   writing the offsets to .log file. If this is

diff --git a/src/lib-index/test-mail-cache-purge.c b/src/lib-index/test-mail-cache-purge.c
index 170f57d1698927b37aa3e31de6c89fca49afdfd9..ba99d48d40179acafa513868dcc8553aee610f1b 100644 (file)
--- a/src/lib-index/test-mail-cache-purge.c
+++ b/src/lib-index/test-mail-cache-purge.c
@@ -235,7 +235,8 @@ static bool cache_equals(struct mail_cache_view *cache_view, uint32_t seq,
        return match;
 }
 
-static void test_mail_cache_write_lost_during_purge_n(unsigned int num_mails)
+static void test_mail_cache_purge_during_write_n(unsigned int num_mails,
+                                                bool commit_saves)
 {
        const struct mail_index_optimization_settings optimization_set = {
                .cache = {
@@ -257,11 +258,13 @@ static void test_mail_cache_write_lost_during_purge_n(unsigned int num_mails)
        trans = mail_index_transaction_begin(ctx.view, 0);
        for (seq = 2; seq <= num_mails; seq++)
                mail_index_append(trans, seq, &seq);
-       test_assert(mail_index_transaction_commit(&trans) == 0);
-       test_mail_cache_view_sync(&ctx);
 
+       if (commit_saves) {
+               test_assert(mail_index_transaction_commit(&trans) == 0);
+               test_mail_cache_view_sync(&ctx);
+               trans = mail_index_transaction_begin(ctx.view, 0);
+       }
        /* start adding a small cached field to mail1 */
-       trans = mail_index_transaction_begin(ctx.view, 0);
        updated_view = mail_index_transaction_open_updated_view(trans);
        cache_view = mail_cache_view_open(ctx.cache, updated_view);
        cache_trans = mail_cache_get_transaction(cache_view, trans);
@@ -285,7 +288,7 @@ static void test_mail_cache_write_lost_during_purge_n(unsigned int num_mails)
                /* the mails are still accessible after purge */
                test_assert(cache_equals(cache_view, 1, ctx.cache_field.idx, "foo1"));
                test_assert(cache_equals(cache_view, 2, ctx.cache_field.idx, huge_field));
-       } else {
+       } else if (!commit_saves) {
                /* add 3rd mail, which attempts to flush 2nd mail and finds
                   that the first mail is already lost */
                test_expect_error_string("Purging lost 1 written cache records");
@@ -295,20 +298,29 @@ static void test_mail_cache_write_lost_during_purge_n(unsigned int num_mails)
                test_assert(cache_equals(cache_view, 1, ctx.cache_field.idx, NULL));
                test_assert(cache_equals(cache_view, 2, ctx.cache_field.idx, huge_field));
                test_assert(cache_equals(cache_view, 3, ctx.cache_field.idx, "foo3"));
+       } else {
+               /* add 3rd mail, which commits the first two mails */
+               mail_cache_add(cache_trans, 3, ctx.cache_field.idx, "foo3", 4);
+               test_assert(cache_equals(cache_view, 1, ctx.cache_field.idx, "foo1"));
+               test_assert(cache_equals(cache_view, 2, ctx.cache_field.idx, huge_field));
+               test_assert(cache_equals(cache_view, 3, ctx.cache_field.idx, "foo3"));
        }
 
        /* finish committing cached fields */
-       if (num_mails == 2)
+       if (num_mails == 2 && !commit_saves)
                test_expect_error_string("Purging lost 1 written cache records");
        test_assert(mail_index_transaction_commit(&trans) == 0);
        test_expect_no_more_errors();
        mail_index_view_close(&updated_view);
        mail_cache_view_close(&cache_view);
 
-       /* see that we lost the first flush, but not the others */
+       /* see that we lost the first flush without commit_saves, but not the others */
        test_mail_cache_view_sync(&ctx);
        cache_view = mail_cache_view_open(ctx.cache, ctx.view);
-       test_assert(cache_equals(cache_view, 1, ctx.cache_field.idx, NULL));
+       if (commit_saves)
+               test_assert(cache_equals(cache_view, 1, ctx.cache_field.idx, "foo1"));
+       else
+               test_assert(cache_equals(cache_view, 1, ctx.cache_field.idx, NULL));
        test_assert(cache_equals(cache_view, 2, ctx.cache_field.idx, huge_field));
        if (num_mails >= 3)
                test_assert(cache_equals(cache_view, 3, ctx.cache_field.idx, "foo3"));
@@ -324,14 +336,28 @@ static void test_mail_cache_write_lost_during_purge_n(unsigned int num_mails)
 static void test_mail_cache_write_lost_during_purge(void)
 {
        test_begin("mail cache write lost during purge");
-       test_mail_cache_write_lost_during_purge_n(2);
+       test_mail_cache_purge_during_write_n(2, FALSE);
        test_end();
 }
 
 static void test_mail_cache_write_lost_during_purge2(void)
 {
        test_begin("mail cache write lost during purge (2)");
-       test_mail_cache_write_lost_during_purge_n(3);
+       test_mail_cache_purge_during_write_n(3, FALSE);
+       test_end();
+}
+
+static void test_mail_cache_write_autocommit(void)
+{
+       test_begin("mail cache write autocommit");
+       test_mail_cache_purge_during_write_n(2, TRUE);
+       test_end();
+}
+
+static void test_mail_cache_write_autocommit2(void)
+{
+       test_begin("mail cache write autocommit");
+       test_mail_cache_purge_during_write_n(3, TRUE);
        test_end();
 }
 
@@ -1024,6 +1050,8 @@ int main(void)
                test_mail_cache_purge_while_cache_locked,
                test_mail_cache_write_lost_during_purge,
                test_mail_cache_write_lost_during_purge2,
+               test_mail_cache_write_autocommit,
+               test_mail_cache_write_autocommit2,
                test_mail_cache_delete_too_large,
                test_mail_cache_delete_too_large2,
                test_mail_cache_purge_too_large,