]> git.ipfire.org Git - thirdparty/dovecot/core.git/commitdiff
dsync: Add per-mailbox sync lock that is always used.
authorTimo Sirainen <timo.sirainen@dovecot.fi>
Thu, 28 Dec 2017 12:10:23 +0000 (14:10 +0200)
committerVille Savolainen <ville.savolainen@dovecot.fi>
Wed, 3 Jan 2018 10:40:28 +0000 (12:40 +0200)
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.

src/doveadm/dsync/dsync-brain-mailbox.c
src/doveadm/dsync/dsync-brain-private.h
src/doveadm/dsync/dsync-brain.c
src/doveadm/dsync/dsync-mailbox.c
src/doveadm/dsync/dsync-mailbox.h

index 560cb571bd9466970aec0f3f936f54b6ff1c7901..3e8b49b318ae823cd4bc18367e86f403b0ab8822 100644 (file)
@@ -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)
index 6c2e1817fe9bfced0da10db6b0f1367d83b5dd7c..3b6d1b9013d55f659a75bd8261a4b608f06befe8 100644 (file)
@@ -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
index 842e658f1b3fb973a223330f58651c90b017e763..8e0e7ecab4918c172d269c675a09815d6e79a340 100644 (file)
@@ -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) {
index 3b7e6f3d4895d58c34ad6e2ad81dd3a3ab81e018..d6d06fde39cbe33abff40ab8dd491d26ee10c668 100644 (file)
@@ -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;
+}
index d3dda9ddbb4885c5541bd9995552f96cc2d1e60c..7e81c0cf769ab7ccc1b00c93978e8ff8f9bf978e 100644 (file)
@@ -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