From: Timo Sirainen Date: Thu, 28 Dec 2017 12:10:23 +0000 (+0200) Subject: dsync: Add per-mailbox sync lock that is always used. X-Git-Tag: 2.2.34~174 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=dfca75521f9e13399e04d5a4f7c9c647701d024f;p=thirdparty%2Fdovecot%2Fcore.git dsync: Add per-mailbox sync lock that is always used. Both importing and exporting gets the lock before they even sync the mailbox. The lock is kept until the import/export finishes. This guarantees that no matter how dsync is run, two dsyncs can't be working on the same mailbox at the same time. This lock is in addition to the optional per-user lock enabled by the -l parameter. If the -l parameter is used, the same lock timeout is used for the per-mailbox lock. Otherwise 30s timeout is used. This should help to avoid email duplication when replication is enabled for public namespaces, and maybe in some other rare situations as well. --- diff --git a/src/doveadm/dsync/dsync-brain-mailbox.c b/src/doveadm/dsync/dsync-brain-mailbox.c index 560cb571bd..3e8b49b318 100644 --- a/src/doveadm/dsync/dsync-brain-mailbox.c +++ b/src/doveadm/dsync/dsync-brain-mailbox.c @@ -127,6 +127,7 @@ void dsync_brain_sync_init_box_states(struct dsync_brain *brain) static void dsync_brain_sync_mailbox_init(struct dsync_brain *brain, struct mailbox *box, + struct file_lock *lock, const struct dsync_mailbox *local_dsync_box, bool wait_for_remote_box) { @@ -137,6 +138,7 @@ dsync_brain_sync_mailbox_init(struct dsync_brain *brain, i_assert(box->synced); brain->box = box; + brain->box_lock = lock; brain->pre_box_state = brain->state; if (wait_for_remote_box) { brain->box_send_state = DSYNC_BOX_STATE_MAILBOX; @@ -382,6 +384,7 @@ void dsync_brain_sync_mailbox_deinit(struct dsync_brain *brain) } if (brain->log_scan != NULL) dsync_transaction_log_scan_deinit(&brain->log_scan); + file_lock_free(&brain->box_lock); mailbox_free(&brain->box); brain->state = brain->pre_box_state; @@ -459,11 +462,13 @@ dsync_brain_has_mailbox_state_changed(struct dsync_brain *brain, static int dsync_brain_try_next_mailbox(struct dsync_brain *brain, struct mailbox **box_r, + struct file_lock **lock_r, struct dsync_mailbox *dsync_box_r) { enum mailbox_flags flags = 0; struct dsync_mailbox dsync_box; struct mailbox *box; + struct file_lock *lock = NULL; struct dsync_mailbox_node *node; const char *vname = NULL; enum mail_error error; @@ -496,6 +501,7 @@ dsync_brain_try_next_mailbox(struct dsync_brain *brain, struct mailbox **box_r, brain->failed = TRUE; } mailbox_free(&box); + file_lock_free(&lock); return ret; } @@ -514,6 +520,7 @@ dsync_brain_try_next_mailbox(struct dsync_brain *brain, struct mailbox **box_r, dsync_box.messages_count); } mailbox_free(&box); + file_lock_free(&lock); return 0; } if (synced) { @@ -522,25 +529,33 @@ dsync_brain_try_next_mailbox(struct dsync_brain *brain, struct mailbox **box_r, } /* mailbox appears to have changed. do a full sync here and get the - state again */ + state again. Lock before syncing. */ + if (dsync_mailbox_lock(brain, box, &lock) < 0) { + brain->failed = TRUE; + mailbox_free(&box); + return -1; + } if (mailbox_sync(box, MAILBOX_SYNC_FLAG_FULL_READ) < 0) { i_error("Can't sync mailbox %s: %s", mailbox_get_vname(box), mailbox_get_last_internal_error(box, &brain->mail_error)); brain->failed = TRUE; mailbox_free(&box); + file_lock_free(&lock); return -1; } synced = TRUE; } *box_r = box; + *lock_r = lock; *dsync_box_r = dsync_box; return 1; } static bool dsync_brain_next_mailbox(struct dsync_brain *brain, struct mailbox **box_r, + struct file_lock **lock_r, struct dsync_mailbox *dsync_box_r) { int ret; @@ -548,7 +563,7 @@ dsync_brain_next_mailbox(struct dsync_brain *brain, struct mailbox **box_r, if (brain->no_mail_sync) return FALSE; - while ((ret = dsync_brain_try_next_mailbox(brain, box_r, dsync_box_r)) == 0) + while ((ret = dsync_brain_try_next_mailbox(brain, box_r, lock_r, dsync_box_r)) == 0) ; return ret > 0; } @@ -557,11 +572,12 @@ void dsync_brain_master_send_mailbox(struct dsync_brain *brain) { struct dsync_mailbox dsync_box; struct mailbox *box; + struct file_lock *lock; i_assert(brain->master_brain); i_assert(brain->box == NULL); - if (!dsync_brain_next_mailbox(brain, &box, &dsync_box)) { + if (!dsync_brain_next_mailbox(brain, &box, &lock, &dsync_box)) { brain->state = DSYNC_STATE_FINISH; dsync_ibc_send_end_of_list(brain->ibc, DSYNC_IBC_EOL_MAILBOX); return; @@ -569,7 +585,7 @@ void dsync_brain_master_send_mailbox(struct dsync_brain *brain) /* start exporting this mailbox (wait for remote to start importing) */ dsync_ibc_send_mailbox(brain->ibc, &dsync_box); - dsync_brain_sync_mailbox_init(brain, box, &dsync_box, TRUE); + dsync_brain_sync_mailbox_init(brain, box, lock, &dsync_box, TRUE); brain->state = DSYNC_STATE_SYNC_MAILS; } @@ -749,6 +765,7 @@ bool dsync_brain_slave_recv_mailbox(struct dsync_brain *brain) const struct dsync_mailbox *dsync_box; struct dsync_mailbox local_dsync_box; struct mailbox *box; + struct file_lock *lock; const char *errstr, *resync_reason; enum mail_error error; int ret; @@ -791,16 +808,24 @@ bool dsync_brain_slave_recv_mailbox(struct dsync_brain *brain) dsync_brain_slave_send_mailbox_lost(brain, dsync_box, FALSE); return TRUE; } + /* Lock before syncing */ + if (dsync_mailbox_lock(brain, box, &lock) < 0) { + mailbox_free(&box); + brain->failed = TRUE; + return TRUE; + } if (mailbox_sync(box, MAILBOX_SYNC_FLAG_FULL_READ) < 0) { i_error("Can't sync mailbox %s: %s", mailbox_get_vname(box), mailbox_get_last_internal_error(box, &brain->mail_error)); + file_lock_free(&lock); mailbox_free(&box); brain->failed = TRUE; return TRUE; } if ((ret = dsync_box_get(box, &local_dsync_box, &error)) <= 0) { + file_lock_free(&lock); mailbox_free(&box); if (ret < 0) { brain->mail_error = error; @@ -831,12 +856,13 @@ bool dsync_brain_slave_recv_mailbox(struct dsync_brain *brain) guid_128_to_string(dsync_box->mailbox_guid)); } dsync_ibc_send_mailbox(brain->ibc, &local_dsync_box); + file_lock_free(&lock); mailbox_free(&box); return TRUE; } /* start export/import */ - dsync_brain_sync_mailbox_init(brain, box, &local_dsync_box, FALSE); + dsync_brain_sync_mailbox_init(brain, box, lock, &local_dsync_box, FALSE); if ((ret = dsync_brain_sync_mailbox_open(brain, dsync_box)) < 0) return TRUE; if (resync) diff --git a/src/doveadm/dsync/dsync-brain-private.h b/src/doveadm/dsync/dsync-brain-private.h index 6c2e1817fe..3b6d1b9013 100644 --- a/src/doveadm/dsync/dsync-brain-private.h +++ b/src/doveadm/dsync/dsync-brain-private.h @@ -7,6 +7,8 @@ #include "dsync-mailbox-state.h" #define DSYNC_LOCK_FILENAME ".dovecot-sync.lock" +#define DSYNC_MAILBOX_LOCK_FILENAME ".dovecot-box-sync.lock" +#define DSYNC_MAILBOX_DEFAULT_LOCK_TIMEOUT_SECS 30 struct dsync_mailbox_tree_sync_change; @@ -85,6 +87,8 @@ struct dsync_brain { struct dsync_mailbox_exporter *box_exporter; struct mailbox *box; + struct file_lock *box_lock; + unsigned int mailbox_lock_timeout_secs; struct dsync_mailbox local_dsync_box, remote_dsync_box; pool_t dsync_box_pool; /* list of mailbox states diff --git a/src/doveadm/dsync/dsync-brain.c b/src/doveadm/dsync/dsync-brain.c index 842e658f1b..8e0e7ecab4 100644 --- a/src/doveadm/dsync/dsync-brain.c +++ b/src/doveadm/dsync/dsync-brain.c @@ -221,6 +221,11 @@ dsync_brain_master_init(struct mail_user *user, struct dsync_ibc *ibc, memcpy(brain->sync_box_guid, set->sync_box_guid, sizeof(brain->sync_box_guid)); brain->lock_timeout = set->lock_timeout_secs; + if (brain->lock_timeout != 0) + brain->mailbox_lock_timeout_secs = brain->lock_timeout; + else + brain->mailbox_lock_timeout_secs = + DSYNC_MAILBOX_DEFAULT_LOCK_TIMEOUT_SECS; brain->import_commit_msgs_interval = set->import_commit_msgs_interval; brain->master_brain = TRUE; brain->hashed_headers = @@ -476,10 +481,14 @@ static bool dsync_brain_slave_recv_handshake(struct dsync_brain *brain) if (ibc_set->lock_timeout > 0) { brain->lock_timeout = ibc_set->lock_timeout; + brain->mailbox_lock_timeout_secs = brain->lock_timeout; if (dsync_brain_lock(brain, ibc_set->hostname) < 0) { brain->failed = TRUE; return FALSE; } + } else { + brain->mailbox_lock_timeout_secs = + DSYNC_MAILBOX_DEFAULT_LOCK_TIMEOUT_SECS; } if (ibc_set->sync_ns_prefixes != NULL) { diff --git a/src/doveadm/dsync/dsync-mailbox.c b/src/doveadm/dsync/dsync-mailbox.c index 3b7e6f3d48..d6d06fde39 100644 --- a/src/doveadm/dsync/dsync-mailbox.c +++ b/src/doveadm/dsync/dsync-mailbox.c @@ -2,6 +2,8 @@ #include "lib.h" #include "istream.h" +#include "mail-storage-private.h" +#include "dsync-brain-private.h" #include "dsync-mailbox.h" void dsync_mailbox_attribute_dup(pool_t pool, @@ -20,3 +22,40 @@ void dsync_mailbox_attribute_dup(pool_t pool, dest_r->last_change = src->last_change; dest_r->modseq = src->modseq; } + +int dsync_mailbox_lock(struct dsync_brain *brain, struct mailbox *box, + struct file_lock **lock_r) +{ + const char *path, *error; + int ret; + + /* Make sure the mailbox is open - locking requires it */ + if (mailbox_open(box) < 0) { + i_error("Can't open mailbox %s: %s", mailbox_get_vname(box), + mailbox_get_last_internal_error(box, &brain->mail_error)); + return -1; + } + + ret = mailbox_get_path_to(box, MAILBOX_LIST_PATH_TYPE_INDEX, &path); + if (ret < 0) { + i_error("Can't get mailbox %s path: %s", mailbox_get_vname(box), + mailbox_get_last_internal_error(box, &brain->mail_error)); + return -1; + } + if (ret == 0) { + /* No index files - don't do any locking. In theory we still + could, but this lock is mainly meant to prevent replication + problems, and replication wouldn't work without indexes. */ + *lock_r = NULL; + return 0; + } + + if (mailbox_lock_file_create(box, DSYNC_MAILBOX_LOCK_FILENAME, + brain->mailbox_lock_timeout_secs, + lock_r, &error) <= 0) { + i_error("Failed to lock mailbox %s for dsyncing: %s", + box->vname, error); + return -1; + } + return 0; +} diff --git a/src/doveadm/dsync/dsync-mailbox.h b/src/doveadm/dsync/dsync-mailbox.h index d3dda9ddbb..7e81c0cf76 100644 --- a/src/doveadm/dsync/dsync-mailbox.h +++ b/src/doveadm/dsync/dsync-mailbox.h @@ -3,6 +3,8 @@ #include "mail-storage.h" +struct dsync_brain; + /* Mailbox that is going to be synced. Its name was already sent in the mailbox tree. */ struct dsync_mailbox { @@ -36,4 +38,7 @@ void dsync_mailbox_attribute_dup(pool_t pool, const struct dsync_mailbox_attribute *src, struct dsync_mailbox_attribute *dest_r); +int dsync_mailbox_lock(struct dsync_brain *brain, struct mailbox *box, + struct file_lock **lock_r); + #endif