From 449250fa06c3a9ce9fb185216885b817005ed12a Mon Sep 17 00:00:00 2001

From: Krutika Dhananjay <kdhananj@redhat.com>

Date: Thu, 8 Dec 2016 22:49:48 +0530

Subject: [PATCH 237/246] cluster/afr: Fix per-txn optimistic changelog

 initialisation

        Backport of: http://review.gluster.org/16075

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: I1b752fc92fdee389daca89dbeadec2012546d7a2

BUG: 1400057

Signed-off-by: Krutika Dhananjay <kdhananj@redhat.com>

Reviewed-on: https://code.engineering.redhat.com/gerrit/92713

Reviewed-by: Pranith Kumar Karampuri <pkarampu@redhat.com>

Tested-by: Pranith Kumar Karampuri <pkarampu@redhat.com>

---

 tests/bugs/replicate/bug-1402730.t        | 42 ++++++++++++++

 xlators/cluster/afr/src/afr-common.c      |  5 --

 xlators/cluster/afr/src/afr-transaction.c | 93 +++++++++++++++++++++++--------

 3 files changed, 111 insertions(+), 29 deletions(-)

 create mode 100644 tests/bugs/replicate/bug-1402730.t

diff --git a/tests/bugs/replicate/bug-1402730.t b/tests/bugs/replicate/bug-1402730.t

new file mode 100644

index 0000000..c6768a0

--- /dev/null

+++ b/tests/bugs/replicate/bug-1402730.t

@@ -0,0 +1,42 @@

+#!/bin/bash

+. $(dirname $0)/../../include.rc

+. $(dirname $0)/../../volume.rc

+

+cleanup

+

+TEST glusterd

+TEST pidof glusterd

+TEST $CLI volume create $V0 replica 3 $H0:$B0/${V0}{0,1,2}

+TEST $CLI volume set $V0 granular-entry-heal on

+TEST $CLI volume set $V0 cluster.data-self-heal off

+TEST $CLI volume set $V0 cluster.metadata-self-heal off

+TEST $CLI volume set $V0 cluster.entry-self-heal off

+TEST $CLI volume set $V0 cluster.self-heal-daemon off

+TEST $CLI volume start $V0

+

+TEST glusterfs --volfile-id=$V0 --volfile-server=$H0 --entry-timeout=0 $M0

+

+TEST mkdir -p $M0/a/b/c -p

+cd $M0/a/b/c

+

+TEST kill_brick $V0 $H0 $B0/${V0}2

+rm -rf $B0/${V0}2/*

+rm -rf $B0/${V0}2/.glusterfs

+TEST $CLI volume start $V0 force

+

+EXPECT_WITHIN $PROCESS_UP_TIMEOUT "1" afr_child_up_status $V0 0

+EXPECT_WITHIN $PROCESS_UP_TIMEOUT "1" afr_child_up_status $V0 1

+EXPECT_WITHIN $PROCESS_UP_TIMEOUT "1" afr_child_up_status $V0 2

+

+TEST touch file

+

+GFID_C=$(get_gfid_string $M0/a/b/c)

+TEST stat $B0/${V0}0/.glusterfs/indices/entry-changes/$GFID_C/file

+TEST stat $B0/${V0}1/.glusterfs/indices/entry-changes/$GFID_C/file

+

+EXPECT "00000001" afr_get_specific_changelog_xattr $B0/${V0}0/a/b/c trusted.afr.$V0-client-2 entry

+EXPECT "00000001" afr_get_specific_changelog_xattr $B0/${V0}1/a/b/c trusted.afr.$V0-client-2 entry

+

+cd ~

+

+cleanup

diff --git a/xlators/cluster/afr/src/afr-common.c b/xlators/cluster/afr/src/afr-common.c

index b825b19..60bae18 100644

--- a/xlators/cluster/afr/src/afr-common.c

+++ b/xlators/cluster/afr/src/afr-common.c

@@ -4949,7 +4949,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;

@@ -4968,10 +4967,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 f397ae7..9a3b2e9 100644

--- a/xlators/cluster/afr/src/afr-transaction.c

+++ b/xlators/cluster/afr/src/afr-transaction.c

@@ -1163,19 +1163,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;

@@ -1186,26 +1188,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;

@@ -1622,6 +1651,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)

@@ -1653,6 +1696,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.

-- 

2.9.3