From 03591027b06c556baa95c6fa4569be0bff4adcd8 Mon Sep 17 00:00:00 2001 From: Pranith Kumar K Date: Tue, 20 Sep 2011 18:30:42 +0530 Subject: cluster/afr: Make local->child_up immutable Afr transaction performs lock, pre-op, op, post-op and unlock steps in that order. The child_up[] is overloaded with the information of where all the first two steps succeeded. This works perfectly fine for Transaction, but the locking/unlocking part of the code is re-used by data self-heal. In that each loop_frame does lock, rchecksum, read-from-source and write-to-sinks, unlock steps. Rchecksum fop assumes that the fop needs to happen on one source + all sinks and sets the call_count to that number. But if the lock step fails on any of the sinks it will mark the child_up of that child to 0, which will result in call_count mismatch and the frame will hang thinking that some more cbks need to come. When this happens loop_frame will never go to unlock step leading to hangs on that file. Change-Id: I3dd0449cc6193a980bacf637d935881f4b22210a BUG: 3597 Reviewed-on: http://review.gluster.com/474 Tested-by: Gluster Build System Reviewed-by: Amar Tumballi Reviewed-by: Vijay Bellur --- xlators/cluster/afr/src/afr-common.c | 16 +++- xlators/cluster/afr/src/afr-dir-write.c | 64 ++++++--------- xlators/cluster/afr/src/afr-inode-write.c | 56 +++++-------- xlators/cluster/afr/src/afr-lk-common.c | 4 - xlators/cluster/afr/src/afr-transaction.c | 130 +++++++++++------------------- xlators/cluster/afr/src/afr.h | 7 +- 6 files changed, 113 insertions(+), 164 deletions(-) diff --git a/xlators/cluster/afr/src/afr-common.c b/xlators/cluster/afr/src/afr-common.c index 0e4e97355..2e5ca71b2 100644 --- a/xlators/cluster/afr/src/afr-common.c +++ b/xlators/cluster/afr/src/afr-common.c @@ -762,6 +762,7 @@ afr_local_transaction_cleanup (afr_local_t *local, xlator_t *this) GF_FREE (local->internal_lock.lower_locked_nodes); + GF_FREE (local->transaction.pre_op); GF_FREE (local->transaction.child_errno); GF_FREE (local->child_errno); GF_FREE (local->transaction.eager_lock); @@ -936,6 +937,13 @@ afr_locked_children_count (unsigned char *children, unsigned int child_count) return afr_set_elem_count_get (children, child_count); } +unsigned int +afr_pre_op_done_children_count (unsigned char *pre_op, + unsigned int child_count) +{ + return afr_set_elem_count_get (pre_op, child_count); +} + gf_boolean_t afr_is_fresh_lookup (loc_t *loc, xlator_t *this) { @@ -3680,11 +3688,17 @@ afr_transaction_local_init (afr_local_t *local, xlator_t *this) if (local->fd) { local->fd_open_on = GF_CALLOC (sizeof (*local->fd_open_on), priv->child_count, - gf_afr_mt_int32_t); + gf_afr_mt_char); if (!local->fd_open_on) goto out; } + local->transaction.pre_op = GF_CALLOC (sizeof (*local->transaction.pre_op), + priv->child_count, + gf_afr_mt_char); + if (!local->transaction.pre_op) + goto out; + for (i = 0; i < priv->child_count; i++) { local->pending[i] = GF_CALLOC (sizeof (*local->pending[i]), 3, /* data + metadata + entry */ diff --git a/xlators/cluster/afr/src/afr-dir-write.c b/xlators/cluster/afr/src/afr-dir-write.c index 9a17c2030..2929ad741 100644 --- a/xlators/cluster/afr/src/afr-dir-write.c +++ b/xlators/cluster/afr/src/afr-dir-write.c @@ -212,16 +212,14 @@ afr_create_wind (call_frame_t *frame, xlator_t *this) { afr_local_t *local = NULL; afr_private_t *priv = NULL; - afr_internal_lock_t *int_lock = NULL; int call_count = -1; int i = 0; local = frame->local; priv = this->private; - int_lock = &local->internal_lock; - call_count = afr_locked_children_count (int_lock->entry_locked_nodes, - priv->child_count); + call_count = afr_pre_op_done_children_count (local->transaction.pre_op, + priv->child_count); if (call_count == 0) { local->transaction.resume (frame, this); @@ -231,7 +229,7 @@ afr_create_wind (call_frame_t *frame, xlator_t *this) local->call_count = call_count; for (i = 0; i < priv->child_count; i++) { - if (local->child_up[i] && int_lock->entry_locked_nodes[i]) { + if (local->transaction.pre_op[i]) { STACK_WIND_COOKIE (frame, afr_create_wind_cbk, (void *) (long) i, priv->children[i], @@ -445,16 +443,14 @@ afr_mknod_wind (call_frame_t *frame, xlator_t *this) { afr_local_t *local = NULL; afr_private_t *priv = NULL; - afr_internal_lock_t *int_lock = NULL; int call_count = -1; int i = 0; local = frame->local; priv = this->private; - int_lock = &local->internal_lock; - call_count = afr_locked_children_count (int_lock->entry_locked_nodes, - priv->child_count); + call_count = afr_pre_op_done_children_count (local->transaction.pre_op, + priv->child_count); if (call_count == 0) { local->transaction.resume (frame, this); @@ -464,7 +460,7 @@ afr_mknod_wind (call_frame_t *frame, xlator_t *this) local->call_count = call_count; for (i = 0; i < priv->child_count; i++) { - if (local->child_up[i] && int_lock->entry_locked_nodes[i]) { + if (local->transaction.pre_op[i]) { STACK_WIND_COOKIE (frame, afr_mknod_wind_cbk, (void *) (long) i, priv->children[i], priv->children[i]->fops->mknod, @@ -673,16 +669,14 @@ afr_mkdir_wind (call_frame_t *frame, xlator_t *this) { afr_local_t *local = NULL; afr_private_t *priv = NULL; - afr_internal_lock_t *int_lock = NULL; int call_count = -1; int i = 0; local = frame->local; priv = this->private; - int_lock = &local->internal_lock; - call_count = afr_locked_children_count (int_lock->entry_locked_nodes, - priv->child_count); + call_count = afr_pre_op_done_children_count (local->transaction.pre_op, + priv->child_count); if (call_count == 0) { local->transaction.resume (frame, this); @@ -692,7 +686,7 @@ afr_mkdir_wind (call_frame_t *frame, xlator_t *this) local->call_count = call_count; for (i = 0; i < priv->child_count; i++) { - if (local->child_up[i] && int_lock->entry_locked_nodes[i]) { + if (local->transaction.pre_op[i]) { STACK_WIND_COOKIE (frame, afr_mkdir_wind_cbk, (void *) (long) i, priv->children[i], @@ -903,16 +897,14 @@ afr_link_wind (call_frame_t *frame, xlator_t *this) { afr_local_t *local = NULL; afr_private_t *priv = NULL; - afr_internal_lock_t *int_lock = NULL; int call_count = -1; int i = 0; local = frame->local; priv = this->private; - int_lock = &local->internal_lock; - call_count = afr_locked_children_count (int_lock->entry_locked_nodes, - priv->child_count); + call_count = afr_pre_op_done_children_count (local->transaction.pre_op, + priv->child_count); if (call_count == 0) { local->transaction.resume (frame, this); @@ -922,7 +914,7 @@ afr_link_wind (call_frame_t *frame, xlator_t *this) local->call_count = call_count; for (i = 0; i < priv->child_count; i++) { - if (local->child_up[i] && int_lock->entry_locked_nodes[i]) { + if (local->transaction.pre_op[i]) { STACK_WIND_COOKIE (frame, afr_link_wind_cbk, (void *) (long) i, priv->children[i], priv->children[i]->fops->link, @@ -1129,16 +1121,14 @@ afr_symlink_wind (call_frame_t *frame, xlator_t *this) { afr_local_t *local = NULL; afr_private_t *priv = NULL; - afr_internal_lock_t *int_lock = NULL; int call_count = -1; int i = 0; local = frame->local; priv = this->private; - int_lock = &local->internal_lock; - call_count = afr_locked_children_count (int_lock->entry_locked_nodes, - priv->child_count); + call_count = afr_pre_op_done_children_count (local->transaction.pre_op, + priv->child_count); if (call_count == 0) { local->transaction.resume (frame, this); @@ -1148,7 +1138,7 @@ afr_symlink_wind (call_frame_t *frame, xlator_t *this) local->call_count = call_count; for (i = 0; i < priv->child_count; i++) { - if (local->child_up[i] && int_lock->entry_locked_nodes[i]) { + if (local->transaction.pre_op[i]) { STACK_WIND_COOKIE (frame, afr_symlink_wind_cbk, (void *) (long) i, priv->children[i], @@ -1353,16 +1343,14 @@ afr_rename_wind (call_frame_t *frame, xlator_t *this) { afr_local_t *local = NULL; afr_private_t *priv = NULL; - afr_internal_lock_t *int_lock = NULL; int call_count = -1; int i = 0; local = frame->local; priv = this->private; - int_lock = &local->internal_lock; - call_count = afr_locked_children_count (int_lock->entry_locked_nodes, - priv->child_count); + call_count = afr_pre_op_done_children_count (local->transaction.pre_op, + priv->child_count); if (call_count == 0) { local->transaction.resume (frame, this); @@ -1372,7 +1360,7 @@ afr_rename_wind (call_frame_t *frame, xlator_t *this) local->call_count = call_count; for (i = 0; i < priv->child_count; i++) { - if (local->child_up[i] && int_lock->entry_locked_nodes[i]) { + if (local->transaction.pre_op[i]) { STACK_WIND_COOKIE (frame, afr_rename_wind_cbk, (void *) (long) i, priv->children[i], @@ -1559,16 +1547,14 @@ afr_unlink_wind (call_frame_t *frame, xlator_t *this) { afr_local_t *local = NULL; afr_private_t *priv = NULL; - afr_internal_lock_t *int_lock = NULL; int call_count = -1; int i = 0; local = frame->local; priv = this->private; - int_lock = &local->internal_lock; - call_count = afr_locked_children_count (int_lock->entry_locked_nodes, - priv->child_count); + call_count = afr_pre_op_done_children_count (local->transaction.pre_op, + priv->child_count); if (call_count == 0) { local->transaction.resume (frame, this); @@ -1578,7 +1564,7 @@ afr_unlink_wind (call_frame_t *frame, xlator_t *this) local->call_count = call_count; for (i = 0; i < priv->child_count; i++) { - if (local->child_up[i] && int_lock->entry_locked_nodes[i]) { + if (local->transaction.pre_op[i]) { STACK_WIND_COOKIE (frame, afr_unlink_wind_cbk, (void *) (long) i, priv->children[i], @@ -1758,16 +1744,14 @@ afr_rmdir_wind (call_frame_t *frame, xlator_t *this) { afr_local_t *local = NULL; afr_private_t *priv = NULL; - afr_internal_lock_t *int_lock = NULL; int call_count = -1; int i = 0; local = frame->local; priv = this->private; - int_lock = &local->internal_lock; - call_count = afr_locked_children_count (int_lock->entry_locked_nodes, - priv->child_count); + call_count = afr_pre_op_done_children_count (local->transaction.pre_op, + priv->child_count); if (call_count == 0) { local->transaction.resume (frame, this); @@ -1777,7 +1761,7 @@ afr_rmdir_wind (call_frame_t *frame, xlator_t *this) local->call_count = call_count; for (i = 0; i < priv->child_count; i++) { - if (local->child_up[i] && int_lock->entry_locked_nodes[i]) { + if (local->transaction.pre_op[i]) { STACK_WIND_COOKIE (frame, afr_rmdir_wind_cbk, (void *) (long) i, priv->children[i], diff --git a/xlators/cluster/afr/src/afr-inode-write.c b/xlators/cluster/afr/src/afr-inode-write.c index c6d7b5f22..639b89b1f 100644 --- a/xlators/cluster/afr/src/afr-inode-write.c +++ b/xlators/cluster/afr/src/afr-inode-write.c @@ -131,16 +131,14 @@ afr_writev_wind (call_frame_t *frame, xlator_t *this) { afr_local_t *local = NULL; afr_private_t *priv = NULL; - afr_internal_lock_t *int_lock = NULL; int i = 0; int call_count = -1; local = frame->local; priv = this->private; - int_lock = &local->internal_lock; - call_count = afr_locked_children_count (int_lock->inode_locked_nodes, - priv->child_count); + call_count = afr_pre_op_done_children_count (local->transaction.pre_op, + priv->child_count); if (call_count == 0) { local->transaction.resume (frame, this); @@ -150,7 +148,7 @@ afr_writev_wind (call_frame_t *frame, xlator_t *this) local->call_count = call_count; for (i = 0; i < priv->child_count; i++) { - if (local->child_up[i] && int_lock->inode_locked_nodes[i]) { + if (local->transaction.pre_op[i]) { STACK_WIND_COOKIE (frame, afr_writev_wind_cbk, (void *) (long) i, priv->children[i], @@ -584,16 +582,14 @@ afr_truncate_wind (call_frame_t *frame, xlator_t *this) { afr_local_t *local = NULL; afr_private_t *priv = NULL; - afr_internal_lock_t *int_lock = NULL; int call_count = -1; int i = 0; local = frame->local; priv = this->private; - int_lock = &local->internal_lock; - call_count = afr_locked_children_count (int_lock->inode_locked_nodes, - priv->child_count); + call_count = afr_pre_op_done_children_count (local->transaction.pre_op, + priv->child_count); if (call_count == 0) { local->transaction.resume (frame, this); @@ -603,7 +599,7 @@ afr_truncate_wind (call_frame_t *frame, xlator_t *this) local->call_count = call_count; for (i = 0; i < priv->child_count; i++) { - if (local->child_up[i] && int_lock->inode_locked_nodes[i]) { + if (local->transaction.pre_op[i]) { STACK_WIND_COOKIE (frame, afr_truncate_wind_cbk, (void *) (long) i, priv->children[i], @@ -794,16 +790,14 @@ afr_ftruncate_wind (call_frame_t *frame, xlator_t *this) { afr_local_t *local = NULL; afr_private_t *priv = NULL; - afr_internal_lock_t *int_lock = NULL; int call_count = -1; int i = 0; local = frame->local; priv = this->private; - int_lock = &local->internal_lock; - call_count = afr_locked_children_count (int_lock->inode_locked_nodes, - priv->child_count); + call_count = afr_pre_op_done_children_count (local->transaction.pre_op, + priv->child_count); if (call_count == 0) { local->transaction.resume (frame, this); @@ -813,7 +807,7 @@ afr_ftruncate_wind (call_frame_t *frame, xlator_t *this) local->call_count = call_count; for (i = 0; i < priv->child_count; i++) { - if (local->child_up[i] && int_lock->inode_locked_nodes[i]) { + if (local->transaction.pre_op[i]) { STACK_WIND_COOKIE (frame, afr_ftruncate_wind_cbk, (void *) (long) i, priv->children[i], @@ -1035,16 +1029,14 @@ afr_setattr_wind (call_frame_t *frame, xlator_t *this) { afr_local_t *local = NULL; afr_private_t *priv = NULL; - afr_internal_lock_t *int_lock = NULL; int call_count = -1; int i = 0; local = frame->local; priv = this->private; - int_lock = &local->internal_lock; - call_count = afr_locked_children_count (int_lock->inode_locked_nodes, - priv->child_count); + call_count = afr_pre_op_done_children_count (local->transaction.pre_op, + priv->child_count); if (call_count == 0) { local->transaction.resume (frame, this); @@ -1054,7 +1046,7 @@ afr_setattr_wind (call_frame_t *frame, xlator_t *this) local->call_count = call_count; for (i = 0; i < priv->child_count; i++) { - if (local->child_up[i] && int_lock->inode_locked_nodes[i]) { + if (local->transaction.pre_op[i]) { STACK_WIND_COOKIE (frame, afr_setattr_wind_cbk, (void *) (long) i, priv->children[i], @@ -1245,16 +1237,14 @@ afr_fsetattr_wind (call_frame_t *frame, xlator_t *this) { afr_local_t *local = NULL; afr_private_t *priv = NULL; - afr_internal_lock_t *int_lock = NULL; int call_count = -1; int i = 0; local = frame->local; priv = this->private; - int_lock = &local->internal_lock; - call_count = afr_locked_children_count (int_lock->inode_locked_nodes, - priv->child_count); + call_count = afr_pre_op_done_children_count (local->transaction.pre_op, + priv->child_count); if (call_count == 0) { local->transaction.resume (frame, this); @@ -1264,7 +1254,7 @@ afr_fsetattr_wind (call_frame_t *frame, xlator_t *this) local->call_count = call_count; for (i = 0; i < priv->child_count; i++) { - if (local->child_up[i] && int_lock->inode_locked_nodes[i]) { + if (local->transaction.pre_op[i]) { STACK_WIND_COOKIE (frame, afr_fsetattr_wind_cbk, (void *) (long) i, priv->children[i], @@ -1439,16 +1429,14 @@ afr_setxattr_wind (call_frame_t *frame, xlator_t *this) { afr_local_t *local = NULL; afr_private_t *priv = NULL; - afr_internal_lock_t *int_lock = NULL; int call_count = -1; int i = 0; local = frame->local; priv = this->private; - int_lock = &local->internal_lock; - call_count = afr_locked_children_count (int_lock->inode_locked_nodes, - priv->child_count); + call_count = afr_pre_op_done_children_count (local->transaction.pre_op, + priv->child_count); if (call_count == 0) { local->transaction.resume (frame, this); @@ -1458,7 +1446,7 @@ afr_setxattr_wind (call_frame_t *frame, xlator_t *this) local->call_count = call_count; for (i = 0; i < priv->child_count; i++) { - if (local->child_up[i] && int_lock->inode_locked_nodes[i]) { + if (local->transaction.pre_op[i]) { STACK_WIND_COOKIE (frame, afr_setxattr_wind_cbk, (void *) (long) i, priv->children[i], @@ -1624,16 +1612,14 @@ afr_removexattr_wind (call_frame_t *frame, xlator_t *this) { afr_local_t *local = NULL; afr_private_t *priv = NULL; - afr_internal_lock_t *int_lock = NULL; int call_count = -1; int i = 0; local = frame->local; priv = this->private; - int_lock = &local->internal_lock; - call_count = afr_locked_children_count (int_lock->inode_locked_nodes, - priv->child_count); + call_count = afr_pre_op_done_children_count (local->transaction.pre_op, + priv->child_count); if (call_count == 0) { local->transaction.resume (frame, this); @@ -1643,7 +1629,7 @@ afr_removexattr_wind (call_frame_t *frame, xlator_t *this) local->call_count = call_count; for (i = 0; i < priv->child_count; i++) { - if (local->child_up[i] && int_lock->inode_locked_nodes[i]) { + if (local->transaction.pre_op[i]) { STACK_WIND_COOKIE (frame, afr_removexattr_wind_cbk, (void *) (long) i, priv->children[i], diff --git a/xlators/cluster/afr/src/afr-lk-common.c b/xlators/cluster/afr/src/afr-lk-common.c index c8dd8b635..ba468dcdc 100644 --- a/xlators/cluster/afr/src/afr-lk-common.c +++ b/xlators/cluster/afr/src/afr-lk-common.c @@ -789,7 +789,6 @@ afr_lock_cbk (call_frame_t *frame, void *cookie, xlator_t *this, int_lock->lock_op_ret = op_ret; } - local->child_up[child_index] = 0; local->op_errno = op_errno; int_lock->lock_op_errno = op_errno; } @@ -852,7 +851,6 @@ afr_lock_lower_cbk (call_frame_t *frame, void *cookie, xlator_t *this, local->op_ret = op_ret; } - local->child_up[child_index] = 0; local->op_errno = op_errno; } } @@ -1180,7 +1178,6 @@ afr_nonblocking_entrylk_cbk (call_frame_t *frame, void *cookie, xlator_t *this, local->op_ret = op_ret; int_lock->lock_op_ret = op_ret; - local->child_up[child_index] = 0; int_lock->lock_op_errno = op_errno; local->op_errno = op_errno; } @@ -1356,7 +1353,6 @@ afr_nonblocking_inodelk_cbk (call_frame_t *frame, void *cookie, xlator_t *this, "please load features/locks xlator on server"); local->op_ret = op_ret; int_lock->lock_op_ret = op_ret; - local->child_up[child_index] = 0; int_lock->lock_op_errno = op_errno; local->op_errno = op_errno; } diff --git a/xlators/cluster/afr/src/afr-transaction.c b/xlators/cluster/afr/src/afr-transaction.c index 795fec255..8ec634fd4 100644 --- a/xlators/cluster/afr/src/afr-transaction.c +++ b/xlators/cluster/afr/src/afr-transaction.c @@ -215,39 +215,7 @@ __changelog_enabled (afr_private_t *priv, afr_transaction_type type) static int -__changelog_needed_pre_op (call_frame_t *frame, xlator_t *this) -{ - afr_private_t * priv = NULL; - afr_local_t * local = NULL; - - int op_ret = 0; - - priv = this->private; - local = frame->local; - - if (__changelog_enabled (priv, local->transaction.type)) { - switch (local->op) { - - case GF_FOP_WRITE: - case GF_FOP_FTRUNCATE: - op_ret = 1; - break; - - case GF_FOP_FLUSH: - op_ret = 0; - break; - - default: - op_ret = 1; - } - } - - return op_ret; -} - - -static int -__changelog_needed_post_op (call_frame_t *frame, xlator_t *this) +__fop_changelog_needed (call_frame_t *frame, xlator_t *this) { afr_private_t * priv = NULL; afr_local_t * local = NULL; @@ -461,33 +429,43 @@ out: return; } -int -afr_fxattrop_call_count (afr_transaction_type type, afr_internal_lock_t *int_lock, - unsigned int child_count) +unsigned char* +afr_locked_nodes_get (afr_transaction_type type, afr_internal_lock_t *int_lock) { - int call_count = 0; - + unsigned char *locked_nodes = NULL; switch (type) { case AFR_DATA_TRANSACTION: case AFR_METADATA_TRANSACTION: - call_count = afr_locked_children_count (int_lock->inode_locked_nodes, - child_count); + locked_nodes = int_lock->inode_locked_nodes; break; case AFR_ENTRY_TRANSACTION: case AFR_ENTRY_RENAME_TRANSACTION: - call_count = afr_locked_children_count (int_lock->entry_locked_nodes, - child_count); + locked_nodes = int_lock->entry_locked_nodes; break; } + return locked_nodes; +} + +int +afr_changelog_pre_op_call_count (afr_transaction_type type, + afr_internal_lock_t *int_lock, + unsigned int child_count) +{ + int call_count = 0; + unsigned char *locked_nodes = NULL; + + locked_nodes = afr_locked_nodes_get (type, int_lock); + GF_ASSERT (locked_nodes); + call_count = afr_locked_children_count (locked_nodes, child_count); if (type == AFR_ENTRY_RENAME_TRANSACTION) { call_count *= 2; } + return call_count; } - int afr_changelog_post_op (call_frame_t *frame, xlator_t *this) { @@ -518,12 +496,11 @@ afr_changelog_post_op (call_frame_t *frame, xlator_t *this) xattr = alloca (priv->child_count * sizeof (*xattr)); memset (xattr, 0, (priv->child_count * sizeof (*xattr))); for (i = 0; i < priv->child_count; i++) { - xattr[i] = get_new_dict (); - dict_ref (xattr[i]); + xattr[i] = dict_new (); } - call_count = afr_fxattrop_call_count (local->transaction.type, int_lock, - priv->child_count); + call_count = afr_pre_op_done_children_count (local->transaction.pre_op, + priv->child_count); local->call_count = call_count; if (local->fd) @@ -531,13 +508,9 @@ afr_changelog_post_op (call_frame_t *frame, xlator_t *this) if (call_count == 0) { /* no child is up */ - for (i = 0; i < priv->child_count; i++) { - dict_unref (xattr[i]); - } - int_lock->lock_cbk = local->transaction.done; afr_unlock (frame, this); - return 0; + goto out; } /* check if something has failed, to handle piggybacking */ @@ -559,11 +532,8 @@ afr_changelog_post_op (call_frame_t *frame, xlator_t *this) } for (i = 0; i < priv->child_count; i++) { - if (!local->child_up[i]) - continue; - if (local->fd && !local->fd_open_on[i]) + if (!local->transaction.pre_op[i]) continue; - ret = afr_set_pending_dict (priv, xattr[i], local->pending); @@ -698,6 +668,7 @@ afr_changelog_post_op (call_frame_t *frame, xlator_t *this) break; } +out: for (i = 0; i < priv->child_count; i++) { dict_unref (xattr[i]); } @@ -719,17 +690,15 @@ afr_changelog_pre_op_cbk (call_frame_t *frame, void *cookie, xlator_t *this, LOCK (&frame->lock); { - if (op_ret == 1) { - /* special op_ret for piggyback */ - } - - if (op_ret == 0) { + switch (op_ret) { + case 0: __mark_pre_op_done_on_fd (frame, this, child_index); - } - - if (op_ret == -1) { - local->child_up[child_index] = 0; - + //fallthrough we need to mark the pre_op + case 1: + local->transaction.pre_op[child_index] = 1; + /* special op_ret for piggyback */ + break; + case -1: if (op_errno == ENOTSUP) { gf_log (this->name, GF_LOG_ERROR, "xattrop not supported by %s", @@ -743,6 +712,7 @@ afr_changelog_pre_op_cbk (call_frame_t *frame, void *cookie, xlator_t *this, strerror (op_errno)); } local->op_errno = op_errno; + break; } call_count = --local->call_count; @@ -778,6 +748,7 @@ afr_changelog_pre_op (call_frame_t *frame, xlator_t *this) afr_local_t *local = NULL; int piggyback = 0; afr_internal_lock_t *int_lock = NULL; + unsigned char *locked_nodes = NULL; local = frame->local; int_lock = &local->internal_lock; @@ -786,22 +757,17 @@ afr_changelog_pre_op (call_frame_t *frame, xlator_t *this) memset (xattr, 0, (priv->child_count * sizeof (*xattr))); for (i = 0; i < priv->child_count; i++) { - xattr[i] = get_new_dict (); - dict_ref (xattr[i]); + xattr[i] = dict_new (); } - call_count = afr_fxattrop_call_count (local->transaction.type, int_lock, - priv->child_count); + call_count = afr_changelog_pre_op_call_count (local->transaction.type, + int_lock, + priv->child_count); if (call_count == 0) { - /* no child is up */ - for (i = 0; i < priv->child_count; i++) { - dict_unref (xattr[i]); - } - local->internal_lock.lock_cbk = local->transaction.done; afr_unlock (frame, this); - return 0; + goto out; } local->call_count = call_count; @@ -812,12 +778,10 @@ afr_changelog_pre_op (call_frame_t *frame, xlator_t *this) if (local->fd) fdctx = afr_fd_ctx_get (local->fd, this); + locked_nodes = afr_locked_nodes_get (local->transaction.type, int_lock); for (i = 0; i < priv->child_count; i++) { - if (!local->child_up[i]) - continue; - if (local->fd && !local->fd_open_on[i]) + if (!locked_nodes[i]) continue; - ret = afr_set_pending_dict (priv, xattr[i], local->pending); @@ -960,7 +924,7 @@ afr_changelog_pre_op (call_frame_t *frame, xlator_t *this) if (!--call_count) break; } - +out: for (i = 0; i < priv->child_count; i++) { dict_unref (xattr[i]); } @@ -1193,7 +1157,7 @@ afr_internal_lock_finish (call_frame_t *frame, xlator_t *this) priv = this->private; local = frame->local; - if (__changelog_needed_pre_op (frame, this)) { + if (__fop_changelog_needed (frame, this)) { afr_changelog_pre_op (frame, this); } else { __mark_all_success (local->pending, priv->child_count, @@ -1219,7 +1183,7 @@ afr_transaction_resume (call_frame_t *frame, xlator_t *this) int_lock = &local->internal_lock; priv = this->private; - if (__changelog_needed_post_op (frame, this)) { + if (__fop_changelog_needed (frame, this)) { afr_changelog_post_op (frame, this); } else { if (afr_lock_server_count (priv, local->transaction.type) == 0) { diff --git a/xlators/cluster/afr/src/afr.h b/xlators/cluster/afr/src/afr.h index 92ccf607f..becbd261f 100644 --- a/xlators/cluster/afr/src/afr.h +++ b/xlators/cluster/afr/src/afr.h @@ -377,7 +377,7 @@ typedef struct _afr_local { loc_t newloc; fd_t *fd; - int32_t *fd_open_on; + unsigned char *fd_open_on; glusterfs_fop_t fop; @@ -682,6 +682,7 @@ typedef struct _afr_local { int last_tried; int32_t *child_errno; + unsigned char *pre_op; call_frame_t *main_frame; @@ -826,6 +827,10 @@ afr_up_children_count (unsigned char *child_up, unsigned int child_count); unsigned int afr_locked_children_count (unsigned char *children, unsigned int child_count); +unsigned int +afr_pre_op_done_children_count (unsigned char *pre_op, + unsigned int child_count); + gf_boolean_t afr_is_fresh_lookup (loc_t *loc, xlator_t *this); -- cgit