Mercurial > dovecot > core-2.2

changeset 22715:20415dd0b85a

Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
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.
author Timo Sirainen <timo.sirainen@dovecot.fi>
date Thu, 28 Dec 2017 14:10:23 +0200
parents a34e1e7232f1
children 6287c6d66f56
files 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
diffstat 5 files changed, 88 insertions(+), 5 deletions(-) [+]
line wrap: on
line diff
--- a/src/doveadm/dsync/dsync-brain-mailbox.c	Thu Dec 28 19:40:29 2017 +0200
+++ b/src/doveadm/dsync/dsync-brain-mailbox.c	Thu Dec 28 14:10:23 2017 +0200
@@ -127,6 +127,7 @@
 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 @@
 	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 @@
 	}
 	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 @@
 
 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 @@
 				brain->failed = TRUE;
 			}
 			mailbox_free(&box);
+			file_lock_free(&lock);
 			return ret;
 		}
 
@@ -514,6 +520,7 @@
 					dsync_box.messages_count);
 			}
 			mailbox_free(&box);
+			file_lock_free(&lock);
 			return 0;
 		}
 		if (synced) {
@@ -522,25 +529,33 @@
 		}
 
 		/* 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 @@
 	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 @@
 {
 	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 @@
 
 	/* 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 @@
 	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 @@
 		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 @@
 				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)
--- a/src/doveadm/dsync/dsync-brain-private.h	Thu Dec 28 19:40:29 2017 +0200
+++ b/src/doveadm/dsync/dsync-brain-private.h	Thu Dec 28 14:10:23 2017 +0200
@@ -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_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
--- a/src/doveadm/dsync/dsync-brain.c	Thu Dec 28 19:40:29 2017 +0200
+++ b/src/doveadm/dsync/dsync-brain.c	Thu Dec 28 14:10:23 2017 +0200
@@ -221,6 +221,11 @@
 	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 @@
 
 	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) {
--- a/src/doveadm/dsync/dsync-mailbox.c	Thu Dec 28 19:40:29 2017 +0200
+++ b/src/doveadm/dsync/dsync-mailbox.c	Thu Dec 28 14:10:23 2017 +0200
@@ -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 @@
 	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;
+}
--- a/src/doveadm/dsync/dsync-mailbox.h	Thu Dec 28 19:40:29 2017 +0200
+++ b/src/doveadm/dsync/dsync-mailbox.h	Thu Dec 28 14:10:23 2017 +0200
@@ -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 @@
 				 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