diff options
author | Jeff Darcy <jdarcy@redhat.com> | 2014-07-08 21:56:04 -0400 |
---|---|---|
committer | Kaleb KEITHLEY <kkeithle@redhat.com> | 2014-10-20 07:52:48 -0700 |
commit | a7356806f9f7c148d4e0a972f6a418d1ca82bcd0 (patch) | |
tree | 51bd3fa3a8de47d2729df076fa11c310516d6778 /xlators | |
parent | bc75418e1e8eacb1978bdf7f7154d49895ffd544 (diff) |
dht: fix rename race
If two clients try to rename the same file at the same time, we
sometimes end up with *no file at all* in either the old or new
location. That's kind of bad. The culprit seems to be some overly
aggressive cleanup code. AFAICT, based on today's study of the code,
the intent of the changed section is to remove any linkfile we might
have created before the actual rename. However, what we're removing
might not be our extra link. If we're racing with another client that's
also doing a rename, it might be the only remaining link to the user's
data. The solution, which is good enough to pass this test but almost
certainly still not complete, is to be more selective about when we do
this unlink. Now, we only do it if we know that, at some point, we did
in fact create the link without error (notably ENOENT on the source or
EEXIST on the destination) ourselves.
Change-Id: I8d8cce150b6f8b372c9fb813c90be58d69f8eb7b
BUG: 1139988
Signed-off-by: Jeff Darcy <jdarcy@redhat.com>
Reviewed-on: http://review.gluster.org/8269
Tested-by: Gluster Build System <jenkins@build.gluster.com>
Reviewed-by: Vijay Bellur <vbellur@redhat.com>
Reviewed-on: http://review.gluster.org/8672
Reviewed-by: Raghavendra G <rgowdapp@redhat.com>
Reviewed-by: Kaleb KEITHLEY <kkeithle@redhat.com>
Diffstat (limited to 'xlators')
-rw-r--r-- | xlators/cluster/dht/src/dht-common.h | 1 | ||||
-rw-r--r-- | xlators/cluster/dht/src/dht-rename.c | 18 |
2 files changed, 15 insertions, 4 deletions
diff --git a/xlators/cluster/dht/src/dht-common.h b/xlators/cluster/dht/src/dht-common.h index 49fbd3878a0..83725f09712 100644 --- a/xlators/cluster/dht/src/dht-common.h +++ b/xlators/cluster/dht/src/dht-common.h @@ -183,6 +183,7 @@ struct dht_local { struct dht_rebalance_ rebalance; xlator_t *first_up_subvol; + gf_boolean_t added_link; }; typedef struct dht_local dht_local_t; diff --git a/xlators/cluster/dht/src/dht-rename.c b/xlators/cluster/dht/src/dht-rename.c index 35fedeaa7a4..7685ad88427 100644 --- a/xlators/cluster/dht/src/dht-rename.c +++ b/xlators/cluster/dht/src/dht-rename.c @@ -378,8 +378,9 @@ dht_rename_cleanup (call_frame_t *frame) if (dst_hashed != src_hashed && dst_hashed != src_cached) call_cnt++; - if (src_cached != dst_hashed) + if (local->added_link && (src_cached != dst_hashed)) { call_cnt++; + } local->call_cnt = call_cnt; @@ -395,10 +396,11 @@ dht_rename_cleanup (call_frame_t *frame) &local->loc, 0, NULL); } - if (src_cached != dst_hashed) { + if (local->added_link && (src_cached != dst_hashed)) { gf_log (this->name, GF_LOG_TRACE, "unlinking link %s => %s (%s)", local->loc.path, local->loc2.path, src_cached->name); + STACK_WIND (frame, dht_rename_unlink_cbk, src_cached, src_cached->fops->unlink, &local->loc2, 0, NULL); @@ -652,9 +654,15 @@ dht_rename_links_cbk (call_frame_t *frame, void *cookie, xlator_t *this, "link/file on %s failed (%s)", prev->this->name, strerror (op_errno)); local->op_ret = -1; - if (op_errno != ENOENT) + if (op_errno != ENOENT) { local->op_errno = op_errno; - } else { + + if (prev->this == local->src_cached) { + local->added_link = _gf_false; + } + } + } else if (local->src_cached == prev->this) { + /* merge of attr returned only from linkfile creation */ dht_iatt_merge (this, &local->stbuf, stbuf, prev->this); } @@ -767,6 +775,8 @@ dht_rename_create_links (call_frame_t *frame) gf_log (this->name, GF_LOG_TRACE, "link %s => %s (%s)", local->loc.path, local->loc2.path, src_cached->name); + + local->added_link = _gf_true; STACK_WIND (frame, dht_rename_links_cbk, src_cached, src_cached->fops->link, &local->loc, &local->loc2, NULL); |