summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorAtin Mukherjee <amukherj@redhat.com>2016-03-31 11:01:53 +0530
committerJeff Darcy <jdarcy@redhat.com>2016-04-01 13:38:54 -0700
commit34899d71f21fd2b4c523b68ffb2d7c655c776641 (patch)
tree4a818e42665ba441c71da92c891c3e2c2f4a7f7d
parentd7001f3606866c1d9bb3cefd2767af56833dd53f (diff)
glusterd: Allocate fresh port on brick (re)start
There is no point of using the same port through the entire volume life cycle for a particular bricks process since there is no guarantee that the same port would be free and no other application wouldn't consume it in between the glusterd/volume restart. We hit a race where on glusterd restart the daemon services start followed by brick processes and the time brick process tries to bind with the port which was allocated by glusterd before a restart is been already consumed by some other client like NFS/SHD/... Note : This is a short term solution as here we reduce the race window but don't eliminate it completely. As a long term solution the port allocation has to be done by glusterfsd and the same should be communicated back to glusterd for book keeping Change-Id: Ibbd1e7ca87e51a7cd9cf216b1fe58ef7783aef24 BUG: 1322805 Signed-off-by: Atin Mukherjee <amukherj@redhat.com> Reviewed-on: http://review.gluster.org/13865 Smoke: Gluster Build System <jenkins@build.gluster.com> NetBSD-regression: NetBSD Build System <jenkins@build.gluster.org> CentOS-regression: Gluster Build System <jenkins@build.gluster.com> Reviewed-by: Jeff Darcy <jdarcy@redhat.com>
-rwxr-xr-xtests/bugs/glusterd/1313628-import-brick-ports-always.t47
-rw-r--r--tests/features/ssl-ciphers.t5
-rw-r--r--xlators/mgmt/glusterd/src/glusterd-messages.h10
-rw-r--r--xlators/mgmt/glusterd/src/glusterd-store.c16
-rw-r--r--xlators/mgmt/glusterd/src/glusterd-utils.c38
5 files changed, 46 insertions, 70 deletions
diff --git a/tests/bugs/glusterd/1313628-import-brick-ports-always.t b/tests/bugs/glusterd/1313628-import-brick-ports-always.t
deleted file mode 100755
index d04c4293466..00000000000
--- a/tests/bugs/glusterd/1313628-import-brick-ports-always.t
+++ /dev/null
@@ -1,47 +0,0 @@
-#!/bin/bash
-. $(dirname $0)/../../include.rc
-. $(dirname $0)/../../cluster.rc
-
-## Check that brick ports are always copied on import
-## --------------------------------------------------
-## This test checks that the brick ports are copied on import by checking that
-## they don't change when the following happens,
-## - Stop a volume
-## - Stop glusterd
-## - Start the stopped volume
-## - Start the stopped glusterd
-
-function get_brick_port() {
- local VOL=$1
- local BRICK=$2
- $CLI2 volume status $VOL $BRICK --xml | sed -ne 's/.*<port>\([0-9]*\)<\/port>/\1/p'
-}
-
-
-cleanup
-
-TEST launch_cluster 2
-TEST $CLI1 peer probe $H2
-EXPECT_WITHIN $PROBE_TIMEOUT 1 peer_count
-
-# Create and start volume so that brick port assignment happens
-TEST $CLI1 volume create $V0 $H1:$B1/$V0 $H2:$B2/$V0
-TEST $CLI1 volume start $V0
-
-# Save port for 2nd brick
-BPORT_ORIG=$(get_brick_port $V0 $H2:$B2/$V0)
-
-# Stop volume, stop 2nd glusterd, start volume, start 2nd glusterd
-TEST $CLI1 volume stop $V0
-TEST kill_glusterd 2
-
-TEST $CLI1 volume start $V0
-TEST start_glusterd 2
-EXPECT_WITHIN $PROBE_TIMEOUT 1 peer_count
-
-# Get new port and compare with old one
-EXPECT_WITHIN $PROCESS_UP_TIMEOUT $BPORT_ORIG get_brick_port $V0 $H2:$B2/$V0
-
-$CLI1 volume stop $V0
-
-cleanup
diff --git a/tests/features/ssl-ciphers.t b/tests/features/ssl-ciphers.t
index 9ee7fc6c16f..f5909f320ac 100644
--- a/tests/features/ssl-ciphers.t
+++ b/tests/features/ssl-ciphers.t
@@ -137,6 +137,7 @@ EXPECT "`pwd`/`dirname $0`/dh1024.pem" volume_option $V0 ssl.dh-param
TEST $CLI volume stop $V0
TEST $CLI volume start $V0
EXPECT_WITHIN $CHILD_UP_TIMEOUT "1" online_brick_count
+BRICK_PORT=`brick_port $V0`
EXPECT "Y" openssl_connect -cipher EDH -connect $H0:$BRICK_PORT
# Test the cipher-list option
@@ -145,6 +146,7 @@ EXPECT AES256-SHA volume_option $V0 ssl.cipher-list
TEST $CLI volume stop $V0
TEST $CLI volume start $V0
EXPECT_WITHIN $CHILD_UP_TIMEOUT "1" online_brick_count
+BRICK_PORT=`brick_port $V0`
EXPECT "Y" openssl_connect -cipher AES256-SHA -connect $H0:$BRICK_PORT
EXPECT "N" openssl_connect -cipher AES128-SHA -connect $H0:$BRICK_PORT
@@ -154,6 +156,7 @@ EXPECT EECDH:EDH:!TLSv1 volume_option $V0 ssl.cipher-list
TEST $CLI volume stop $V0
TEST $CLI volume start $V0
EXPECT_WITHIN $CHILD_UP_TIMEOUT "1" online_brick_count
+BRICK_PORT=`brick_port $V0`
EXPECT "N" openssl_connect -cipher AES256-SHA -connect $H0:$BRICK_PORT
EXPECT "Y" openssl_connect -cipher EECDH -connect $H0:$BRICK_PORT
@@ -162,6 +165,7 @@ EXPECT invalid volume_option $V0 ssl.ec-curve
TEST $CLI volume stop $V0
TEST $CLI volume start $V0
EXPECT_WITHIN $CHILD_UP_TIMEOUT "1" online_brick_count
+BRICK_PORT=`brick_port $V0`
EXPECT "N" openssl_connect -cipher EECDH -connect $H0:$BRICK_PORT
TEST $CLI volume set $V0 ssl.ec-curve secp521r1
@@ -169,6 +173,7 @@ EXPECT secp521r1 volume_option $V0 ssl.ec-curve
TEST $CLI volume stop $V0
TEST $CLI volume start $V0
EXPECT_WITHIN $CHILD_UP_TIMEOUT "1" online_brick_count
+BRICK_PORT=`brick_port $V0`
EXPECT "Y" openssl_connect -cipher EECDH -connect $H0:$BRICK_PORT
# test revocation
diff --git a/xlators/mgmt/glusterd/src/glusterd-messages.h b/xlators/mgmt/glusterd/src/glusterd-messages.h
index ffcfa7773e9..04d630001df 100644
--- a/xlators/mgmt/glusterd/src/glusterd-messages.h
+++ b/xlators/mgmt/glusterd/src/glusterd-messages.h
@@ -41,7 +41,7 @@
#define GLUSTERD_COMP_BASE GLFS_MSGID_GLUSTERD
-#define GLFS_NUM_MESSAGES 573
+#define GLFS_NUM_MESSAGES 574
#define GLFS_MSGID_END (GLUSTERD_COMP_BASE + GLFS_NUM_MESSAGES + 1)
/* Messaged with message IDs */
@@ -4633,6 +4633,14 @@
*/
#define GD_MSG_SNAP_RESUME_TIER_FAIL (GLUSTERD_COMP_BASE + 573)
+/*!
+ * @messageid
+ * @diagnosis
+ * @recommendedaction
+ *
+ */
+#define GD_MSG_PMAP_REMOVE_FAIL (GLUSTERD_COMP_BASE + 574)
+
/*------------*/
#define glfs_msg_end_x GLFS_MSGID_END, "Invalid: End of messages"
#endif /* !_GLUSTERD_MESSAGES_H_ */
diff --git a/xlators/mgmt/glusterd/src/glusterd-store.c b/xlators/mgmt/glusterd/src/glusterd-store.c
index b1f943074f3..97f5147dd39 100644
--- a/xlators/mgmt/glusterd/src/glusterd-store.c
+++ b/xlators/mgmt/glusterd/src/glusterd-store.c
@@ -2313,13 +2313,6 @@ glusterd_store_retrieve_bricks (glusterd_volinfo_t *volinfo)
/* This is required to adhere to the
IANA standards */
brickinfo->port = 0;
- } else {
- /* This is required to have proper ports
- assigned to bricks after restart */
- pmap = pmap_registry_get (THIS);
- if (pmap->last_alloc <= brickinfo->port)
- pmap->last_alloc =
- brickinfo->port + 1;
}
} else if (!strncmp (key, GLUSTERD_STORE_KEY_BRICK_RDMA_PORT,
strlen (GLUSTERD_STORE_KEY_BRICK_RDMA_PORT))) {
@@ -2329,16 +2322,7 @@ glusterd_store_retrieve_bricks (glusterd_volinfo_t *volinfo)
/* This is required to adhere to the
IANA standards */
brickinfo->rdma_port = 0;
- } else {
- /* This is required to have proper ports
- assigned to bricks after restart */
- pmap = pmap_registry_get (THIS);
- if (pmap->last_alloc <=
- brickinfo->rdma_port)
- pmap->last_alloc =
- brickinfo->rdma_port +1;
}
-
} else if (!strncmp (key, GLUSTERD_STORE_KEY_BRICK_DECOMMISSIONED,
strlen (GLUSTERD_STORE_KEY_BRICK_DECOMMISSIONED))) {
gf_string2int (value, &brickinfo->decommissioned);
diff --git a/xlators/mgmt/glusterd/src/glusterd-utils.c b/xlators/mgmt/glusterd/src/glusterd-utils.c
index a0cc5d409c8..55597cd39cf 100644
--- a/xlators/mgmt/glusterd/src/glusterd-utils.c
+++ b/xlators/mgmt/glusterd/src/glusterd-utils.c
@@ -1725,9 +1725,22 @@ glusterd_volume_start_glusterfs (glusterd_volinfo_t *volinfo,
if (gf_is_service_running (pidfile, NULL))
goto connect;
- port = brickinfo->port;
- if (!port)
- port = pmap_registry_alloc (THIS);
+ /* Do a pmap registry remove on the older connected port */
+ if (brickinfo->port) {
+ ret = pmap_registry_remove (this, brickinfo->port,
+ brickinfo->path,
+ GF_PMAP_PORT_BRICKSERVER,
+ NULL);
+ if (ret) {
+ gf_msg (this->name, GF_LOG_ERROR, 0,
+ GD_MSG_PMAP_REMOVE_FAIL, "Failed to remove pmap"
+ "registry for older signin port %d",
+ brickinfo->port);
+ goto out;
+ }
+ }
+
+ port = pmap_registry_alloc (THIS);
/* Build the exp_path, before starting the glusterfsd even in
valgrind mode. Otherwise all the glusterfsd processes start
@@ -1790,9 +1803,22 @@ glusterd_volume_start_glusterfs (glusterd_volinfo_t *volinfo,
if (volinfo->transport_type != GF_TRANSPORT_BOTH_TCP_RDMA) {
runner_argprintf (&runner, "%d", port);
} else {
- rdma_port = brickinfo->rdma_port;
- if (!rdma_port)
- rdma_port = pmap_registry_alloc (THIS);
+ /* Do a pmap registry remove on the older connected port */
+ if (brickinfo->rdma_port) {
+ ret = pmap_registry_remove (this, brickinfo->rdma_port,
+ brickinfo->path,
+ GF_PMAP_PORT_BRICKSERVER,
+ NULL);
+ if (ret) {
+ gf_msg (this->name, GF_LOG_ERROR, 0,
+ GD_MSG_PMAP_REMOVE_FAIL, "Failed to "
+ "remove pmap registry for older signin "
+ "port %d", brickinfo->rdma_port);
+ goto out;
+ }
+ }
+ rdma_port = pmap_registry_alloc (THIS);
+
runner_argprintf (&runner, "%d,%d", port, rdma_port);
runner_add_arg (&runner, "--xlator-option");
runner_argprintf (&runner, "%s-server.transport.rdma.listen-port=%d",