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 <jenkins@build.gluster.com>
Reviewed-by: Amar Tumballi <amar@gluster.com>
Reviewed-by: Vijay Bellur <vijay@gluster.com>

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 0e4e9735503..2e5ca71b219 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 9a17c20309b..2929ad741d8 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 c6d7b5f2219..639b89b1ffd 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 c8dd8b635e6..ba468dcdcb1 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 795fec255d3..8ec634fd4bb 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 92ccf607f10..becbd261f57 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);