dovecot - Apr 2013 - [PATCH] mailbox_get_metadata() for mailboxes with open transactions.

Hi everyone,

I am writing a dovecot statistic plugin, which calls 
mailbox_get_metadata() to get the virtual size of the mailbox, if a mail 
is copied. I have noticed that mailbox_get_metadata() calls 
mailbox_sync(), and mailbox_sync() will fail for the mailboxes with open 
transactions. But if a mail is copied, there must be at least one 
transaction opened for the mailbox. So I can not get the virtual size.

Commit [1] has fixed the same problem in mailbox_get_status(). I think 
the same should also be done in mailbox_get_metadata(). See the patch below.

Dovecot version: 2.2.0
OS: OS: Linux 3.4.39-dist i686
maildir:~/Maildir:LAYOUT=fs:INBOX=~/Maildir/INBOX

Dafan


[1] http://hg.dovecot.org/dovecot-2.2/rev/12136db6e31f


# HG changeset patch
# User Dafan Zhai <dafan.zhai at securepoint.de>
# Date 1366118323 -7200
# Node ID 7f0c5122d863d303ef5e74bbf650c2bac89ac564
# Parent f33bacb03cc22e02ca2d11d527d8dc0d99214486
lib-storage: mailbox_get_metadata() no longer forces mailbox to be synced.
This fixes getting virtual size through mailbox_get_metadata() of 
mailboxes with open transactions.

diff -r f33bacb03cc2 -r 7f0c5122d863 src/lib-storage/index/index-status.c
--- a/src/lib-storage/index/index-status.c Tue Apr 16 15:54:05 2013 +0300
+++ b/src/lib-storage/index/index-status.c Tue Apr 16 15:18:43 2013 +0200
@@ -371,10 +371,10 @@
if (!box->opened) {
if (mailbox_open(box) < 0)
return -1;
- }
- if (!box->synced && (items & MAILBOX_METADATA_SYNC_ITEMS) != 0)
{
- if (mailbox_sync(box, MAILBOX_SYNC_FLAG_FAST) < 0)
- return -1;
+ if ((items & MAILBOX_METADATA_SYNC_ITEMS) != 0) {
+ if (mailbox_sync(box, MAILBOX_SYNC_FLAG_FAST) < 0)
+ return -1;
+ }
}

if ((items & MAILBOX_METADATA_VIRTUAL_SIZE) != 0) {

On 16.4.2013, at 17.04, Dafan Zhai <dafan.zhai at securepoint.de> wrote:
> I am writing a dovecot statistic plugin, which calls mailbox_get_metadata()
to get the virtual size of the mailbox, if a mail is copied. I have noticed that
mailbox_get_metadata() calls mailbox_sync(), and mailbox_sync() will fail for
the mailboxes with open transactions. But if a mail is copied, there must be at
least one transaction opened for the mailbox. So I can not get the virtual size.
If you hook into mailbox_copy() you'll be calling it for each mail
separately that is copied within the same command. Also you're calling it
even if the copy fails later (e.g. IMAP COPY gets aborted if some messages are
missing). So you should delay calling it until after transaction commit (set
your own flag to the transaction that messages were copied). Maybe even until
the next sync is called.
> Commit [1] has fixed the same problem in mailbox_get_status(). I think the
same should also be done in mailbox_get_metadata(). See the patch below.
Too much of a chance of breaking some code that relies on the current behavior.