]> 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)
committerTimo Sirainen <timo.sirainen@dovecot.fi>
Thu, 28 Dec 2017 18:14:09 +0000 (20:14 +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 80f68a542dc572a4ded96c933cdae83380a9897d..5c1e65e1cc0ada13033fd590f122ee093d817729 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;
@@ -387,6 +389,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;
@@ -464,11 +467,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;
@@ -501,6 +506,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;
                }
 
@@ -519,6 +525,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) {
@@ -527,25 +534,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;
@@ -553,7 +568,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;
 }
@@ -562,11 +577,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;
@@ -574,7 +590,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;
 }
 
@@ -754,6 +770,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;
@@ -796,16 +813,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;
@@ -836,12 +861,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 2abcf8f60f428563d4d983c45b607a628a90ff79..1d407822d98a7329010f20b894b63413708b1c26 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 4fb92490d34656f9a39d70239af377c1c045ddaa..55874545d3af415332d0fc20aca5398a3e1ebf1d 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 =
@@ -498,10 +503,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 18c2b5d0fadcf88f8581d6c073de78b24a5a6770..ffa06071b9be282dcb3ee3e65274a633cc1371b5 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