Mercurial > dovecot > original-hg > dovecot-1.2

changeset 8527:6d07bedcdb80 HEAD

Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
mbox: Added a new index header where dirtyness state is stored. This also fixes a bug where sync_size wasn't updated in header when mbox was marked dirty, causing other parts of code to assume wrong file size and return wrong size for the last message and/or cause "Next message unexpectedly lost" errors.
author Timo Sirainen <tss@iki.fi>
date Sat, 13 Dec 2008 09:38:52 +0200
parents c63cc3580150
children 1a9e14718bd6
files src/lib-storage/index/mbox/mbox-file.c src/lib-storage/index/mbox/mbox-mail.c src/lib-storage/index/mbox/mbox-save.c src/lib-storage/index/mbox/mbox-storage.c src/lib-storage/index/mbox/mbox-storage.h src/lib-storage/index/mbox/mbox-sync-private.h src/lib-storage/index/mbox/mbox-sync.c src/util/idxview.c
diffstat 8 files changed, 134 insertions(+), 89 deletions(-) [+]
line wrap: on
line diff
--- a/src/lib-storage/index/mbox/mbox-file.c	Sat Dec 13 08:36:59 2008 +0200
+++ b/src/lib-storage/index/mbox/mbox-file.c	Sat Dec 13 09:38:52 2008 +0200
@@ -149,7 +149,8 @@
 		mail_storage_set_critical(&mbox->storage->storage,
 			"Cached message offset lost for seq %u in mbox file %s",
 			seq, mbox->path);
-                mbox->mbox_sync_dirty = TRUE;
+                mbox->mbox_hdr.dirty_flag = TRUE;
+                mbox->mbox_broken_offsets = TRUE;
 		return 0;
 	}
 
@@ -179,17 +180,18 @@
 			return -1;
 		}
 
-		if (mbox->mbox_sync_dirty)
+		if (mbox->mbox_hdr.dirty_flag)
 			return 0;
 
 		mail_storage_set_critical(&mbox->storage->storage,
 			"Cached message offset %s is invalid for mbox file %s",
 			dec2str(offset), mbox->path);
-		mbox->mbox_sync_dirty = TRUE;
+		mbox->mbox_hdr.dirty_flag = TRUE;
+		mbox->mbox_broken_offsets = TRUE;
 		return 0;
 	}
 
-	if (mbox->mbox_sync_dirty) {
+	if (mbox->mbox_hdr.dirty_flag) {
 		/* we're dirty - make sure this is the correct mail */
 		if (!mbox_sync_parse_match_mail(mbox, view, seq))
 			return 0;
--- a/src/lib-storage/index/mbox/mbox-mail.c	Sat Dec 13 08:36:59 2008 +0200
+++ b/src/lib-storage/index/mbox/mbox-mail.c	Sat Dec 13 09:38:52 2008 +0200
@@ -189,10 +189,11 @@
 	}
 
 	/* We can't really trust trans_view. The next message may already be
-	   expugned from it. hdr.sync_size may also be updated, but
-	   hdr.messages_count not. So refresh the index to get the latest
-	   changes and get the next message's offset using a new view. */
-	(void)mail_index_refresh(mail->ibox->index);
+	   expunged from it. Also there hdr.messages_count may be incorrect.
+	   So refresh the index to get the latest changes and get the next
+	   message's offset using a new view. */
+	if (mbox_sync_header_refresh(mbox) < 0)
+		return -1;
 
 	view = mail_index_view_open(mail->ibox->index);
 	hdr = mail_index_get_header(view);
@@ -203,7 +204,7 @@
 		/* last message, use the synced mbox size */
 		trailer_size = (mbox->storage->storage.flags &
 				MAIL_STORAGE_FLAG_SAVE_CRLF) != 0 ? 2 : 1;
-		*next_offset_r = hdr->sync_size - trailer_size;
+		*next_offset_r = mbox->mbox_hdr.sync_size - trailer_size;
 	} else {
 		if (mbox_file_lookup_offset(mbox, view, seq + 1,
 					    next_offset_r) <= 0)
--- a/src/lib-storage/index/mbox/mbox-save.c	Sat Dec 13 08:36:59 2008 +0200
+++ b/src/lib-storage/index/mbox/mbox-save.c	Sat Dec 13 09:38:52 2008 +0200
@@ -718,41 +718,34 @@
 {
 	struct mbox_transaction_context *t =
 		(struct mbox_transaction_context *)ctx->ctx.transaction;
+	struct mbox_mailbox *mbox = ctx->mbox;
 	struct stat st;
 	int ret = 0;
 
 	i_assert(ctx->finished);
 
-	if (fstat(ctx->mbox->mbox_fd, &st) < 0) {
-		mbox_set_syscall_error(ctx->mbox, "fstat()");
+	if (fstat(mbox->mbox_fd, &st) < 0) {
+		mbox_set_syscall_error(mbox, "fstat()");
 		ret = -1;
 	}
 
 	if (ctx->synced) {
 		*t->ictx.saved_uid_validity = ctx->uid_validity;
 		*t->ictx.first_saved_uid = ctx->first_saved_uid;
+		*t->ictx.last_saved_uid = ctx->next_uid - 1;
 
 		mail_index_update_header(ctx->trans,
 			offsetof(struct mail_index_header, next_uid),
 			&ctx->next_uid, sizeof(ctx->next_uid), FALSE);
 
-		if (!ctx->mbox->mbox_sync_dirty && ret == 0) {
-			uint32_t sync_stamp = st.st_mtime;
-
-			mail_index_update_header(ctx->trans,
-				offsetof(struct mail_index_header, sync_stamp),
-				&sync_stamp, sizeof(sync_stamp), TRUE);
+		if (ret == 0) {
+			mbox->mbox_hdr.sync_mtime = st.st_mtime;
+			mbox->mbox_hdr.sync_size = st.st_size;
+			mail_index_update_header_ext(ctx->trans,
+						     mbox->mbox_ext_idx,
+						     0, &mbox->mbox_hdr,
+						     sizeof(mbox->mbox_hdr));
 		}
-		if (ret == 0) {
-			/* sync_size is used in calculating the last message's
-			   size. it must be up-to-date. */
-			uint64_t sync_size = st.st_size;
-
-			mail_index_update_header(ctx->trans,
-				offsetof(struct mail_index_header, sync_size),
-				&sync_size, sizeof(sync_size), TRUE);
-		}
-		*t->ictx.last_saved_uid = ctx->next_uid - 1;
 	}
 
 	if (ret == 0 && ctx->orig_atime != st.st_atime) {
@@ -761,14 +754,14 @@
 
 		buf.modtime = st.st_mtime;
 		buf.actime = ctx->orig_atime;
-		if (utime(ctx->mbox->path, &buf) < 0)
-			mbox_set_syscall_error(ctx->mbox, "utime()");
+		if (utime(mbox->path, &buf) < 0)
+			mbox_set_syscall_error(mbox, "utime()");
 	}
 
-	if (!ctx->synced && ctx->mbox->mbox_fd != -1 &&
-	    !ctx->mbox->mbox_writeonly && !ctx->mbox->ibox.fsync_disable) {
-		if (fdatasync(ctx->mbox->mbox_fd) < 0) {
-			mbox_set_syscall_error(ctx->mbox, "fdatasync()");
+	if (!ctx->synced && mbox->mbox_fd != -1 &&
+	    !mbox->mbox_writeonly && !mbox->ibox.fsync_disable) {
+		if (fdatasync(mbox->mbox_fd) < 0) {
+			mbox_set_syscall_error(mbox, "fdatasync()");
 			ret = -1;
 		}
 	}
--- a/src/lib-storage/index/mbox/mbox-storage.c	Sat Dec 13 08:36:59 2008 +0200
+++ b/src/lib-storage/index/mbox/mbox-storage.c	Sat Dec 13 09:38:52 2008 +0200
@@ -565,7 +565,8 @@
 	mbox->mbox_fd = -1;
 	mbox->mbox_lock_type = F_UNLCK;
 	mbox->mbox_ext_idx =
-		mail_index_ext_register(index, "mbox", 0,
+		mail_index_ext_register(index, "mbox",
+					sizeof(mbox->mbox_hdr),
 					sizeof(uint64_t), sizeof(uint64_t));
 
         mbox->mbox_very_dirty_syncs = getenv("MBOX_VERY_DIRTY_SYNCS") != NULL;
--- a/src/lib-storage/index/mbox/mbox-storage.h	Sat Dec 13 08:36:59 2008 +0200
+++ b/src/lib-storage/index/mbox/mbox-storage.h	Sat Dec 13 09:38:52 2008 +0200
@@ -14,6 +14,12 @@
 #include "index-storage.h"
 #include "mailbox-list-private.h"
 
+struct mbox_index_header {
+	uint64_t sync_size;
+	uint32_t sync_mtime;
+	uint8_t dirty_flag;
+	uint8_t unused[3];
+};
 struct mbox_storage {
 	struct mail_storage storage;
 
@@ -36,14 +42,13 @@
 	unsigned int mbox_lock_id, mbox_global_lock_id;
 	struct timeout *keep_lock_to;
 	bool mbox_readonly, mbox_writeonly;
-	time_t mbox_dirty_stamp;
-	off_t mbox_dirty_size;
 
 	uint32_t mbox_ext_idx;
+	struct mbox_index_header mbox_hdr;
 
 	unsigned int no_mbox_file:1;
 	unsigned int invalid_mbox_file:1;
-	unsigned int mbox_sync_dirty:1;
+	unsigned int mbox_broken_offsets:1;
 	unsigned int mbox_do_dirty_syncs:1;
 	unsigned int mbox_very_dirty_syncs:1;
 	unsigned int mbox_save_md5:1;
--- a/src/lib-storage/index/mbox/mbox-sync-private.h	Sat Dec 13 08:36:59 2008 +0200
+++ b/src/lib-storage/index/mbox/mbox-sync-private.h	Sat Dec 13 09:38:52 2008 +0200
@@ -146,6 +146,7 @@
 	unsigned int errors:1;
 };
 
+int mbox_sync_header_refresh(struct mbox_mailbox *mbox);
 int mbox_sync(struct mbox_mailbox *mbox, enum mbox_sync_flags flags);
 int mbox_sync_has_changed(struct mbox_mailbox *mbox, bool leave_dirty);
 int mbox_sync_has_changed_full(struct mbox_mailbox *mbox, bool leave_dirty,
--- a/src/lib-storage/index/mbox/mbox-sync.c	Sat Dec 13 08:36:59 2008 +0200
+++ b/src/lib-storage/index/mbox/mbox-sync.c	Sat Dec 13 09:38:52 2008 +0200
@@ -935,7 +935,7 @@
 	} else {
 		/* if there's no sync records left, we can stop. except if
 		   this is a dirty sync, check if there are new messages. */
-		if (!sync_ctx->mbox->mbox_sync_dirty)
+		if (!sync_ctx->mbox->mbox_hdr.dirty_flag)
 			return 0;
 
 		messages_count =
@@ -1006,7 +1006,7 @@
 
 		if (mail_ctx->seq == 1) {
 			if (mbox_sync_uidvalidity_changed(sync_ctx)) {
-				sync_ctx->mbox->mbox_sync_dirty = TRUE;
+				sync_ctx->mbox->mbox_hdr.dirty_flag = TRUE;
 				return 0;
 			}
 		}
@@ -1014,14 +1014,14 @@
 		if (mail_ctx->mail.uid_broken && partial) {
 			/* UID ordering problems, resync everything to make
 			   sure we get everything right */
-			if (sync_ctx->mbox->mbox_sync_dirty)
+			if (sync_ctx->mbox->mbox_hdr.dirty_flag)
 				return 0;
 
 			mbox_sync_set_critical(sync_ctx,
 				"UIDs broken with partial sync in mbox file %s",
 				sync_ctx->mbox->path);
 
-			sync_ctx->mbox->mbox_sync_dirty = TRUE;
+			sync_ctx->mbox->mbox_hdr.dirty_flag = TRUE;
 			return 0;
 		}
 		if (mail_ctx->mail.uid_broken)
@@ -1159,13 +1159,14 @@
 	}
 
 	if (!skipped_mails)
-		sync_ctx->mbox->mbox_sync_dirty = FALSE;
+		sync_ctx->mbox->mbox_hdr.dirty_flag = FALSE;
+	sync_ctx->mbox->mbox_broken_offsets = FALSE;
 
 	if (uids_broken && sync_ctx->delay_writes) {
 		/* once we get around to writing the changes, we'll need to do
 		   a full sync to avoid the "UIDs broken in partial sync"
 		   error */
-		sync_ctx->mbox->mbox_sync_dirty = TRUE;
+		sync_ctx->mbox->mbox_hdr.dirty_flag = TRUE;
 	}
 	return 1;
 }
@@ -1328,6 +1329,23 @@
 	return 0;
 }
 
+static void
+mbox_sync_index_update_ext_header(struct mbox_sync_context *sync_ctx)
+{
+	struct mbox_mailbox *mbox = sync_ctx->mbox;
+	const void *data;
+	size_t data_size;
+
+	mail_index_get_header_ext(mbox->ibox.view, mbox->mbox_ext_idx,
+				  &data, &data_size);
+	if (data_size != sizeof(mbox->mbox_hdr) ||
+	    memcmp(data, &mbox->mbox_hdr, data_size) != 0) {
+		mail_index_update_header_ext(sync_ctx->t, mbox->mbox_ext_idx,
+					     0, &mbox->mbox_hdr,
+					     sizeof(mbox->mbox_hdr));
+	}
+}
+
 static int mbox_sync_update_index_header(struct mbox_sync_context *sync_ctx)
 {
 	struct mail_index_view *view;
@@ -1341,7 +1359,7 @@
 	}
 
 	if (sync_ctx->moved_offsets &&
-	    ((uint64_t)st->st_size == sync_ctx->hdr->sync_size ||
+	    ((uint64_t)st->st_size == sync_ctx->mbox->mbox_hdr.sync_size ||
 	     (uint64_t)st->st_size == sync_ctx->orig_size)) {
 		/* We moved messages inside the mbox file without changing
 		   the file's size. If mtime doesn't change, another process
@@ -1372,6 +1390,10 @@
 		}
 	}
 
+	sync_ctx->mbox->mbox_hdr.sync_mtime = st->st_mtime;
+	sync_ctx->mbox->mbox_hdr.sync_size = st->st_size;
+	mbox_sync_index_update_ext_header(sync_ctx);
+
 	/* only reason not to have UID validity at this point is if the file
 	   is entirely empty. In that case just make up a new one if needed. */
 	i_assert(sync_ctx->base_uid_validity != 0 || st->st_size <= 0);
@@ -1396,24 +1418,6 @@
 			&sync_ctx->next_uid, sizeof(sync_ctx->next_uid), FALSE);
 	}
 
-	if ((uint32_t)st->st_mtime != sync_ctx->hdr->sync_stamp &&
-	    !sync_ctx->mbox->mbox_sync_dirty) {
-		uint32_t sync_stamp = st->st_mtime;
-
-		mail_index_update_header(sync_ctx->t,
-			offsetof(struct mail_index_header, sync_stamp),
-			&sync_stamp, sizeof(sync_stamp), TRUE);
-	}
-
-	if ((uint64_t)st->st_size != sync_ctx->hdr->sync_size &&
-	    !sync_ctx->mbox->mbox_sync_dirty) {
-		uint64_t sync_size = st->st_size;
-
-		mail_index_update_header(sync_ctx->t,
-			offsetof(struct mail_index_header, sync_size),
-			&sync_size, sizeof(sync_size), TRUE);
-	}
-
 	if (sync_ctx->last_nonrecent_uid < sync_ctx->hdr->first_recent_uid) {
 		/* other sessions have already marked more messages as
 		   recent. */
@@ -1437,10 +1441,6 @@
 			offsetof(struct mail_index_header, first_recent_uid),
 			&first_recent_uid, sizeof(first_recent_uid), FALSE);
 	}
-
-	sync_ctx->mbox->mbox_dirty_stamp = st->st_mtime;
-	sync_ctx->mbox->mbox_dirty_size = st->st_size;
-
 	return 0;
 }
 
@@ -1481,6 +1481,7 @@
 static int mbox_sync_do(struct mbox_sync_context *sync_ctx,
 			enum mbox_sync_flags flags)
 {
+	struct mbox_index_header *mbox_hdr = &sync_ctx->mbox->mbox_hdr;
 	struct mbox_sync_mail_context mail_ctx;
 	const struct stat *st;
 	unsigned int i;
@@ -1499,26 +1500,28 @@
 	if ((flags & MBOX_SYNC_FORCE_SYNC) != 0) {
 		/* forcing a full sync. assume file has changed. */
 		partial = FALSE;
-		sync_ctx->mbox->mbox_sync_dirty = TRUE;
-	} else if ((uint32_t)st->st_mtime == sync_ctx->hdr->sync_stamp &&
-		   (uint64_t)st->st_size == sync_ctx->hdr->sync_size) {
+		mbox_hdr->dirty_flag = TRUE;
+	} else if ((uint32_t)st->st_mtime == mbox_hdr->sync_mtime &&
+		   (uint64_t)st->st_size == mbox_hdr->sync_size) {
 		/* file is fully synced */
-		partial = TRUE;
-		sync_ctx->mbox->mbox_sync_dirty = FALSE;
+		if (mbox_hdr->dirty_flag && (flags & MBOX_SYNC_UNDIRTY) != 0)
+			partial = FALSE;
+		else
+			partial = TRUE;
 	} else if ((flags & MBOX_SYNC_UNDIRTY) != 0 ||
-		   (uint64_t)st->st_size == sync_ctx->hdr->sync_size) {
+		   (uint64_t)st->st_size == mbox_hdr->sync_size) {
 		/* we want to do full syncing. always do this if
 		   file size hasn't changed but timestamp has. it most
 		   likely means that someone had modified some header
 		   and we probably want to know about it */
 		partial = FALSE;
-		sync_ctx->mbox->mbox_sync_dirty = TRUE;
+		sync_ctx->mbox->mbox_hdr.dirty_flag = TRUE;
 	} else {
 		/* see if we can delay syncing the whole file.
 		   normally we only notice expunges and appends
 		   in partial syncing. */
 		partial = TRUE;
-		sync_ctx->mbox->mbox_sync_dirty = TRUE;
+		sync_ctx->mbox->mbox_hdr.dirty_flag = TRUE;
 	}
 
 	mbox_sync_restart(sync_ctx);
@@ -1567,6 +1570,33 @@
 	return 0;
 }
 
+int mbox_sync_header_refresh(struct mbox_mailbox *mbox)
+{
+	const struct mail_index_header *hdr;
+	const void *data;
+	size_t data_size;
+
+	if (mail_index_refresh(mbox->ibox.index) < 0) {
+		mail_storage_set_index_error(&mbox->ibox);
+		return -1;
+	}
+
+	mail_index_get_header_ext(mbox->ibox.view, mbox->mbox_ext_idx,
+				  &data, &data_size);
+	if (data_size == 0) {
+		/* doesn't exist. FIXME: backwards compatibility copying */
+		hdr = mail_index_get_header(mbox->ibox.view);
+		mbox->mbox_hdr.sync_mtime = hdr->sync_stamp;
+		mbox->mbox_hdr.sync_size = hdr->sync_size;
+		return 0;
+	}
+
+	memcpy(&mbox->mbox_hdr, data, I_MIN(sizeof(mbox->mbox_hdr), data_size));
+	if (mbox->mbox_broken_offsets)
+		mbox->mbox_hdr.dirty_flag = TRUE;
+	return 0;
+}
+
 int mbox_sync_has_changed(struct mbox_mailbox *mbox, bool leave_dirty)
 {
 	bool empty;
@@ -1577,7 +1607,6 @@
 int mbox_sync_has_changed_full(struct mbox_mailbox *mbox, bool leave_dirty,
 			       bool *empty_r)
 {
-	const struct mail_index_header *hdr;
 	const struct stat *st;
 	struct stat statbuf;
 
@@ -1605,22 +1634,19 @@
 	}
 	*empty_r = st->st_size == 0;
 
-	hdr = mail_index_get_header(mbox->ibox.view);
+	if (mbox_sync_header_refresh(mbox) < 0)
+		return -1;
 
-	if ((uint32_t)st->st_mtime == hdr->sync_stamp &&
-	    (uint64_t)st->st_size == hdr->sync_size) {
+	if ((uint32_t)st->st_mtime == mbox->mbox_hdr.sync_mtime &&
+	    (uint64_t)st->st_size == mbox->mbox_hdr.sync_size) {
 		/* fully synced */
-		mbox->mbox_sync_dirty = FALSE;
-		return 0;
+		if (!mbox->mbox_hdr.dirty_flag || leave_dirty)
+			return 0;
+		/* flushing dirtyness */
 	}
 
-	if (!mbox->mbox_sync_dirty || !leave_dirty) {
-		mbox->mbox_sync_dirty = TRUE;
-		return 1;
-	}
-
-	return st->st_mtime != mbox->mbox_dirty_stamp ||
-		st->st_size != mbox->mbox_dirty_size;
+	/* file changed */
+	return 1;
 }
 
 static void mbox_sync_context_free(struct mbox_sync_context *sync_ctx)
@@ -1659,9 +1685,11 @@
 	}
 
 	if ((flags & MBOX_SYNC_HEADER) != 0 ||
-	    (flags & MBOX_SYNC_FORCE_SYNC) != 0)
+	    (flags & MBOX_SYNC_FORCE_SYNC) != 0) {
+		if (mbox_sync_header_refresh(mbox) < 0)
+			return -1;
 		changed = 1;
-	else {
+	} else {
 		bool leave_dirty = (flags & MBOX_SYNC_UNDIRTY) == 0;
 		if ((changed = mbox_sync_has_changed(mbox, leave_dirty)) < 0) {
 			if ((flags & MBOX_SYNC_LOCK_READING) != 0)
--- a/src/util/idxview.c	Sat Dec 13 08:36:59 2008 +0200
+++ b/src/util/idxview.c	Sat Dec 13 09:38:52 2008 +0200
@@ -19,6 +19,12 @@
 	uint32_t cur_check_time, cur_mtime, cur_mtime_nsecs;
 	uint32_t uidlist_mtime, uidlist_mtime_nsecs, uidlist_size;
 };
+struct mbox_index_header {
+	uint64_t sync_size;
+	uint32_t sync_mtime;
+	uint8_t dirty_flag;
+	uint8_t unused[3];
+};
 struct dbox_index_header {
 	uint32_t last_dirty_flush_stamp;
 };
@@ -95,6 +101,14 @@
 		printf(" - uidlist_mtime ..... = %s\n", unixdate2str(hdr->uidlist_mtime));
 		printf(" - uidlist_mtime_nsecs = %u\n", hdr->uidlist_mtime_nsecs);
 		printf(" - uidlist_size ...... = %u\n", hdr->uidlist_size);
+	} else if (strcmp(ext->name, "mbox") == 0) {
+		const struct mbox_index_header *hdr = data;
+
+		printf("header\n");
+		printf(" - sync_mtime = %s\n", unixdate2str(hdr->sync_mtime));
+		printf(" - sync_size = %llu\n",
+		       (unsigned long long)hdr->sync_size);
+		printf(" - dirty_flag = %d\n", hdr->dirty_flag);
 	} else if (strcmp(ext->name, "dbox-hdr") == 0) {
 		const struct dbox_index_header *hdr = data;