summaryrefslogtreecommitdiffstats
path: root/xlators
diff options
context:
space:
mode:
authorRavishankar N <ravishankar@redhat.com>2018-12-03 18:17:34 +0530
committerRavishankar N <ravishankar@redhat.com>2018-12-03 18:26:09 +0530
commit1eef0a0f398413145063b1b8690213ca3aa31800 (patch)
tree927a5446ff69225b87d2319175aeb1e1a9fb66b1 /xlators
parent70ae565c759cb019b73c7b4d6ac3650293984bc2 (diff)
afr: assign gfid during name heal when no 'source' is present
Backport of https://review.gluster.org/#/c/glusterfs/+/21297/ Problem: If parent dir is in split-brain or has dirty xattrs set, and the file has gfid missing on one of the bricks, then name heal won't assign the gfid. Fix: Use the brick we select the gfid from as the 'source'. Note: Problem was found while trying to debug a split-brain issue on Cynthia Zhou's setup. fixes: bz#1655561 Change-Id: Id088d4f0fb017aa35122de426654194e581ed742 Reported-by: Cynthia Zhou <cynthia.zhou@nokia-sbell.com> Signed-off-by: Ravishankar N <ravishankar@redhat.com>
Diffstat (limited to 'xlators')
-rw-r--r--xlators/cluster/afr/src/afr-self-heal-common.c52
-rw-r--r--xlators/cluster/afr/src/afr-self-heal-entry.c6
-rw-r--r--xlators/cluster/afr/src/afr-self-heal-name.c46
-rw-r--r--xlators/cluster/afr/src/afr-self-heal.h2
4 files changed, 55 insertions, 51 deletions
diff --git a/xlators/cluster/afr/src/afr-self-heal-common.c b/xlators/cluster/afr/src/afr-self-heal-common.c
index 44d724112a9..a05c90f1b33 100644
--- a/xlators/cluster/afr/src/afr-self-heal-common.c
+++ b/xlators/cluster/afr/src/afr-self-heal-common.c
@@ -22,7 +22,8 @@ afr_heal_synctask (xlator_t *this, afr_local_t *local);
int
afr_lookup_and_heal_gfid (xlator_t *this, inode_t *parent, const char *name,
inode_t *inode, struct afr_reply *replies,
- int source, unsigned char *sources, void *gfid)
+ int source, unsigned char *sources, void *gfid,
+ int *gfid_idx)
{
afr_private_t *priv = NULL;
call_frame_t *frame = NULL;
@@ -36,15 +37,30 @@ afr_lookup_and_heal_gfid (xlator_t *this, inode_t *parent, const char *name,
priv = this->private;
wind_on = alloca0 (priv->child_count);
- ia_type = replies[source].poststat.ia_type;
- if ((ia_type == IA_INVAL) &&
- (AFR_COUNT(sources, priv->child_count) == priv->child_count)) {
- /* If a file is present on some bricks of the replica but parent
- * dir does not have pending xattrs, all bricks are sources and
- * the 'source' we selected earlier might be one where the file
- * is not actually present. Hence check if file is present in
- * any of the sources.*/
- for (i = 0; i < priv->child_count; i++) {
+ if (source >= 0 && replies[source].valid && replies[source].op_ret == 0)
+ ia_type = replies[source].poststat.ia_type;
+
+ if (ia_type != IA_INVAL)
+ goto heal;
+
+ /* If ia_type is still invalid, it means either
+ * (a)'source' was -1, i.e. parent dir pending xattrs are in split-brain
+ * (or) (b) The parent dir pending xattrs are all zeroes (i.e. all bricks
+ * are sources) and the 'source' we selected earlier might be the one where
+ * the file is not actually present.
+ *
+ * In both cases, let us pick a brick with a successful reply and use its
+ * ia_type.
+ */
+ for (i = 0; i < priv->child_count; i++) {
+ if (source == -1) {
+ /* case (a) above. */
+ if (replies[i].valid && replies[i].op_ret == 0) {
+ ia_type = replies[i].poststat.ia_type;
+ break;
+ }
+ } else {
+ /* case (b) above. */
if (i == source)
continue;
if (sources[i] && replies[i].valid &&
@@ -55,6 +71,7 @@ afr_lookup_and_heal_gfid (xlator_t *this, inode_t *parent, const char *name,
}
}
+heal:
/* gfid heal on those subvolumes that do not have gfid associated
* with the inode and update those replies.
*/
@@ -104,7 +121,22 @@ afr_lookup_and_heal_gfid (xlator_t *this, inode_t *parent, const char *name,
afr_reply_wipe (&replies[i]);
afr_reply_copy (&replies[i], &local->replies[i]);
}
+ if (gfid_idx && (*gfid_idx == -1)) {
+ /*Pick a brick where the gifd heal was successful.*/
+ for (i = 0; i < priv->child_count; i++) {
+ if (!wind_on[i])
+ continue;
+ if (replies[i].valid && replies[i].op_ret == 0 &&
+ !gf_uuid_is_null(replies[i].poststat.ia_gfid)) {
+ *gfid_idx = i;
+ break;
+ }
+ }
+ }
out:
+ if (gfid_idx && (*gfid_idx == -1) && (ret == 0)) {
+ ret = -afr_final_errno(local, priv);
+ }
loc_wipe (&loc);
if (frame)
AFR_STACK_DESTROY (frame);
diff --git a/xlators/cluster/afr/src/afr-self-heal-entry.c b/xlators/cluster/afr/src/afr-self-heal-entry.c
index 8514f0df595..cb03a3f9717 100644
--- a/xlators/cluster/afr/src/afr-self-heal-entry.c
+++ b/xlators/cluster/afr/src/afr-self-heal-entry.c
@@ -188,7 +188,8 @@ __afr_selfheal_heal_dirent (call_frame_t *frame, xlator_t *this, fd_t *fd,
if (replies[source].op_ret == 0) {
ret = afr_lookup_and_heal_gfid (this, fd->inode, name,
inode, replies, source, sources,
- &replies[source].poststat.ia_gfid);
+ &replies[source].poststat.ia_gfid,
+ NULL);
if (ret)
return ret;
}
@@ -321,7 +322,8 @@ __afr_selfheal_merge_dirent (call_frame_t *frame, xlator_t *this, fd_t *fd,
ret = afr_lookup_and_heal_gfid (this, fd->inode, name, inode, replies,
source, sources,
- &replies[source].poststat.ia_gfid);
+ &replies[source].poststat.ia_gfid,
+ NULL);
if (ret)
return ret;
diff --git a/xlators/cluster/afr/src/afr-self-heal-name.c b/xlators/cluster/afr/src/afr-self-heal-name.c
index 0a5be29d5ee..869bdbd49c6 100644
--- a/xlators/cluster/afr/src/afr-self-heal-name.c
+++ b/xlators/cluster/afr/src/afr-self-heal-name.c
@@ -19,7 +19,8 @@ __afr_selfheal_assign_gfid (xlator_t *this, inode_t *parent, uuid_t pargfid,
const char *bname, inode_t *inode,
struct afr_reply *replies, void *gfid,
unsigned char *locked_on, int source,
- unsigned char *sources, gf_boolean_t is_gfid_absent)
+ unsigned char *sources, gf_boolean_t is_gfid_absent,
+ int *gfid_idx)
{
int ret = 0;
int up_count = 0;
@@ -48,7 +49,7 @@ __afr_selfheal_assign_gfid (xlator_t *this, inode_t *parent, uuid_t pargfid,
}
afr_lookup_and_heal_gfid (this, parent, bname, inode, replies, source,
- sources, gfid);
+ sources, gfid, gfid_idx);
out:
return ret;
@@ -152,35 +153,6 @@ __afr_selfheal_name_expunge (xlator_t *this, inode_t *parent, uuid_t pargfid,
}
-/* This function is to be called after ensuring that there is no gfid mismatch
- * for the inode across multiple sources
- */
-static int
-afr_selfheal_gfid_idx_get (xlator_t *this, struct afr_reply *replies,
- unsigned char *sources)
-{
- int i = 0;
- int gfid_idx = -1;
- afr_private_t *priv = NULL;
-
- priv = this->private;
-
- for (i = 0; i < priv->child_count; i++) {
- if (!replies[i].valid || replies[i].op_ret != 0)
- continue;
-
- if (!sources[i])
- continue;
-
- if (gf_uuid_is_null (replies[i].poststat.ia_gfid))
- continue;
-
- gfid_idx = i;
- break;
- }
- return gfid_idx;
-}
-
static gf_boolean_t
afr_selfheal_name_need_heal_check (xlator_t *this, struct afr_reply *replies)
{
@@ -421,21 +393,19 @@ __afr_selfheal_name_do (call_frame_t *frame, xlator_t *this, inode_t *parent,
gfid = gfid_req;
} else {
gfid = &replies[gfid_idx].poststat.ia_gfid;
+ if (source == -1)
+ /* Either entry split-brain or dirty xattrs are
+ * present on parent.*/
+ source = gfid_idx;
}
is_gfid_absent = (gfid_idx == -1) ? _gf_true : _gf_false;
ret = __afr_selfheal_assign_gfid (this, parent, pargfid, bname, inode,
replies, gfid, locked_on, source,
- sources, is_gfid_absent);
+ sources, is_gfid_absent, &gfid_idx);
if (ret)
return ret;
- if (gfid_idx == -1) {
- gfid_idx = afr_selfheal_gfid_idx_get (this, replies, sources);
- if (gfid_idx == -1)
- return -1;
- }
-
ret = __afr_selfheal_name_impunge (frame, this, parent, pargfid,
bname, inode, replies, gfid_idx);
if (ret == -EIO)
diff --git a/xlators/cluster/afr/src/afr-self-heal.h b/xlators/cluster/afr/src/afr-self-heal.h
index bac5e51341e..d7491e1522d 100644
--- a/xlators/cluster/afr/src/afr-self-heal.h
+++ b/xlators/cluster/afr/src/afr-self-heal.h
@@ -113,7 +113,7 @@ afr_selfheal_entry (call_frame_t *frame, xlator_t *this, inode_t *inode);
int
afr_lookup_and_heal_gfid (xlator_t *this, inode_t *parent, const char *name,
inode_t *inode, struct afr_reply *replies, int source,
- unsigned char *sources, void *gfid);
+ unsigned char *sources, void *gfid, int *gfid_idx);
int
afr_selfheal_inodelk (call_frame_t *frame, xlator_t *this, inode_t *inode,