Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sync failure vom cyrus-imap 3.0 to 3.2 or newer for Mailboxes with ANNOTATIONS due to missing MODSEQ #4967

Open
MichaelMenge opened this issue Jul 9, 2024 · 5 comments

Comments

@MichaelMenge
Copy link
Contributor

MichaelMenge commented Jul 9, 2024

Trying to sync form 3.0 to 3.4 I was unable to sycn some folders

Initial report https://cyrus.topicbox.com/groups/info/T8556e55b2133ffdf-M14a53cba2a5af4cd506ea63b

during debugging I found that Cyrus 3.2 and newer are expecting a MODSEQ for ANNOTATIONS
see imap/sync_support.c:1515

but cyrus-imapd-3.0 sync_clients do not send a MODSEQ for ANNOTATION

Edit:
The problem is also confiremd for replicating from 3.0 to 3.2
see https://lists.andrew.cmu.edu/pipermail/info-cyrus/2020-September/041396.html

@MichaelMenge
Copy link
Contributor Author

FYI the messages did have "/vendor/cmu/cyrus-imapd/thrid" ANNOTATION.

It took me some time, but i found that "thrid" is short for Thread ID
And that the Thread ID / Conversation ID was stored as ANNOTATION before "version 12"
https://github.com/cyrusimap/cyrus-imapd/blob/5604bdaf83d8a9ceeaf7cb9f51c7634ea0c4501c/imap/annotate.c#L2098C9-L2098C94

We did test the conversationdb at some point, but did disable it again shortly after.
I suspect that the ANNOTATIONS could be some artifacts of the test

@elliefm
Copy link
Contributor

elliefm commented Jul 12, 2024

elliefm@a49023d

This patch (unfinished) makes it lenient about accepting missing MODSEQ (and also missing USERID, based on parse_annotation() elsewhere in this file describing USERID as optional). This gets us past the "IMAP_PROTOCOL_ERROR" issue, it no longer outright rejects annotations without modseqs.

But, now it fails on a CRC mismatch. "SYNCNOTICE: CRC error after apply" appears in syslog, and ">1720757254>S3 NO IMAP_SYNC_CHECKSUM Checksum Failure" in telemetry. On newer versions which log the mismatched CRCs, it's clear that it's the annot CRC that's mismatched. This is surprising to me, since crc_annot() does not use modseq in its calculation.

There was a change in crc_annot() a while ago about missing USERID, but USERID isn't missing in this case, so I think it's a red herring. But that means I can't explain why the CRCs don't match, though they definitely don't.

Patch behaves the same whether applied to master, 3.4, or 3.2.

@MichaelMenge
Copy link
Contributor Author

hello elli,

I can confirm that the patch did solve the IMAP_PROTOCOL_ERROR but does run into IMAP_SYNC_CHECKSUM.

The 3.0 cyrus did compute a Checksum with an empty userid,
buf_printf(&buf, "%u %s %s ", uid, entry, userid ? userid : "");
while 3.4 does return 0 if there is no userid if (!userid) return 0;

So we need to check for the case where the checksum on the server is 0

@elliefm
Copy link
Contributor

elliefm commented Jul 15, 2024

In my test, USERID isn't missing, but it still fails the CRC checks, so I think the missing USERID is a distraction.

I've added a bunch of logging to observe the CRC values returned by crc_annot, and they have not changed between 3.0 and 3.2 or even 3.11. But the final value that's saved to the mailbox is different between 3.0 and 3.2+, and that's where the mismatch is detected and rejected. Not sure this is even fixable without just breaking compatibility between some other versions instead.

Do these mailboxes fail to replicate on a first replication to an empty replica? Or do they work fine the first time, but fail later on incremental updates?

@MichaelMenge
Copy link
Contributor Author

I did could not test with a partial synced folder.
My production servers are now all on 3.4

I did a sync_reset after my first test with your a49023d patch to ensure that the fairure was not due to an
inconsistent sync state.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants