From: Timo Sirainen Date: Wed, 1 Apr 2020 19:53:55 +0000 (+0300) Subject: lib-index: When flushing cache changes to disk, try to also write offsets to transact... X-Git-Tag: 2.3.11.2~343 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=9efb99924d0b7de27ca83e373f2290f3dd5b22cf;p=thirdparty%2Fdovecot%2Fcore.git lib-index: When flushing cache changes to disk, try to also write offsets to transaction log 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 c6f2e0680a..d75de363a8 100644 --- 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 170f57d169..ba99d48d40 100644 --- 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,