From 220e093d7838493d2f88c77ebda18d277b21a4d9 Mon Sep 17 00:00:00 2001

From: karthik-us <ksubrahm@redhat.com>

Date: Thu, 5 Jan 2017 14:06:21 +0530

Subject: [PATCH 266/267] glusterd: Fail add-brick on replica count change, if

 brick is down

Problem:

1. Have a replica 2 volume with bricks b1 and b2

2. Before setting the layout, b1 goes down

3. Set the layout write some data, which gets populated on b2

4. b2 goes down then b1 comes up

5. Add another brick b3, and heal will take place from b1 to b3, which

   basically have no data

6. Write some data. Both b1 and b3 will mark b2 for pending writes

7. b1 goes down, and b2 comes up

8. b2 gets heald from b1. During heal it removes the data which is already

   in b2, considering that as stale data. This leads to data loss.

Solution:

1. In glusterd stage-op, while adding bricks, check whether the replica

   count is being increased

2. If yes, then check whether any of the bricks are down at that time

3. If yes, then fail the add-brick to avoid such data loss

4. Else continue the normal operation.

This check will work enen when we convert plain distribute volume to replicate

Test:

1. Create a replica 2 volume

2. Kill one brick from the volume

3. Try adding a brick to the volume

4. It should fail with all bricks are not up error

5. Cretae a distribute volume and kill one of the brick

6. Try to convert it to replicate volume, by adding bricks.

7. This should also fail.

>Reviewed-on: http://review.gluster.org/16330

>Tested-by: Ravishankar N <ravishankar@redhat.com>

>Smoke: Gluster Build System <jenkins@build.gluster.org>

>NetBSD-regression: NetBSD Build System <jenkins@build.gluster.org>

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

>CentOS-regression: Gluster Build System <jenkins@build.gluster.org>

>Reviewed-by: N Balachandran <nbalacha@redhat.com>

>Reviewed-by: Atin Mukherjee <amukherj@redhat.com>

Change-Id: I9c8d2ab104263e4206814c94c19212ab914ed07c

BUG: 1404989

Signed-off-by: karthik-us <ksubrahm@redhat.com>

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

Reviewed-by: Atin Mukherjee <amukherj@redhat.com>

Tested-by: Atin Mukherjee <amukherj@redhat.com>

---

 tests/basic/afr/arbiter-add-brick.t                | 10 ++++--

 ...406411-fail-add-brick-on-replica-count-change.t | 40 ++++++++++++++++++++++

 xlators/mgmt/glusterd/src/glusterd-brick-ops.c     | 19 ++++++++++

 3 files changed, 66 insertions(+), 3 deletions(-)

 create mode 100644 tests/bugs/glusterd/bug-1406411-fail-add-brick-on-replica-count-change.t

diff --git a/tests/basic/afr/arbiter-add-brick.t b/tests/basic/afr/arbiter-add-brick.t

index 357f59b..69e1326 100644

--- a/tests/basic/afr/arbiter-add-brick.t

+++ b/tests/basic/afr/arbiter-add-brick.t

@@ -10,6 +10,7 @@ TEST pidof glusterd

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

 TEST $CLI volume set $V0 performance.stat-prefetch off

 TEST $CLI volume start $V0

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

 TEST glusterfs --volfile-id=$V0 --volfile-server=$H0 $M0;

 TEST mkdir  $M0/dir1

 TEST dd if=/dev/urandom of=$M0/file1 bs=1024 count=1

@@ -20,21 +21,24 @@ TEST mkdir $M0/dir2

 TEST dd if=/dev/urandom of=$M0/file1 bs=1024 count=1024

+#convert replica 2 to arbiter volume

+TEST $CLI volume start $V0 force

+EXPECT_WITHIN $PROCESS_UP_TIMEOUT "1" brick_up_status $V0 $H0 $B0/${V0}1

+

 #syntax check for add-brick.

 TEST ! $CLI volume add-brick $V0 replica 2 arbiter 1 $H0:$B0/${V0}2

 TEST ! $CLI volume add-brick $V0 replica 3 arbiter 2 $H0:$B0/${V0}2

-#convert replica 2 to arbiter volume

 TEST $CLI volume add-brick $V0 replica 3 arbiter 1 $H0:$B0/${V0}2

 EXPECT_WITHIN $PROCESS_UP_TIMEOUT "1" brick_up_status $V0 $H0 $B0/${V0}2

 #Heal files

-TEST $CLI volume start $V0 force

-EXPECT_WITHIN $PROCESS_UP_TIMEOUT "1" brick_up_status $V0 $H0 $B0/${V0}1

+TEST $CLI volume set $V0 self-heal-daemon on

 EXPECT_WITHIN $PROCESS_UP_TIMEOUT "Y" glustershd_up_status

 EXPECT_WITHIN $CHILD_UP_TIMEOUT "1" afr_child_up_status_in_shd $V0 0

 EXPECT_WITHIN $CHILD_UP_TIMEOUT "1" afr_child_up_status_in_shd $V0 1

 EXPECT_WITHIN $CHILD_UP_TIMEOUT "1" afr_child_up_status_in_shd $V0 2

+TEST $CLI volume heal $V0

 EXPECT_WITHIN $HEAL_TIMEOUT "0" get_pending_heal_count $V0

 #Perform I/O after add-brick

diff --git a/tests/bugs/glusterd/bug-1406411-fail-add-brick-on-replica-count-change.t b/tests/bugs/glusterd/bug-1406411-fail-add-brick-on-replica-count-change.t

new file mode 100644

index 0000000..bbd2d4c

--- /dev/null

+++ b/tests/bugs/glusterd/bug-1406411-fail-add-brick-on-replica-count-change.t

@@ -0,0 +1,40 @@

+#!/bin/bash

+

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

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

+

+cleanup;

+

+TEST glusterd;

+TEST pidof glusterd

+

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

+TEST $CLI volume start $V0

+

+EXPECT_WITHIN $PROCESS_UP_TIMEOUT "1" brick_up_status $V0 $H0 $B0/${V0}1

+EXPECT_WITHIN $PROCESS_UP_TIMEOUT "1" brick_up_status $V0 $H0 $B0/${V0}2

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

+

+#add-brick should fail

+TEST ! $CLI volume add-brick $V0 replica 3 $H0:$B0/${V0}3

+

+TEST $CLI volume start $V0 force

+EXPECT_WITHIN $PROCESS_UP_TIMEOUT "1" brick_up_status $V0 $H0 $B0/${V0}1

+EXPECT_WITHIN $PROCESS_UP_TIMEOUT "1" brick_up_status $V0 $H0 $B0/${V0}2

+TEST $CLI volume add-brick $V0 replica 3 $H0:$B0/${V0}3

+

+TEST $CLI volume create $V1 $H0:$B0/${V1}{1,2};

+TEST $CLI volume start $V1

+

+EXPECT_WITHIN $PROCESS_UP_TIMEOUT "1" brick_up_status $V1 $H0 $B0/${V1}1

+EXPECT_WITHIN $PROCESS_UP_TIMEOUT "1" brick_up_status $V1 $H0 $B0/${V1}2

+TEST kill_brick $V1 $H0 $B0/${V1}1

+

+#add-brick should fail

+TEST ! $CLI volume add-brick $V1 replica 2 $H0:$B0/${V1}{3,4}

+

+TEST $CLI volume start $V1 force

+EXPECT_WITHIN $PROCESS_UP_TIMEOUT "1" brick_up_status $V1 $H0 $B0/${V1}1

+EXPECT_WITHIN $PROCESS_UP_TIMEOUT "1" brick_up_status $V1 $H0 $B0/${V1}2

+TEST $CLI volume add-brick $V1 replica 2 $H0:$B0/${V1}{3,4}

+cleanup;

diff --git a/xlators/mgmt/glusterd/src/glusterd-brick-ops.c b/xlators/mgmt/glusterd/src/glusterd-brick-ops.c

index 5791e98..bd6025d 100644

--- a/xlators/mgmt/glusterd/src/glusterd-brick-ops.c

+++ b/xlators/mgmt/glusterd/src/glusterd-brick-ops.c

@@ -1724,6 +1724,25 @@ glusterd_op_stage_add_brick (dict_t *dict, char **op_errstr, dict_t *rsp_dict)

                 }

         }

+        if (volinfo->replica_count < replica_count) {

+                cds_list_for_each_entry (brickinfo, &volinfo->bricks,

+                                         brick_list) {

+                        if (gf_uuid_compare (brickinfo->uuid, MY_UUID))

+                                continue;

+                        if (brickinfo->status == GF_BRICK_STOPPED) {

+                                ret = -1;

+                                snprintf (msg, sizeof (msg), "Brick %s is down,"

+                                          " changing replica count needs all "

+                                          "the bricks to be up to avoid data "

+                                          "loss", brickinfo->path);

+                                gf_msg (THIS->name, GF_LOG_ERROR, 0,

+                                        GD_MSG_BRICK_ADD_FAIL, "%s", msg);

+                                *op_errstr = gf_strdup (msg);

+                                goto out;

+                        }

+                }

+        }

+

         if (conf->op_version > GD_OP_VERSION_3_7_5 &&

             is_origin_glusterd (dict)) {

                 ret = glusterd_validate_quorum (this, GD_OP_ADD_BRICK, dict,

-- 

2.9.3