cluster/afr: Prevent split-brain when bricks are brought off and on in cyclic order

When the bricks are brought offline and then online in cyclic
order while writes are in progress on a file, thanks to inode
refresh in write txns, AFR will mostly fail the write attempt
when the only good copy is offline. However, there is still a
remote possibility that the file will run into split-brain if
the brick that has the lone good copy goes offline *after* the
inode refresh but *before* the write txn completes (I call it
in-flight split-brain in the patch for ease of reference),
requiring intervention from admin to resolve the split-brain
before the IO can resume normally on the file. To get around this,
the patch does the following things:
i) retains the dirty xattrs on the file
ii) avoids marking the last of the good copies as bad (or accused)
    in case it is the one to go down during the course of a write.
iii) fails that particular write with the appropriate errno.

This way, we still have one good copy left despite the split-brain situation
which when it is back online, will be chosen as source to do the heal.

Change-Id: I9ca634b026ac830b172bac076437cc3bf1ae7d8a
BUG: 1363721
Signed-off-by: Krutika Dhananjay <kdhananj@redhat.com>
Reviewed-on: http://review.gluster.org/15080
Tested-by: Pranith Kumar Karampuri <pkarampu@redhat.com>
Smoke: Gluster Build System <jenkins@build.gluster.org>
CentOS-regression: Gluster Build System <jenkins@build.gluster.org>
Reviewed-by: Ravishankar N <ravishankar@redhat.com>
Reviewed-by: Oleksandr Natalenko <oleksandr@natalenko.name>
NetBSD-regression: NetBSD Build System <jenkins@build.gluster.org>
Reviewed-by: Pranith Kumar Karampuri <pkarampu@redhat.com>

5 files changed, 152 insertions, 19 deletions

diff --git a/xlators/cluster/afr/src/afr-common.c b/xlators/cluster/afr/src/afr-common.c
index 557b2cd8891..9b2c0d7caea 100644
--- a/xlators/cluster/afr/src/afr-common.c
+++ b/xlators/cluster/afr/src/afr-common.c
@@ -155,6 +155,119 @@ out:
  */
 
 int
+__afr_set_in_flight_sb_status (xlator_t *this, afr_local_t *local,
+                               inode_t *inode)
+{
+        int                 i               = 0;
+        int                 ret             = -1;
+        int                 txn_type        = 0;
+        int                 count           = 0;
+        int                 index           = -1;
+        uint16_t            datamap_old     = 0;
+        uint16_t            metadatamap_old = 0;
+        uint16_t            datamap         = 0;
+        uint16_t            metadatamap     = 0;
+        uint16_t            tmp_map         = 0;
+        uint16_t            mask            = 0;
+        uint32_t            event           = 0;
+        uint64_t            val             = 0;
+        afr_private_t      *priv            = NULL;
+        afr_inode_ctx_t    *ctx             = NULL;
+
+        priv = this->private;
+        txn_type = local->transaction.type;
+
+        ret = __afr_inode_ctx_get (this, inode, &ctx);
+        if (ret < 0)
+                return ret;
+
+        val = ctx->read_subvol;
+
+        metadatamap_old = metadatamap = (val & 0x000000000000ffff);
+        datamap_old = datamap = (val & 0x00000000ffff0000) >> 16;
+        /* Hard-code event to 0 since there is a failure and the inode
+         * needs to be refreshed anyway.
+         */
+        event = 0;
+
+        if (txn_type == AFR_DATA_TRANSACTION)
+                tmp_map = datamap;
+        else if (txn_type == AFR_METADATA_TRANSACTION)
+                tmp_map = metadatamap;
+
+        count = gf_bits_count (tmp_map);
+
+        if (count == 1)
+                index = gf_bits_index (tmp_map);
+
+        for (i = 0; i < priv->child_count; i++) {
+                mask = 0;
+                if (!local->transaction.failed_subvols[i])
+                        continue;
+
+                mask = 1 << i;
+                if (txn_type == AFR_METADATA_TRANSACTION)
+                        metadatamap &= ~mask;
+                else if (txn_type == AFR_DATA_TRANSACTION)
+                        datamap &= ~mask;
+        }
+
+        switch (txn_type) {
+        case AFR_METADATA_TRANSACTION:
+                if ((metadatamap_old != 0) && (metadatamap == 0) &&
+                    (count == 1)) {
+                        local->transaction.in_flight_sb_errno =
+                                                local->replies[index].op_errno;
+                        local->transaction.in_flight_sb = _gf_true;
+                        metadatamap |= (1 << index);
+                }
+                break;
+
+        case AFR_DATA_TRANSACTION:
+                if ((datamap_old != 0) && (datamap == 0) && (count == 1)) {
+                        local->transaction.in_flight_sb_errno =
+                                                local->replies[index].op_errno;
+                        local->transaction.in_flight_sb = _gf_true;
+                        datamap |= (1 << index);
+                }
+                break;
+
+        default:
+        break;
+        }
+
+        val = ((uint64_t) metadatamap) |
+                (((uint64_t) datamap) << 16) |
+                (((uint64_t) event) << 32);
+
+        ctx->read_subvol = val;
+
+        return ret;
+}
+
+int
+afr_set_in_flight_sb_status (xlator_t *this, afr_local_t *local, inode_t *inode)
+{
+        int            ret  = -1;
+        afr_private_t *priv = NULL;
+
+        priv = this->private;
+
+        /* If this transaction saw no failures, then exit. */
+        if (AFR_COUNT (local->transaction.failed_subvols,
+                       priv->child_count) == 0)
+                return 0;
+
+        LOCK (&inode->lock);
+        {
+                ret = __afr_set_in_flight_sb_status (this, local, inode);
+        }
+        UNLOCK (&inode->lock);
+
+        return ret;
+}
+
+int
 __afr_inode_read_subvol_get_small (inode_t *inode, xlator_t *this,
 				   unsigned char *data, unsigned char *metadata,
 				   int *event_p)
@@ -233,12 +346,12 @@ out:
 int
 __afr_inode_read_subvol_reset_small (inode_t *inode, xlator_t *this)
 {
-	int ret = -1;
-	uint16_t datamap = 0;
-	uint16_t metadatamap = 0;
-	uint32_t event = 0;
-	uint64_t val = 0;
-        afr_inode_ctx_t *ctx = NULL;
+	int               ret         = -1;
+	uint16_t          datamap     = 0;
+	uint16_t          metadatamap = 0;
+	uint32_t          event       = 0;
+	uint64_t          val         = 0;
+        afr_inode_ctx_t  *ctx         = NULL;
 
 	ret = __afr_inode_ctx_get (this, inode, &ctx);
         if (ret)
diff --git a/xlators/cluster/afr/src/afr-dir-write.c b/xlators/cluster/afr/src/afr-dir-write.c
index f3de5352d7e..286a5392da6 100644
--- a/xlators/cluster/afr/src/afr-dir-write.c
+++ b/xlators/cluster/afr/src/afr-dir-write.c
@@ -122,8 +122,7 @@ __afr_dir_write_finalize (call_frame_t *frame, xlator_t *this)
 			continue;
 		if (local->replies[i].op_ret < 0) {
 			if (local->inode)
-				afr_inode_read_subvol_reset (local->inode,
-							     this);
+				afr_inode_read_subvol_reset (local->inode, this);
 			if (local->parent)
 				afr_inode_read_subvol_reset (local->parent,
 							     this);
diff --git a/xlators/cluster/afr/src/afr-inode-write.c b/xlators/cluster/afr/src/afr-inode-write.c
index 323f50c9413..ddc257dbde4 100644
--- a/xlators/cluster/afr/src/afr-inode-write.c
+++ b/xlators/cluster/afr/src/afr-inode-write.c
@@ -38,13 +38,13 @@
 static void
 __afr_inode_write_finalize (call_frame_t *frame, xlator_t *this)
 {
-	afr_local_t *local = NULL;
-	afr_private_t *priv = NULL;
-	int read_subvol = 0;
-	int i = 0;
-        afr_read_subvol_args_t args = {0,};
-        struct iatt  *stbuf = NULL;
-        int    ret = 0;
+	int                       i               = 0;
+        int                       ret             = 0;
+	int                       read_subvol     = 0;
+        struct iatt              *stbuf           = NULL;
+	afr_local_t              *local           = NULL;
+	afr_private_t            *priv            = NULL;
+        afr_read_subvol_args_t    args            = {0,};
 
 	local = frame->local;
 	priv = this->private;
@@ -94,10 +94,8 @@ __afr_inode_write_finalize (call_frame_t *frame, xlator_t *this)
 	for (i = 0; i < priv->child_count; i++) {
 		if (!local->replies[i].valid)
 			continue;
-		if (local->replies[i].op_ret < 0) {
-			afr_inode_read_subvol_reset (local->inode, this);
+		if (local->replies[i].op_ret < 0)
 			continue;
-		}
 
 		/* Order of checks in the compound conditional
 		   below is important.
@@ -134,6 +132,7 @@ __afr_inode_write_finalize (call_frame_t *frame, xlator_t *this)
 	}
 
         afr_txn_arbitrate_fop_cbk (frame, this);
+        afr_set_in_flight_sb_status (this, local, local->inode);
 }
 
 
diff --git a/xlators/cluster/afr/src/afr-transaction.c b/xlators/cluster/afr/src/afr-transaction.c
index 274e90d6f3c..6130ad76543 100644
--- a/xlators/cluster/afr/src/afr-transaction.c
+++ b/xlators/cluster/afr/src/afr-transaction.c
@@ -850,6 +850,13 @@ afr_changelog_post_op_now (call_frame_t *frame, xlator_t *this)
                 goto out;
 	}
 
+        if (local->transaction.in_flight_sb) {
+                local->op_ret = -1;
+                local->op_errno = local->transaction.in_flight_sb_errno;
+                afr_changelog_post_op_done (frame, this);
+                goto out;
+        }
+
 	xattr = dict_new ();
 	if (!xattr) {
 		local->op_ret = -1;
diff --git a/xlators/cluster/afr/src/afr.h b/xlators/cluster/afr/src/afr.h
index 54f988e8f33..29008287e6d 100644
--- a/xlators/cluster/afr/src/afr.h
+++ b/xlators/cluster/afr/src/afr.h
@@ -735,10 +735,17 @@ typedef struct _afr_local {
 		gf_boolean_t uninherit_done;
 		gf_boolean_t uninherit_value;
 
+                gf_boolean_t in_flight_sb; /* Indicator for occurrence of
+                                              split-brain while in the middle of
+                                              a txn. */
+                int32_t in_flight_sb_errno; /* This is where the cause of the
+                                               failure on the last good copy of
+                                               the file is stored.
+                                               */
+
 		/* @changelog_resume: function to be called after changlogging
 		   (either pre-op or post-op) is done
 		*/
-
 		afr_changelog_resume_t changelog_resume;
 
                 call_frame_t *main_frame;
@@ -871,6 +878,10 @@ afr_read_subvol_get (inode_t *inode, xlator_t *this, int *subvol_p,
 	afr_read_subvol_get(i, t, s, r, e, AFR_METADATA_TRANSACTION, a)
 
 int
+afr_inode_ctx_reset_unreadable_subvol (inode_t *inode, xlator_t *this,
+                                       int subvol_idx, int txn_type);
+
+int
 afr_inode_refresh (call_frame_t *frame, xlator_t *this, inode_t *inode,
                    uuid_t gfid, afr_inode_refresh_cbk_t cbk);
 
@@ -1145,4 +1156,8 @@ afr_selfheal_data_open (xlator_t *this, inode_t *inode, fd_t **fd);
 
 int
 afr_get_msg_id (char *op_type);
+
+int
+afr_set_in_flight_sb_status (xlator_t *this, afr_local_t *local,
+                             inode_t *inode);
 #endif /* __AFR_H__ */