Duplicate mails on pop3 expunge with dsync replication on 2.2.35 (2.2.33.2 works)

[Gerald Galster]

Hi Tim,

> Do you have any new insights on the problem with reappearing mail using dovecot replication + pop3?
> 
> It's driving me mad. I'm running dovecot 2.2.34 (874deae) on OpenBSD and it looks like I have the same problem as you have.

unfortunately there has been no response, I'm stuck with 2.2.33.2 for the time being.

I can only suspect it has something to do with mailbox locking which was introduced in 2.2.34 (int dsync_mailbox_lock)
or maybe with some cached values (drop_older_timestamp) as it just happens when the mail is deleted on the node that
it has been synced to but not if it's deleted on the node where it has originally been received.

Best regards,
Gerald



diff -Nru dovecot-2.2.33.2/src/doveadm/dsync/dsync-brain-mailbox.c dovecot-2.2.34/src/doveadm/dsync/dsync-brain-mailbox.c
--- dovecot-2.2.33.2/src/doveadm/dsync/dsync-brain-mailbox.c	2017-10-05 19:10:44.000000000 +0200
+++ dovecot-2.2.34/src/doveadm/dsync/dsync-brain-mailbox.c	2018-02-28 15:45:34.000000000 +0100


@@ -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));

...

@@ -599,6 +615,7 @@
 static void
 dsync_cache_fields_update(const struct dsync_mailbox *local_box,
 			  const struct dsync_mailbox *remote_box,
+			  struct mailbox *box,
 			  struct mailbox_update *update)
 {
 	ARRAY_TYPE(mailbox_cache_field) local_sorted, remote_sorted, changes;
@@ -630,7 +647,8 @@
 	local_fields = array_get(&local_sorted, &local_count);
 	remote_fields = array_get(&remote_sorted, &remote_count);
 	t_array_init(&changes, local_count + remote_count);
-	drop_older_timestamp = ioloop_time - MAIL_CACHE_FIELD_DROP_SECS;
+	drop_older_timestamp = ioloop_time -
+		box->index->optimization_set.cache.unaccessed_field_drop_secs;






diff -Nru dovecot-2.2.33.2/src/doveadm/dsync/dsync-mailbox.c dovecot-2.2.34/src/doveadm/dsync/dsync-mailbox.c
--- dovecot-2.2.33.2/src/doveadm/dsync/dsync-mailbox.c	2017-06-23 13:18:28.000000000 +0200
+++ dovecot-2.2.34/src/doveadm/dsync/dsync-mailbox.c	2018-02-28 15:45:34.000000000 +0100
@@ -1,7 +1,9 @@
-/* Copyright (c) 2013-2017 Dovecot authors, see the included COPYING file */
+/* Copyright (c) 2013-2018 Dovecot authors, see the included COPYING file */
 
 #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;
+}