cluster/afr: Fix per-txn optimistic changelog initialisation

Incorrect initialisation of local->optimistic_change_log was leading
to skipped pre-op and post-op even when a brick didn't participate in
the txn because it was down.
The result - missing granular name index resulting in some entries
never getting healed.

FIX:
Initialise local->optimistic_change_log just before pre-op.

Also fixed granular entry heal to create the granular name index in
pre-op as opposed to post-op. This is to prevent loss of granular
information when during an entry txn, the good (src) brick goes
offline before the post-op is done. This would cause self-heal to
do conservative merge (since dirty xattr is the only information
available), which when granular-entry-heal is enabled, expects
granular indices, the lack of which can lead to loss of data in
the worst case.

Change-Id: Ia3ad716d6fb1821555f02180e86e8711a79f958d
BUG: 1402730
Signed-off-by: Krutika Dhananjay <kdhananj@redhat.com>
Reviewed-on: http://review.gluster.org/16075
Smoke: Gluster Build System <jenkins@build.gluster.org>
Reviewed-by: Pranith Kumar Karampuri <pkarampu@redhat.com>
NetBSD-regression: NetBSD Build System <jenkins@build.gluster.org>
CentOS-regression: Gluster Build System <jenkins@build.gluster.org>

2 files changed, 69 insertions, 29 deletions

diff --git a/xlators/cluster/afr/src/afr-common.c b/xlators/cluster/afr/src/afr-common.c
index fcbfe9b0f2d..57745c323e4 100644
--- a/xlators/cluster/afr/src/afr-common.c
+++ b/xlators/cluster/afr/src/afr-common.c
@@ -5046,7 +5046,6 @@ out:
 int
 afr_transaction_local_init (afr_local_t *local, xlator_t *this)
 {
-        int            child_up_count = 0;
         int            ret = -ENOMEM;
         afr_private_t *priv = NULL;
 
@@ -5065,10 +5064,6 @@ afr_transaction_local_init (afr_local_t *local, xlator_t *this)
         }
 
         ret = -ENOMEM;
-        child_up_count = AFR_COUNT (local->child_up, priv->child_count);
-        if (priv->optimistic_change_log && child_up_count == priv->child_count)
-                local->optimistic_change_log = 1;
-
 	local->pre_op_compat = priv->pre_op_compat;
 
         local->transaction.eager_lock =
diff --git a/xlators/cluster/afr/src/afr-transaction.c b/xlators/cluster/afr/src/afr-transaction.c
index aef594bfc12..adaad407254 100644
--- a/xlators/cluster/afr/src/afr-transaction.c
+++ b/xlators/cluster/afr/src/afr-transaction.c
@@ -1183,19 +1183,21 @@ void
 afr_changelog_populate_xdata (call_frame_t *frame, afr_xattrop_type_t op,
                               dict_t **xdata, dict_t **newloc_xdata)
 {
-        dict_t      *xdata1 = NULL;
-        dict_t      *xdata2 = NULL;
-        afr_local_t *local  = NULL;
-        afr_private_t *priv = NULL;
-        int         ret     = 0;
-        const char  *name   = NULL;
+        int              i                    = 0;
+        int              ret                  = 0;
+        char            *key                  = NULL;
+        const char      *name                 = NULL;
+        dict_t          *xdata1               = NULL;
+        dict_t          *xdata2               = NULL;
+        xlator_t        *this                 = NULL;
+        afr_local_t     *local                = NULL;
+        afr_private_t   *priv                 = NULL;
+        gf_boolean_t     need_entry_key_set   = _gf_true;
 
         local = frame->local;
-        priv = THIS->private;
+        this = THIS;
+        priv = this->private;
 
-        /*Populate xdata for POST_OP only.*/
-        if (op == AFR_TRANSACTION_PRE_OP)
-                goto out;
         if (local->transaction.type == AFR_DATA_TRANSACTION ||
             local->transaction.type == AFR_METADATA_TRANSACTION)
                 goto out;
@@ -1206,26 +1208,53 @@ afr_changelog_populate_xdata (call_frame_t *frame, afr_xattrop_type_t op,
         xdata1 = dict_new();
         if (!xdata1)
                 goto out;
+
         name = local->loc.name;
         if (local->op == GF_FOP_LINK)
                 name = local->newloc.name;
-        ret = dict_set_str (xdata1, GF_XATTROP_ENTRY_IN_KEY, (char *)name);
-        if (ret)
-                gf_msg (THIS->name, GF_LOG_ERROR, 0, AFR_MSG_DICT_SET_FAILED,
-                        "%s/%s: Could not set xattrop-entry key during post-op",
-                        uuid_utoa (local->loc.pargfid), local->loc.name);
-        if (local->transaction.type == AFR_ENTRY_RENAME_TRANSACTION) {
-                xdata2 = dict_new();
-                if (!xdata2)
-                        goto out;
-                ret = dict_set_str (xdata2, GF_XATTROP_ENTRY_IN_KEY,
-                                    (char *)local->newloc.name);
+
+        switch (op) {
+        case AFR_TRANSACTION_PRE_OP:
+                key = GF_XATTROP_ENTRY_IN_KEY;
+                break;
+        case AFR_TRANSACTION_POST_OP:
+                if (afr_txn_nothing_failed (frame, this)) {
+                        key = GF_XATTROP_ENTRY_OUT_KEY;
+                        for (i = 0; i < priv->child_count; i++) {
+                                if (!local->transaction.failed_subvols[i])
+                                        continue;
+                                need_entry_key_set = _gf_false;
+                                break;
+                        }
+                } else {
+                        key = GF_XATTROP_ENTRY_IN_KEY;
+                }
+                break;
+        }
+
+        if (need_entry_key_set) {
+                ret = dict_set_str (xdata1, key, (char *)name);
                 if (ret)
                         gf_msg (THIS->name, GF_LOG_ERROR, 0,
                                 AFR_MSG_DICT_SET_FAILED,
-                                "%s/%s: Could not set xattrop-entry key during"
-                                " post-op", uuid_utoa (local->newloc.pargfid),
-                                local->newloc.name);
+                                "%s/%s: Could not set %s key during xattrop",
+                                uuid_utoa (local->loc.pargfid), local->loc.name,
+                                key);
+                if (local->transaction.type == AFR_ENTRY_RENAME_TRANSACTION) {
+                        xdata2 = dict_new ();
+                        if (!xdata2)
+                                goto out;
+
+                        ret = dict_set_str (xdata2, key,
+                                            (char *)local->newloc.name);
+                        if (ret)
+                                gf_msg (THIS->name, GF_LOG_ERROR, 0,
+                                        AFR_MSG_DICT_SET_FAILED,
+                                        "%s/%s: Could not set %s key during "
+                                        "xattrop",
+                                        uuid_utoa (local->newloc.pargfid),
+                                        local->newloc.name, key);
+                }
         }
 
         *xdata = xdata1;
@@ -1635,6 +1664,20 @@ afr_changelog_do (call_frame_t *frame, xlator_t *this, dict_t *xattr,
 	return 0;
 }
 
+static void
+afr_init_optimistic_changelog_for_txn (xlator_t *this, afr_local_t *local)
+{
+        int                locked_count   = 0;
+        afr_private_t     *priv           = NULL;
+
+        priv = this->private;
+
+        locked_count = AFR_COUNT (local->transaction.pre_op, priv->child_count);
+        if (priv->optimistic_change_log && locked_count == priv->child_count)
+                local->optimistic_change_log = 1;
+
+        return;
+}
 
 int
 afr_changelog_pre_op (call_frame_t *frame, xlator_t *this)
@@ -1666,6 +1709,8 @@ afr_changelog_pre_op (call_frame_t *frame, xlator_t *this)
                 }
 	}
 
+        afr_init_optimistic_changelog_for_txn (this, local);
+
         /* This condition should not be met with present code, as
          * transaction.done will be called if locks are not acquired on even a
          * single node.