From 2b76520ca3e41cbac8f9318dce87e0b8d670c0ee Mon Sep 17 00:00:00 2001 From: Krutika Dhananjay Date: Thu, 8 Dec 2016 22:49:48 +0530 Subject: 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 Reviewed-on: http://review.gluster.org/16075 Smoke: Gluster Build System Reviewed-by: Pranith Kumar Karampuri NetBSD-regression: NetBSD Build System CentOS-regression: Gluster Build System --- xlators/cluster/afr/src/afr-transaction.c | 93 +++++++++++++++++++++++-------- 1 file changed, 69 insertions(+), 24 deletions(-) (limited to 'xlators/cluster/afr/src/afr-transaction.c') 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. -- cgit