cyrusimap / cyrus-imapd

Cyrus IMAP is an email, contacts and calendar server
http://cyrusimap.org
Other
532 stars 146 forks source link

sync_client: add flag for reserving all mailbox messages, not just new ones #1818

Open elliefm opened 7 years ago

elliefm commented 7 years ago

sync_client is currently unable to detect/re-upload messages that are present in replica index but missing on replica storage.

Add an option that overrides the from_uid argument to sync_find_reserve_messages() such that when the flag is set, sync_client will try to reserve all the messages in the given mailbox(es), not just the new ones. That should make the remote end check and report if any are missing, which can then be re-uploaded.

This will be particularly useful in recovering from damaged backup files without needing to discard everything following the corruption point.

brong commented 7 years ago

This feels wrong in a bunch of ways. You don't want to be re-reserving every single message in the everyday sync case. We need to be able to fix a broken replica by re-fetching raw messages from another copy as well (see hm/scripts/audit_slot.pl in FastMail for example) - and the pattern here should be that the backup which knows it has corrupted files actively seeks out those missing files.

Potentially we could extend the sync protocol to have a MISSING response to MAILBOXES or USER such that it says "I have indexes to such and such a state, but I'm missing these GUIDs that I should have". I'd like replication to be able to run either way, so reconstruct can connect to a remote sync_server and ask it to APPLY things back. A "pull" version of replication. That plus MISSING support could be used quite nicely to fix missing files. We could add a new system_flag to store the fact that the file is missing.

Anyway, I think we need to do some design work on this to make something that's useful for any sort of repair rather than a hack to work around one single bug.

elliefm commented 7 years ago

You don't want to be re-reserving every single message in the everyday sync case.

I want this to be a command line option to sync_client -- you would only run it with this option if you already knew/suspected that messages had gone missing.

Potentially we could extend the sync protocol to have a MISSING response to MAILBOXES or USER such that it says "I have indexes to such and such a state, but I'm missing these GUIDs that I should have". I'd like replication to be able to run either way, so reconstruct can connect to a remote sync_server and ask it to APPLY things back. A "pull" version of replication. That plus MISSING support could be used quite nicely to fix missing files. We could add a new system_flag to store the fact that the file is missing.

I like that as a longer term direction, but having a slightly crude "fix it now" tool is also good (especially since the proposed change to sync_client won't be invasive)

Anyway, I think we need to do some design work on this to make something that's useful for any sort of repair rather than a hack to work around one single bug.

Yeah agreed. Between this and the sync cache stuff, it'd be good to really nail this down. But I think of that as more "large piece of work for 3.1 (or whatever the next major release is called)".

In the meantime, backups are labelled as experimental in 3.0, and the iterative fixes will be a way of identifying what need or need to avoid later

elliefm commented 7 years ago

https://github.com/cyrusimap/cyrus-imapd/compare/master...elliefm:v30/1818-sync_client-reserve-all-option