dovecot-2.0: mdbox: Modified map locking behavior to avoid deadl...
dovecot at dovecot.org
dovecot at dovecot.org
Mon Jun 28 18:47:41 EEST 2010
details: http://hg.dovecot.org/dovecot-2.0/rev/612df9b3df83
changeset: 11647:612df9b3df83
user: Timo Sirainen <tss at iki.fi>
date: Mon Jun 28 16:47:34 2010 +0100
description:
mdbox: Modified map locking behavior to avoid deadlocks when rebuilding storage.
If both mailbox and map index need to be locked, the map index must now be
locked first. Mailbox syncing optimistically tries to first sync without
map locking, but if it sees expunges, it restarts with the map lock.
The map lock is held now slightly longer during sync than before, but it
shouldn't be noticeable.
diffstat:
src/lib-storage/index/dbox-multi/mdbox-map-private.h | 18 +-
src/lib-storage/index/dbox-multi/mdbox-map.c | 235 +++++++++++--------
src/lib-storage/index/dbox-multi/mdbox-map.h | 19 +-
src/lib-storage/index/dbox-multi/mdbox-purge.c | 12 +-
src/lib-storage/index/dbox-multi/mdbox-save.c | 42 ++-
src/lib-storage/index/dbox-multi/mdbox-storage-rebuild.c | 68 +++--
src/lib-storage/index/dbox-multi/mdbox-storage-rebuild.h | 4 +
src/lib-storage/index/dbox-multi/mdbox-storage.c | 6 +-
src/lib-storage/index/dbox-multi/mdbox-sync.c | 155 ++++++------
src/lib-storage/index/dbox-multi/mdbox-sync.h | 2 +
10 files changed, 326 insertions(+), 235 deletions(-)
diffs (truncated from 1088 to 300 lines):
diff -r b2aca3e50f44 -r 612df9b3df83 src/lib-storage/index/dbox-multi/mdbox-map-private.h
--- a/src/lib-storage/index/dbox-multi/mdbox-map-private.h Mon Jun 28 16:31:39 2010 +0100
+++ b/src/lib-storage/index/dbox-multi/mdbox-map-private.h Mon Jun 28 16:47:34 2010 +0100
@@ -32,10 +32,8 @@
struct mdbox_map_append_context {
struct mdbox_map *map;
-
- struct mail_index_sync_ctx *sync_ctx;
- struct mail_index_view *sync_view;
- struct mail_index_transaction *sync_trans, *trans;
+ struct mdbox_map_atomic_context *atomic;
+ struct mail_index_transaction *trans;
ARRAY_DEFINE(file_appends, struct dbox_file_append_context *);
ARRAY_DEFINE(files, struct dbox_file *);
@@ -46,7 +44,17 @@
unsigned int files_nonappendable_count;
unsigned int failed:1;
- unsigned int committed:1;
+};
+
+struct mdbox_map_atomic_context {
+ struct mdbox_map *map;
+ struct mail_index_transaction *sync_trans;
+ struct mail_index_sync_ctx *sync_ctx;
+ struct mail_index_view *sync_view;
+
+ unsigned int map_refreshed:1;
+ unsigned int locked:1;
+ unsigned int success:1;
};
int mdbox_map_view_lookup_rec(struct mdbox_map *map,
diff -r b2aca3e50f44 -r 612df9b3df83 src/lib-storage/index/dbox-multi/mdbox-map.c
--- a/src/lib-storage/index/dbox-multi/mdbox-map.c Mon Jun 28 16:31:39 2010 +0100
+++ b/src/lib-storage/index/dbox-multi/mdbox-map.c Mon Jun 28 16:47:34 2010 +0100
@@ -22,12 +22,11 @@
#define MAP_STORAGE(map) (&(map)->storage->storage.storage)
struct mdbox_map_transaction_context {
- struct mdbox_map *map;
+ struct mdbox_map_atomic_context *atomic;
struct mail_index_transaction *trans;
- struct mail_index_sync_ctx *sync_ctx;
unsigned int changed:1;
- unsigned int success:1;
+ unsigned int committed:1;
};
static int mdbox_map_generate_uid_validity(struct mdbox_map *map);
@@ -408,22 +407,13 @@
return 0;
}
-struct mdbox_map_transaction_context *
-mdbox_map_transaction_begin(struct mdbox_map *map, bool external)
+struct mdbox_map_atomic_context *mdbox_map_atomic_begin(struct mdbox_map *map)
{
- struct mdbox_map_transaction_context *ctx;
- enum mail_index_transaction_flags flags =
- MAIL_INDEX_TRANSACTION_FLAG_FSYNC;
+ struct mdbox_map_atomic_context *atomic;
- if (external)
- flags |= MAIL_INDEX_TRANSACTION_FLAG_EXTERNAL;
-
- ctx = i_new(struct mdbox_map_transaction_context, 1);
- ctx->map = map;
- if (mdbox_map_open(map) > 0 &&
- mdbox_map_refresh(map) == 0)
- ctx->trans = mail_index_transaction_begin(map->view, flags);
- return ctx;
+ atomic = i_new(struct mdbox_map_atomic_context, 1);
+ atomic->map = map;
+ return atomic;
}
static void
@@ -446,57 +436,120 @@
}
}
+int mdbox_map_atomic_lock(struct mdbox_map_atomic_context *atomic)
+{
+ int ret;
+
+ if (atomic->locked)
+ return 0;
+
+ if (mdbox_map_open_or_create(atomic->map) < 0)
+ return -1;
+
+ /* use syncing to lock the transaction log, so that we always see
+ log's head_offset = tail_offset */
+ ret = mail_index_sync_begin(atomic->map->index, &atomic->sync_ctx,
+ &atomic->sync_view, &atomic->sync_trans, 0);
+ if (ret <= 0) {
+ i_assert(ret != 0);
+ mail_storage_set_internal_error(MAP_STORAGE(atomic->map));
+ mail_index_reset_error(atomic->map->index);
+ return -1;
+ }
+ atomic->locked = TRUE;
+ /* reset refresh state so that if it's wanted to be done locked,
+ it gets the latest changes */
+ atomic->map_refreshed = FALSE;
+ mdbox_map_sync_handle(atomic->map, atomic->sync_ctx);
+ return 0;
+}
+
+bool mdbox_map_atomic_is_locked(struct mdbox_map_atomic_context *atomic)
+{
+ return atomic->locked;
+}
+
+void mdbox_map_atomic_set_failed(struct mdbox_map_atomic_context *atomic)
+{
+ atomic->success = FALSE;
+}
+
+int mdbox_map_atomic_finish(struct mdbox_map_atomic_context **_atomic)
+{
+ struct mdbox_map_atomic_context *atomic = *_atomic;
+ int ret = 0;
+
+ *_atomic = NULL;
+
+ if (atomic->success) {
+ if (mail_index_sync_commit(&atomic->sync_ctx) < 0) {
+ mail_storage_set_internal_error(MAP_STORAGE(atomic->map));
+ mail_index_reset_error(atomic->map->index);
+ ret = -1;
+ }
+ } else if (atomic->sync_ctx != NULL) {
+ mail_index_sync_rollback(&atomic->sync_ctx);
+ }
+ i_free(atomic);
+ return ret;
+}
+
+struct mdbox_map_transaction_context *
+mdbox_map_transaction_begin(struct mdbox_map_atomic_context *atomic,
+ bool external)
+{
+ struct mdbox_map_transaction_context *ctx;
+ enum mail_index_transaction_flags flags =
+ MAIL_INDEX_TRANSACTION_FLAG_FSYNC;
+ bool success;
+
+ if (external)
+ flags |= MAIL_INDEX_TRANSACTION_FLAG_EXTERNAL;
+
+ ctx = i_new(struct mdbox_map_transaction_context, 1);
+ ctx->atomic = atomic;
+ if (atomic->locked && atomic->map_refreshed) {
+ /* already refreshed within a lock, don't do it again */
+ success = TRUE;
+ } else {
+ success = mdbox_map_open(atomic->map) > 0 &&
+ mdbox_map_refresh(atomic->map) == 0;
+ }
+
+ if (success) {
+ atomic->map_refreshed = TRUE;
+ ctx->trans = mail_index_transaction_begin(atomic->map->view,
+ flags);
+ }
+ return ctx;
+}
+
int mdbox_map_transaction_commit(struct mdbox_map_transaction_context *ctx)
{
- struct mdbox_map *map = ctx->map;
- struct mail_index_view *view;
- struct mail_index_transaction *sync_trans;
- int ret;
+ i_assert(!ctx->committed);
+ ctx->committed = TRUE;
if (!ctx->changed)
return 0;
- /* use syncing to lock the transaction log, so that we always see
- log's head_offset = tail_offset */
- ret = mail_index_sync_begin(map->index, &ctx->sync_ctx,
- &view, &sync_trans, 0);
- if (ret <= 0) {
- i_assert(ret != 0);
- mail_storage_set_internal_error(MAP_STORAGE(map));
- mail_index_reset_error(map->index);
- mail_index_transaction_rollback(&ctx->trans);
+ if (mdbox_map_atomic_lock(ctx->atomic) < 0)
+ return -1;
+
+ if (mail_index_transaction_commit(&ctx->trans) < 0) {
+ mail_storage_set_internal_error(MAP_STORAGE(ctx->atomic->map));
+ mail_index_reset_error(ctx->atomic->map->index);
return -1;
}
- mdbox_map_sync_handle(map, ctx->sync_ctx);
-
- if (mail_index_transaction_commit(&ctx->trans) < 0) {
- mail_storage_set_internal_error(MAP_STORAGE(map));
- mail_index_reset_error(map->index);
- return -1;
- }
- ctx->success = TRUE;
+ ctx->atomic->success = TRUE;
return 0;
}
-void mdbox_map_transaction_set_failed(struct mdbox_map_transaction_context *ctx)
-{
- ctx->success = FALSE;
-}
-
void mdbox_map_transaction_free(struct mdbox_map_transaction_context **_ctx)
{
struct mdbox_map_transaction_context *ctx = *_ctx;
- struct mdbox_map *map = ctx->map;
*_ctx = NULL;
- if (ctx->success) {
- if (mail_index_sync_commit(&ctx->sync_ctx) < 0) {
- mail_storage_set_internal_error(MAP_STORAGE(map));
- mail_index_reset_error(map->index);
- }
- } else if (ctx->sync_ctx != NULL) {
- mail_index_sync_rollback(&ctx->sync_ctx);
- }
+
if (ctx->trans != NULL)
mail_index_transaction_rollback(&ctx->trans);
i_free(ctx);
@@ -505,7 +558,7 @@
int mdbox_map_update_refcount(struct mdbox_map_transaction_context *ctx,
uint32_t map_uid, int diff)
{
- struct mdbox_map *map = ctx->map;
+ struct mdbox_map *map = ctx->atomic->map;
const void *data;
uint32_t seq;
bool expunged;
@@ -564,6 +617,7 @@
int mdbox_map_remove_file_id(struct mdbox_map *map, uint32_t file_id)
{
+ struct mdbox_map_atomic_context *atomic;
struct mdbox_map_transaction_context *map_trans;
const struct mail_index_header *hdr;
const struct mdbox_map_mail_index_record *rec;
@@ -576,7 +630,8 @@
messages that have already been moved to other files. */
/* we need a per-file transaction, otherwise we can't refresh the map */
- map_trans = mdbox_map_transaction_begin(map, TRUE);
+ atomic = mdbox_map_atomic_begin(map);
+ map_trans = mdbox_map_transaction_begin(atomic, TRUE);
hdr = mail_index_get_header(map->view);
for (seq = 1; seq <= hdr->messages_count; seq++) {
@@ -595,29 +650,35 @@
}
}
if (ret == 0)
- (void)mdbox_map_transaction_commit(map_trans);
+ ret = mdbox_map_transaction_commit(map_trans);
mdbox_map_transaction_free(&map_trans);
+ if (mdbox_map_atomic_finish(&atomic) < 0)
+ ret = -1;
return ret;
}
struct mdbox_map_append_context *
-mdbox_map_append_begin(struct mdbox_map *map)
+mdbox_map_append_begin(struct mdbox_map_atomic_context *atomic)
{
struct mdbox_map_append_context *ctx;
ctx = i_new(struct mdbox_map_append_context, 1);
- ctx->map = map;
+ ctx->atomic = atomic;
+ ctx->map = atomic->map;
ctx->first_new_file_id = (uint32_t)-1;
i_array_init(&ctx->file_appends, 64);
i_array_init(&ctx->files, 64);
i_array_init(&ctx->appends, 128);
- if (mdbox_map_open_or_create(map) < 0)
+ if (mdbox_map_open_or_create(atomic->map) < 0)
ctx->failed = TRUE;
else {
/* refresh the map so we can try appending to the
latest files */
- (void)mdbox_map_refresh(ctx->map);
+ if (mdbox_map_refresh(atomic->map) == 0)
+ atomic->map_refreshed = TRUE;
More information about the dovecot-cvs
mailing list