From 231cea9678734044d2294e74d3742e61058bc760 Mon Sep 17 00:00:00 2001
From: Pranith Kumar K <pkarampu@redhat.com>
Date: Fri, 27 May 2016 15:47:07 +0530
Subject: [PATCH 175/178] cluster/afr: Unwind xdata_rsp even in case of failures
DHT expects GF_PREOP_CHECK_FAILED to be present in xdata_rsp in case of mkdir
failures because of stale layout. But AFR was unwinding null xdata_rsp in case
of failures. This was leading to mkdir failures just after remove-brick. Unwind
the xdata_rsp in case of failures to make sure the response from brick reaches
dht.
>BUG: 1340623
>Change-Id: Idd3f7b95730e8ea987b608e892011ff190e181d1
>Signed-off-by: Pranith Kumar K <pkarampu@redhat.com>
>Reviewed-on: http://review.gluster.org/14553
>NetBSD-regression: NetBSD Build System <jenkins@build.gluster.org>
>Reviewed-by: Ravishankar N <ravishankar@redhat.com>
>Smoke: Gluster Build System <jenkins@build.gluster.com>
>CentOS-regression: Gluster Build System <jenkins@build.gluster.com>
>Reviewed-by: Anuradha Talur <atalur@redhat.com>
>Reviewed-by: Krutika Dhananjay <kdhananj@redhat.com>
BUG: 1340085
Change-Id: I7fb653cea3e888de9f0db6f96a32156ace3c3fea
Signed-off-by: Pranith Kumar K <pkarampu@redhat.com>
Reviewed-on: https://code.engineering.redhat.com/gerrit/75455
---
xlators/cluster/afr/src/afr-common.c | 29 +++++++++---
xlators/cluster/afr/src/afr-dir-read.c | 3 +-
xlators/cluster/afr/src/afr-dir-write.c | 20 ++++++---
xlators/cluster/afr/src/afr-inode-write.c | 6 +-
xlators/cluster/afr/src/afr-transaction.c | 71 +++++++++++++++++++++++++++++
xlators/cluster/afr/src/afr-transaction.h | 4 ++
xlators/cluster/afr/src/afr.h | 8 ++-
7 files changed, 120 insertions(+), 21 deletions(-)
diff --git a/xlators/cluster/afr/src/afr-common.c b/xlators/cluster/afr/src/afr-common.c
index 3bed0c5..605c641 100644
--- a/xlators/cluster/afr/src/afr-common.c
+++ b/xlators/cluster/afr/src/afr-common.c
@@ -1274,6 +1274,7 @@ afr_inode_read_subvol_type_get (inode_t *inode, xlator_t *this,
int
afr_read_subvol_get (inode_t *inode, xlator_t *this, int *subvol_p,
+ unsigned char *readables,
int *event_p, afr_transaction_type type,
afr_read_subvol_args_t *args)
{
@@ -1310,6 +1311,9 @@ afr_read_subvol_get (inode_t *inode, xlator_t *this, int *subvol_p,
*subvol_p = subvol;
if (event_p)
*event_p = event;
+ if (readables)
+ memcpy (readables, readable,
+ sizeof (*readables) * priv->child_count);
return subvol;
}
@@ -1439,6 +1443,7 @@ afr_local_cleanup (afr_local_t *local, xlator_t *this)
GF_FREE (local->read_attempted);
GF_FREE (local->readable);
+ GF_FREE (local->readable2);
if (local->inode)
inode_unref (local->inode);
@@ -1598,8 +1603,8 @@ afr_get_parent_read_subvol (xlator_t *this, inode_t *parent,
priv = this->private;
if (parent)
- par_read_subvol = afr_data_subvol_get (parent, this, 0, 0,
- NULL);
+ par_read_subvol = afr_data_subvol_get (parent, this, NULL, NULL,
+ NULL, NULL);
for (i = 0; i < priv->child_count; i++) {
if (!replies[i].valid)
@@ -1638,8 +1643,7 @@ afr_read_subvol_decide (inode_t *inode, xlator_t *this,
int data_subvol = -1;
int mdata_subvol = -1;
- data_subvol = afr_data_subvol_get (inode, this,
- 0, 0, args);
+ data_subvol = afr_data_subvol_get (inode, this, NULL, NULL, NULL, args);
mdata_subvol = afr_metadata_subvol_get (inode, this,
0, 0, args);
if (data_subvol == -1 || mdata_subvol == -1)
@@ -1787,7 +1791,7 @@ afr_lookup_done (call_frame_t *frame, xlator_t *this)
goto cant_interpret;
} else {
read_subvol = afr_data_subvol_get (local->inode, this,
- 0, 0, &args);
+ NULL, NULL, NULL, &args);
}
} else {
cant_interpret:
@@ -2414,7 +2418,7 @@ afr_discover (call_frame_t *frame, xlator_t *this, loc_t *loc, dict_t *xattr_req
return 0;
}
- afr_read_subvol_get (loc->inode, this, NULL, &event,
+ afr_read_subvol_get (loc->inode, this, NULL, NULL, &event,
AFR_DATA_TRANSACTION, NULL);
if (event != local->event_generation)
@@ -2565,7 +2569,7 @@ afr_lookup (call_frame_t *frame, xlator_t *this, loc_t *loc, dict_t *xattr_req)
}
}
- afr_read_subvol_get (loc->parent, this, NULL, &event,
+ afr_read_subvol_get (loc->parent, this, NULL, NULL, &event,
AFR_DATA_TRANSACTION, NULL);
if (event != local->event_generation)
@@ -2888,7 +2892,8 @@ afr_fsync_cbk (call_frame_t *frame, void *cookie, xlator_t *this,
local = frame->local;
- read_subvol = afr_data_subvol_get (local->inode, this, 0, 0, NULL);
+ read_subvol = afr_data_subvol_get (local->inode, this, NULL, NULL,
+ NULL, NULL);
LOCK (&frame->lock);
{
@@ -4288,6 +4293,14 @@ afr_local_init (afr_local_t *local, afr_private_t *priv, int32_t *op_errno)
goto out;
}
+ local->readable2 = GF_CALLOC (priv->child_count, sizeof (char),
+ gf_afr_mt_char);
+ if (!local->readable2) {
+ if (op_errno)
+ *op_errno = ENOMEM;
+ goto out;
+ }
+
local->replies = GF_CALLOC(priv->child_count, sizeof(*local->replies),
gf_afr_mt_reply_t);
if (!local->replies) {
diff --git a/xlators/cluster/afr/src/afr-dir-read.c b/xlators/cluster/afr/src/afr-dir-read.c
index 559211e..f7da55f 100644
--- a/xlators/cluster/afr/src/afr-dir-read.c
+++ b/xlators/cluster/afr/src/afr-dir-read.c
@@ -158,7 +158,8 @@ afr_validate_read_subvol (inode_t *inode, xlator_t *this, int par_read_subvol)
* -1 above due to gen being 0, which is why it is OK to pass NULL for
* read_subvol_args here.
*/
- entry_read_subvol = afr_data_subvol_get (inode, this, 0, 0, NULL);
+ entry_read_subvol = afr_data_subvol_get (inode, this, NULL, NULL,
+ NULL, NULL);
if (entry_read_subvol != par_read_subvol)
return -1;
diff --git a/xlators/cluster/afr/src/afr-dir-write.c b/xlators/cluster/afr/src/afr-dir-write.c
index b1ff557..bd24276 100644
--- a/xlators/cluster/afr/src/afr-dir-write.c
+++ b/xlators/cluster/afr/src/afr-dir-write.c
@@ -106,18 +106,21 @@ __afr_dir_write_finalize (call_frame_t *frame, xlator_t *this)
if (local->inode) {
afr_replies_interpret (frame, this, local->inode, NULL);
inode_read_subvol = afr_data_subvol_get (local->inode, this,
- NULL, NULL, &args);
+ NULL, NULL, NULL, &args);
}
if (local->parent)
parent_read_subvol = afr_data_subvol_get (local->parent, this,
- NULL, NULL, NULL);
+ NULL, local->readable, NULL, NULL);
+
if (local->parent2)
parent2_read_subvol = afr_data_subvol_get (local->parent2, this,
- NULL, NULL, NULL);
+ NULL, local->readable2, NULL, NULL);
local->op_ret = -1;
local->op_errno = afr_final_errno (local, priv);
+ afr_pick_error_xdata (local, priv, local->parent, local->readable,
+ local->parent2, local->readable2);
for (i = 0; i < priv->child_count; i++) {
if (!local->replies[i].valid)
@@ -149,6 +152,11 @@ __afr_dir_write_finalize (call_frame_t *frame, xlator_t *this)
local->replies[i].preparent2;
local->cont.dir_fop.postnewparent =
local->replies[i].postparent2;
+ if (local->xdata_rsp) {
+ dict_unref (local->xdata_rsp);
+ local->xdata_rsp = NULL;
+ }
+
if (local->replies[i].xdata)
local->xdata_rsp =
dict_ref (local->replies[i].xdata);
@@ -201,6 +209,9 @@ __afr_dir_write_fill (call_frame_t *frame, xlator_t *this, int child_index,
local->replies[child_index].valid = 1;
local->replies[child_index].op_ret = op_ret;
local->replies[child_index].op_errno = op_errno;
+ if (xdata)
+ local->replies[child_index].xdata = dict_ref (xdata);
+
if (op_ret >= 0) {
if (poststat)
@@ -213,9 +224,6 @@ __afr_dir_write_fill (call_frame_t *frame, xlator_t *this, int child_index,
local->replies[child_index].preparent2 = *preparent2;
if (postparent2)
local->replies[child_index].postparent2 = *postparent2;
- if (xdata)
- local->replies[child_index].xdata = dict_ref (xdata);
-
if (fd_ctx)
fd_ctx->opened_on[child_index] = AFR_FD_OPENED;
} else {
diff --git a/xlators/cluster/afr/src/afr-inode-write.c b/xlators/cluster/afr/src/afr-inode-write.c
index baa5871..9206681 100644
--- a/xlators/cluster/afr/src/afr-inode-write.c
+++ b/xlators/cluster/afr/src/afr-inode-write.c
@@ -89,7 +89,7 @@ __afr_inode_write_finalize (call_frame_t *frame, xlator_t *this)
&args);
else
read_subvol = afr_data_subvol_get (local->inode, this,
- NULL, NULL, &args);
+ NULL, NULL, NULL, &args);
}
local->op_ret = -1;
@@ -169,8 +169,8 @@ __afr_inode_write_fill (call_frame_t *frame, xlator_t *this, int child_index,
local->replies[child_index].poststat = *postbuf;
if (xattr)
local->replies[child_index].xattr = dict_ref (xattr);
- if (xdata)
- local->replies[child_index].xdata = dict_ref (xdata);
+ if (xdata)
+ local->replies[child_index].xdata = dict_ref (xdata);
} else {
afr_transaction_fop_failed (frame, this, child_index);
}
diff --git a/xlators/cluster/afr/src/afr-transaction.c b/xlators/cluster/afr/src/afr-transaction.c
index 8b667c9..316c7cd 100644
--- a/xlators/cluster/afr/src/afr-transaction.c
+++ b/xlators/cluster/afr/src/afr-transaction.c
@@ -51,6 +51,66 @@ afr_zero_fill_stat (afr_local_t *local)
}
}
+/* In case of errors afr needs to choose which xdata from lower xlators it needs
+ * to unwind with. The way it is done is by checking if there are
+ * any good subvols which failed. Give preference to errnos other than
+ * ENOTCONN even if the child is source */
+void
+afr_pick_error_xdata (afr_local_t *local, afr_private_t *priv,
+ inode_t *inode1, unsigned char *readable1,
+ inode_t *inode2, unsigned char *readable2)
+{
+ int s = -1;/*selection*/
+ int i = 0;
+ unsigned char *readable = NULL;
+
+ if (local->xdata_rsp) {
+ dict_unref (local->xdata_rsp);
+ local->xdata_rsp = NULL;
+ }
+
+ readable = alloca0 (priv->child_count * sizeof (*readable));
+ if (inode2 && readable2) {/*rename fop*/
+ AFR_INTERSECT (readable, readable1, readable2,
+ priv->child_count);
+ } else {
+ memcpy (readable, readable1,
+ sizeof (*readable) * priv->child_count);
+ }
+
+ for (i = 0; i < priv->child_count; i++) {
+ if (!local->replies[i].valid)
+ continue;
+
+ if (local->replies[i].op_ret >= 0)
+ continue;
+
+ if (local->replies[i].op_errno == ENOTCONN)
+ continue;
+
+ /*Order is important in the following condition*/
+ if ((s < 0) || (!readable[s] && readable[i]))
+ s = i;
+ }
+
+ if (s != -1 && local->replies[s].xdata) {
+ local->xdata_rsp = dict_ref (local->replies[s].xdata);
+ } else if (s == -1) {
+ for (i = 0; i < priv->child_count; i++) {
+ if (!local->replies[i].valid)
+ continue;
+
+ if (local->replies[i].op_ret >= 0)
+ continue;
+
+ if (!local->replies[i].xdata)
+ continue;
+ local->xdata_rsp = dict_ref (local->replies[i].xdata);
+ break;
+ }
+ }
+}
+
gf_boolean_t
afr_needs_changelog_update (afr_local_t *local)
{
@@ -741,6 +801,17 @@ afr_handle_quorum (call_frame_t *frame)
local->op_errno = afr_final_errno (local, priv);
if (local->op_errno == 0)
local->op_errno = afr_quorum_errno (priv);
+ switch (local->transaction.type) {
+ case AFR_ENTRY_TRANSACTION:
+ case AFR_ENTRY_RENAME_TRANSACTION:
+ afr_pick_error_xdata (local, priv, local->parent,
+ local->readable, local->parent2,
+ local->readable2);
+ break;
+ default:
+ /*TBD*/
+ break;
+ }
}
int
diff --git a/xlators/cluster/afr/src/afr-transaction.h b/xlators/cluster/afr/src/afr-transaction.h
index c58531e..ca8fcfe 100644
--- a/xlators/cluster/afr/src/afr-transaction.h
+++ b/xlators/cluster/afr/src/afr-transaction.h
@@ -55,4 +55,8 @@ gf_boolean_t afr_has_quorum (unsigned char *subvols, xlator_t *this);
gf_boolean_t afr_needs_changelog_update (afr_local_t *local);
void afr_zero_fill_stat (afr_local_t *local);
+void
+afr_pick_error_xdata (afr_local_t *local, afr_private_t *priv,
+ inode_t *inode1, unsigned char *readable1,
+ inode_t *inode2, unsigned char *readable2);
#endif /* __TRANSACTION_H__ */
diff --git a/xlators/cluster/afr/src/afr.h b/xlators/cluster/afr/src/afr.h
index 6370577..63be1fb 100644
--- a/xlators/cluster/afr/src/afr.h
+++ b/xlators/cluster/afr/src/afr.h
@@ -417,6 +417,7 @@ typedef struct _afr_local {
performed. This is the output of afr_inode_refresh()
*/
unsigned char *readable;
+ unsigned char *readable2; /*For rename transaction*/
afr_inode_refresh_cbk_t refreshfn;
@@ -836,14 +837,15 @@ afr_inode_read_subvol_type_get (inode_t *inode, xlator_t *this,
int type);
int
afr_read_subvol_get (inode_t *inode, xlator_t *this, int *subvol_p,
+ unsigned char *readables,
int *event_p, afr_transaction_type type,
afr_read_subvol_args_t *args);
-#define afr_data_subvol_get(i, t, s, e, a) \
- afr_read_subvol_get(i, t, s, e, AFR_DATA_TRANSACTION, a)
+#define afr_data_subvol_get(i, t, s, r, e, a) \
+ afr_read_subvol_get(i, t, s, r, e, AFR_DATA_TRANSACTION, a)
#define afr_metadata_subvol_get(i, t, s, e, a) \
- afr_read_subvol_get(i, t, s, e, AFR_METADATA_TRANSACTION, a)
+ afr_read_subvol_get(i, t, s, NULL, e, AFR_METADATA_TRANSACTION, a)
int
afr_inode_refresh (call_frame_t *frame, xlator_t *this, inode_t *inode,
--
1.7.1