XFER from 2.4.18 to 2.5.7 destroys cyrus.index if annotation exists

[brong]

From: Wolfgang Breyha Bugzilla-Id: 3927 Version: 2.5.x (next) Owner: Bron Gondwana

[brong]

From: Wolfgang Breyha

I transfer a mailbox from 2.4.18 to 2.5.7 within my murder cluster.

My 2.4.18 is bugfixed for #2922 to prevent index version downgrade and dumps version 12 index in binary format. That works perfectly as long as the mailbox has no annotations (like squat, expire, ...).

If it has annotations cyrus 2.5.7 mangles the cyrus.index to death by overwriting the UNDUMPed version 12 cyrus.index with an empty version 13 cyrus.index. It seems it roles back to the state it had after the mailbox was created on the new backend by failing to reread the transferred index file.

To make it even worse the cyrus.header and cyrus.cache is kept from the UNDUMP leaving the mailbox in a state which trigger SEGV in reconstruct. I have to call reconstruct at least twice to fix such a mailbox. State information of the mailbox is lost.

[brong]

From: Wolfgang Breyha

Meanwhile I was able to get a strace of the undump call. It clearly shows that if an annotation is transferred the first mailbox_close() in mbdump.c:1238 does not close the old filehandles of cyrus.index and cyrus.cache as "usual"/without annotation.

After the quota gets updated the second mailbox_close() in mbdump.c:1302 fsync's the empty cyrus.index from LOCALCREATE back and overwrites the newly UNDUMPed version.

As already mentioned the mailbox ends up in a state which lets reconstruct crash.

I did not find the cause yet and ask for the urgent help of the cyrus maintainers. Please fix #2922 for 2.4 and this bug. A working upgrade path and interoperability between supported cyrus-imapd version is essential in a murder cluster.

IMO this can happen on 2.5 => 2.5 backend transfers as well.

[brong]

From: Wolfgang Breyha

All the references to #2922 are wrong. I meant #3922 "index version downgrade". Sorry

[brong]

Attachment-Id: 1569 From: Wolfgang Breyha Type: text/plain File: cyrus257-mbdump_annotation_reopen_index.patch

reopen mailbox if first annotation is detected

[brong]

From: elliefm

Thanks for the patch, it's now on cyrus-imapd-2.5 and master branches.

I'm not sure if the patch is the correct fix for the problem, or a workaround for a deeper underlying issue? Leaving this open for further investigation.

[brong]

From: Wolfgang Breyha

I traced it down as follows:

mbdump.c:undump()->annotate.c:annotate_state_write()->write_entry()->mailbox.c:mailbox_annot_changed().

There mailbox_index_dirty() is called unconditionally what causes the overwriting of the freshly undumped cyrus.index if a annotation was transferred

But IMO the only right thing to do is to reread the undumped cyrus.header|index as soon as needed to work on the correct copies regardless what happens next. That's why I chose to add the close_mailbox to the first annotation read and move annotate_state_set_mailbox to this moment as well.

A more "correct" fix could be to add some synchronization point to dump/undump marking the point at which all binary cyrus data was transferred to give the recipient a chance to reread/reopen all the binary stuff before starting with other things like quota, annotations, ....

I recognized that eg. the ACLs are not reread as well yet. They are set bei a distinct GETACL/SETACL IMAP command from the transferring host. But the ACLs are not reread from cyrus.header after UNDUMPed. I recognized it because I always set all folders to "rd" before XFERing them. But the GETACL returns "all" of a freshly created mailbox getting SET(ACL) afterwards to "rd".