From: Timo Sirainen <timo.sirainen@open-xchange.com>
Date: Mon, 10 Jun 2019 20:07:56 +0000 (+0300)
Subject: lib-index: Compress cache immediately after enough mails have been expunged
X-Git-Tag: 2.3.9~431
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=e31b0637d8788885a71db2def5743ebf14c698f3;p=thirdparty%2Fdovecot%2Fcore.git

lib-index: Compress cache immediately after enough mails have been expunged

This fixes a bug where cache may never become compressed in certain mailbox
access patterns. Especially for autoexpunging in lazy_expunge mailboxes.
The cache compression happened only when:

a) After the mailbox_sync() that finishes expunging there was another
mailbox_sync(). If mailbox was immediately closed after expunging, this
didn't happen.

b) Fields are fetched from cache

If neither of these happened, the cache just kept growing. This happened
especially with lazy_expunge mailboxes.

sdbox format was always doing b) during expunging, because it looked up
GUIDs from cache. However, this helped only with regular expunges, not
with autoexpunges, because autoexpunging didn't finish with any
mailbox_sync() (which is a bug on its own).

mdbox also did GUID lookups from cache, but it wasn't doing cache
compressions due the bug fixed by the previous commit.
---

diff --git a/src/lib-index/mail-index-sync.c b/src/lib-index/mail-index-sync.c
index 2e8082b5c2..a133552097 100644
--- a/src/lib-index/mail-index-sync.c
+++ b/src/lib-index/mail-index-sync.c
@@ -879,13 +879,6 @@ int mail_index_sync_commit(struct mail_index_sync_ctx **_ctx)
 	}
 
 	mail_index_sync_update_mailbox_offset(ctx);
-	if (mail_cache_need_compress(index->cache)) {
-		/* if cache compression fails, we don't really care.
-		   the cache offsets are updated only if the compression was
-		   successful. */
-		(void)mail_cache_compress(index->cache, ctx->ext_trans,
-					  &cache_lock);
-	}
 
 	if ((ctx->flags & MAIL_INDEX_SYNC_FLAG_DROP_RECENT) != 0) {
 		next_uid = mail_index_transaction_get_next_uid(ctx->ext_trans);
@@ -906,8 +899,6 @@ int mail_index_sync_commit(struct mail_index_sync_ctx **_ctx)
 	}
 
 	ret2 = mail_index_transaction_commit(&ctx->ext_trans);
-	if (cache_lock != NULL)
-		mail_cache_compress_unlock(&cache_lock);
 	if (ret2 < 0) {
 		mail_index_sync_end(&ctx);
 		return -1;
@@ -928,6 +919,27 @@ int mail_index_sync_commit(struct mail_index_sync_ctx **_ctx)
 		ret = -1;
 	index->sync_commit_result = NULL;
 
+	/* The previously called expunged handlers will update cache's
+	   record_count and deleted_record_count. That also has a side effect
+	   of updating whether cache needs to be compressed. */
+	if (ret == 0 && mail_cache_need_compress(index->cache)) {
+		struct mail_index_transaction *cache_trans;
+		enum mail_index_transaction_flags trans_flags;
+
+		trans_flags = MAIL_INDEX_TRANSACTION_FLAG_EXTERNAL;
+		if ((ctx->flags & MAIL_INDEX_SYNC_FLAG_FSYNC) != 0)
+			trans_flags |= MAIL_INDEX_TRANSACTION_FLAG_FSYNC;
+		cache_trans = mail_index_transaction_begin(ctx->view, trans_flags);
+		if (mail_cache_compress(index->cache, cache_trans,
+					&cache_lock) < 0)
+			mail_index_transaction_rollback(&cache_trans);
+		else {
+			/* can't really do anything if index commit fails */
+			(void)mail_index_transaction_commit(&cache_trans);
+			mail_cache_compress_unlock(&cache_lock);
+		}
+	}
+
 	want_rotate = mail_transaction_log_want_rotate(index->log);
 	if (ret == 0 &&
 	    (want_rotate || mail_index_sync_want_index_write(index))) {