From 3cf776c49bc60b7f616a4c503a8b10b2d19ad04b Mon Sep 17 00:00:00 2001 From: Niels de Vos Date: Sun, 21 Jun 2015 15:07:58 +0200 Subject: nfs: make it possible to disable nfs.mount-rmtab When there are many NFS-clients doing very often mount/unmount actions, the updating of the 'rmtab' can become a bottleneck and cause delays. In these situations, the output of 'showmount' may be less important than the responsiveness of the (un)mounting. By setting 'nfs.mount-rmtab' to the value "/-", the cache file is not updated anymore, and the entries are only kept in memory. Cherry picked from commit 40407afb529f6e5fa2f79e9778c2f527122d75eb: > Cherry picked from commit 331ef6e1a86bfc0a93f8a9dec6ad35c417873849: >> BUG: 1169317 >> Change-Id: I40c4d8d754932f86fb2b1b2588843390464c773d >> Reported-by: Cyril Peponnet >> Signed-off-by: Niels de Vos >> Reviewed-on: http://review.gluster.org/9223 >> Tested-by: Gluster Build System >> Reviewed-by: soumya k >> Reviewed-by: jiffin tony Thottan >> Reviewed-by: Kaleb KEITHLEY > > This change also contains the fixes to the test-case from: >> >> nfs: fix spurious failure in bug-1166862.t >> >> In some environments, "showmount" could return an NFS-client that does >> not start with "1". This would cause the test-case to fail. The check is >> incorrect, the number of lines should get counted instead. >> >> Also moving the test-case to the .../nfs/... subdirectory. >> >> Cherry picked from commit ee9b35a780607daddc2832b9af5ed6bf414aebc0: >> BUG: 1166862 >> Change-Id: Ic03aa8145ca57d78aea01564466e924b03bb302a >> Signed-off-by: Niels de Vos >> Reviewed-on: http://review.gluster.org/10419 >> Tested-by: Gluster Build System >> Reviewed-by: Vijay Bellur >> > > Change-Id: I40c4d8d754932f86fb2b1b2588843390464c773d > BUG: 1215385 > Signed-off-by: Niels de Vos > Reviewed-on: http://review.gluster.org/10379 > Tested-by: NetBSD Build System > Tested-by: Gluster Build System > Reviewed-by: Vijay Bellur GLUSTERD_WORKDIR has been added to tests/include.rc and is not configurable through ./configure like on newer branches. It is not suitable to change the GlusterD working directory in an update for a stable release. Change-Id: I40c4d8d754932f86fb2b1b2588843390464c773d BUG: 1166862 Signed-off-by: Niels de Vos Reviewed-on: http://review.gluster.org/11336 Tested-by: Gluster Build System Reviewed-by: jiffin tony Thottan Reviewed-by: Kaleb KEITHLEY --- libglusterfs/src/store.c | 2 +- libglusterfs/src/store.h | 2 +- tests/bugs/nfs/bug-1166862.t | 66 +++++++++++++++++++++++++ tests/include.rc | 1 + xlators/nfs/server/src/mount3.c | 105 ++++++++++++++++++++++++---------------- xlators/nfs/server/src/nfs.c | 17 ++++++- 6 files changed, 146 insertions(+), 47 deletions(-) create mode 100755 tests/bugs/nfs/bug-1166862.t diff --git a/libglusterfs/src/store.c b/libglusterfs/src/store.c index 48c79ee0289..f21585a14cd 100644 --- a/libglusterfs/src/store.c +++ b/libglusterfs/src/store.c @@ -350,7 +350,7 @@ out: } int32_t -gf_store_handle_new (char *path, gf_store_handle_t **handle) +gf_store_handle_new (const char *path, gf_store_handle_t **handle) { int32_t ret = -1; gf_store_handle_t *shandle = NULL; diff --git a/libglusterfs/src/store.h b/libglusterfs/src/store.h index 337103ff73e..04950448743 100644 --- a/libglusterfs/src/store.h +++ b/libglusterfs/src/store.h @@ -72,7 +72,7 @@ int32_t gf_store_save_value (int fd, char *key, char *value); int32_t -gf_store_handle_new (char *path, gf_store_handle_t **handle); +gf_store_handle_new (const char *path, gf_store_handle_t **handle); int gf_store_handle_retrieve (char *path, gf_store_handle_t **handle); diff --git a/tests/bugs/nfs/bug-1166862.t b/tests/bugs/nfs/bug-1166862.t new file mode 100755 index 00000000000..c8f63d50b0c --- /dev/null +++ b/tests/bugs/nfs/bug-1166862.t @@ -0,0 +1,66 @@ +#!/bin/bash +# +# When nfs.mount-rmtab is disabled, it should not get updated. +# +# Based on: bug-904065.t +# + +# count the lines of a file, return 0 if the file does not exist +function count_lines() +{ + if [ -n "$1" ] + then + $@ 2>/dev/null | wc -l + else + echo 0 + fi +} + + +. $(dirname $0)/../../include.rc +. $(dirname $0)/../../nfs.rc +. $(dirname $0)/../../volume.rc + +cleanup + +TEST glusterd +TEST pidof glusterd + +TEST $CLI volume create $V0 $H0:$B0/brick1 +EXPECT 'Created' volinfo_field $V0 'Status' + +TEST $CLI volume start $V0; +EXPECT 'Started' volinfo_field $V0 'Status' + +# glusterfs/nfs needs some time to start up in the background +EXPECT_WITHIN $NFS_EXPORT_TIMEOUT 1 is_nfs_export_available + +# disable the rmtab by settting it to the magic "/-" value +TEST $CLI volume set $V0 nfs.mount-rmtab /- + +# before mounting the rmtab should be empty +EXPECT '0' count_lines cat $GLUSTERD_WORKDIR/nfs/rmtab + +TEST mount_nfs $H0:/$V0 $N0 nolock +EXPECT '0' count_lines cat $GLUSTERD_WORKDIR/nfs/rmtab + +# showmount should list one client +EXPECT '1' count_lines showmount --no-headers $H0 + +# unmount +EXPECT_WITHIN $UMOUNT_TIMEOUT "Y" force_umount $N0 + +# after resetting the option, the rmtab should get updated again +TEST $CLI volume reset $V0 nfs.mount-rmtab + +# before mounting the rmtab should be empty +EXPECT '0' count_lines cat $GLUSTERD_WORKDIR/nfs/rmtab + +TEST mount_nfs $H0:/$V0 $N0 nolock +EXPECT '2' count_lines cat $GLUSTERD_WORKDIR/nfs/rmtab + +# removing a mount +EXPECT_WITHIN $UMOUNT_TIMEOUT "Y" force_umount $N0 +EXPECT '0' count_lines cat $GLUSTERD_WORKDIR/nfs/rmtab + +cleanup diff --git a/tests/include.rc b/tests/include.rc index 8043d6090ad..fc0ad6cc2b5 100644 --- a/tests/include.rc +++ b/tests/include.rc @@ -7,6 +7,7 @@ B0=${B0:=/d/backends}; # top level of brick directories H0=${H0:=`hostname --fqdn`}; # hostname DEBUG=${DEBUG:=0} # turn on debugging? statedumpdir=`gluster --print-statedumpdir`; # Default directory for statedump +GLUSTERD_WORKDIR=/var/lib/glusterd # hardcoded for 3.5-stable CLI="gluster --mode=script"; CC=cc diff --git a/xlators/nfs/server/src/mount3.c b/xlators/nfs/server/src/mount3.c index 6c5f80db3b5..b314f8ea9da 100644 --- a/xlators/nfs/server/src/mount3.c +++ b/xlators/nfs/server/src/mount3.c @@ -353,6 +353,25 @@ fail: gf_store_unlink_tmppath (sh); } +static gf_boolean_t +mount_open_rmtab (const char *rmtab, gf_store_handle_t **sh) +{ + int ret = -1; + + /* updating the rmtab is disabled, use in-memory only */ + if (!rmtab || rmtab[0] == '\0') + return _gf_false; + + ret = gf_store_handle_new (rmtab, sh); + if (ret) { + gf_log (GF_MNT, GF_LOG_WARNING, "Failed to open '%s'", rmtab); + return _gf_false; + } + + return _gf_true; +} + + /* Read the rmtab into a clean ms->mountlist. */ static void @@ -360,16 +379,13 @@ mount_read_rmtab (struct mount3_state *ms) { gf_store_handle_t *sh = NULL; struct nfs_state *nfs = NULL; - int ret; + gf_boolean_t read_rmtab = _gf_false; nfs = (struct nfs_state *)ms->nfsx->private; - ret = gf_store_handle_new (nfs->rmtab, &sh); - if (ret) { - gf_log (GF_MNT, GF_LOG_WARNING, "Failed to open '%s'", - nfs->rmtab); + read_rmtab = mount_open_rmtab (nfs->rmtab, &sh); + if (!read_rmtab) return; - } if (gf_store_lock (sh)) { gf_log (GF_MNT, GF_LOG_WARNING, "Failed to lock '%s'", @@ -389,6 +405,7 @@ out: * The rmtab could be empty, or it can exists and have been updated by a * different storage server without our knowing. * + * 0. if opening the nfs->rmtab fails, return gracefully * 1. takes the store_handle lock on the current rmtab * - blocks if an other storage server rewrites the rmtab at the same time * 2. [if new_rmtab] takes the store_handle lock on the new rmtab @@ -407,17 +424,15 @@ mount_rewrite_rmtab (struct mount3_state *ms, char *new_rmtab) struct nfs_state *nfs = NULL; int ret; char *rmtab = NULL; + gf_boolean_t got_old_rmtab = _gf_false; nfs = (struct nfs_state *)ms->nfsx->private; - ret = gf_store_handle_new (nfs->rmtab, &sh); - if (ret) { - gf_log (GF_MNT, GF_LOG_WARNING, "Failed to open '%s'", - nfs->rmtab); + got_old_rmtab = mount_open_rmtab (nfs->rmtab, &sh); + if (!got_old_rmtab && !new_rmtab) return; - } - if (gf_store_lock (sh)) { + if (got_old_rmtab && gf_store_lock (sh)) { gf_log (GF_MNT, GF_LOG_WARNING, "Not rewriting '%s'", nfs->rmtab); goto free_sh; @@ -439,7 +454,8 @@ mount_rewrite_rmtab (struct mount3_state *ms, char *new_rmtab) } /* always read the currently used rmtab */ - __mount_read_rmtab (sh, &ms->mountlist, _gf_true); + if (got_old_rmtab) + __mount_read_rmtab (sh, &ms->mountlist, _gf_true); if (new_rmtab) { /* read the new rmtab and write changes to the new location */ @@ -466,9 +482,11 @@ free_nsh: if (new_rmtab) gf_store_handle_destroy (nsh); unlock_sh: - gf_store_unlock (sh); + if (got_old_rmtab) + gf_store_unlock (sh); free_sh: - gf_store_handle_destroy (sh); + if (got_old_rmtab) + gf_store_handle_destroy (sh); } /* Add a new NFS-client to the ms->mountlist and update the rmtab if we can. @@ -492,6 +510,7 @@ mnt3svc_update_mountlist (struct mount3_state *ms, rpcsvc_request_t *req, char *colon = NULL; struct nfs_state *nfs = NULL; gf_store_handle_t *sh = NULL; + gf_boolean_t update_rmtab = _gf_false; if ((!ms) || (!req) || (!expname)) return -1; @@ -503,12 +522,7 @@ mnt3svc_update_mountlist (struct mount3_state *ms, rpcsvc_request_t *req, nfs = (struct nfs_state *)ms->nfsx->private; - ret = gf_store_handle_new (nfs->rmtab, &sh); - if (ret) { - gf_log (GF_MNT, GF_LOG_WARNING, "Failed to open '%s'", - nfs->rmtab); - goto free_err; - } + update_rmtab = mount_open_rmtab (nfs->rmtab, &sh); strncpy (me->exname, expname, MNTPATHLEN); @@ -518,7 +532,7 @@ mnt3svc_update_mountlist (struct mount3_state *ms, rpcsvc_request_t *req, */ ret = rpcsvc_transport_peername (req->trans, me->hostname, MNTPATHLEN); if (ret == -1) - goto free_err2; + goto free_err; colon = strrchr (me->hostname, ':'); if (colon) { @@ -527,10 +541,10 @@ mnt3svc_update_mountlist (struct mount3_state *ms, rpcsvc_request_t *req, LOCK (&ms->mountlock); { /* in case locking fails, we just don't write the rmtab */ - if (gf_store_lock (sh)) { + if (update_rmtab && gf_store_lock (sh)) { gf_log (GF_MNT, GF_LOG_WARNING, "Failed to lock '%s'" ", changes will not be written", nfs->rmtab); - } else { + } else if (update_rmtab) { __mount_read_rmtab (sh, &ms->mountlist, _gf_false); } @@ -545,19 +559,19 @@ mnt3svc_update_mountlist (struct mount3_state *ms, rpcsvc_request_t *req, list_add_tail (&me->mlist, &ms->mountlist); /* only write the rmtab in case it was locked */ - if (gf_store_locked_local (sh)) + if (update_rmtab && gf_store_locked_local (sh)) __mount_rewrite_rmtab (ms, sh); } dont_add: - if (gf_store_locked_local (sh)) + if (update_rmtab && gf_store_locked_local (sh)) gf_store_unlock (sh); UNLOCK (&ms->mountlock); -free_err2: - gf_store_handle_destroy (sh); - free_err: + if (update_rmtab) + gf_store_handle_destroy (sh); + if (ret == -1) GF_FREE (me); @@ -1589,27 +1603,25 @@ mnt3svc_umount (struct mount3_state *ms, char *dirpath, char *hostname) int ret = -1; gf_store_handle_t *sh = NULL; struct nfs_state *nfs = NULL; + gf_boolean_t update_rmtab = _gf_false; if ((!ms) || (!dirpath) || (!hostname)) return -1; nfs = (struct nfs_state *)ms->nfsx->private; - ret = gf_store_handle_new (nfs->rmtab, &sh); - if (ret) { - gf_log (GF_MNT, GF_LOG_WARNING, "Failed to open '%s'", - nfs->rmtab); - return 0; - } - - ret = gf_store_lock (sh); - if (ret) { - goto out_free; + update_rmtab = mount_open_rmtab (nfs->rmtab, &sh); + if (update_rmtab) { + ret = gf_store_lock (sh); + if (ret) + goto out_free; } LOCK (&ms->mountlock); { - __mount_read_rmtab (sh, &ms->mountlist, _gf_false); + if (update_rmtab) + __mount_read_rmtab (sh, &ms->mountlist, _gf_false); + if (list_empty (&ms->mountlist)) { ret = 0; goto out_unlock; @@ -1641,13 +1653,20 @@ mnt3svc_umount (struct mount3_state *ms, char *dirpath, char *hostname) list_del (&me->mlist); GF_FREE (me); - __mount_rewrite_rmtab (ms, sh); + + if (update_rmtab) + __mount_rewrite_rmtab (ms, sh); } out_unlock: UNLOCK (&ms->mountlock); - gf_store_unlock (sh); + + if (update_rmtab) + gf_store_unlock (sh); + out_free: - gf_store_handle_destroy (sh); + if (update_rmtab) + gf_store_handle_destroy (sh); + return ret; } diff --git a/xlators/nfs/server/src/nfs.c b/xlators/nfs/server/src/nfs.c index 928754917c1..1be6caa7b5e 100644 --- a/xlators/nfs/server/src/nfs.c +++ b/xlators/nfs/server/src/nfs.c @@ -881,6 +881,12 @@ nfs_init_state (xlator_t *this) gf_log (GF_NFS, GF_LOG_ERROR, "Failed to parse dict"); goto free_foppool; } + + /* check if writing the rmtab is disabled*/ + if (nfs->rmtab && strcmp ("/-", nfs->rmtab) == 0) { + GF_FREE (nfs->rmtab); + nfs->rmtab = NULL; + } } /* support both options rpc-auth.ports.insecure and @@ -1079,7 +1085,13 @@ nfs_reconfigure_state (xlator_t *this, dict_t *options) } gf_path_strip_trailing_slashes (optstr); } - if (strcmp (nfs->rmtab, optstr) != 0) { + /* check if writing the rmtab is disabled*/ + if (strcmp ("/-", optstr) == 0) { + GF_FREE (nfs->rmtab); + nfs->rmtab = NULL; + gf_log (GF_NFS, GF_LOG_INFO, + "Disabled writing of nfs.mount-rmtab"); + } else if (!nfs->rmtab || strcmp (nfs->rmtab, optstr) != 0) { mount_rewrite_rmtab (nfs->mstate, optstr); gf_log (GF_NFS, GF_LOG_INFO, "Reconfigured nfs.mount-rmtab path: %s", @@ -1817,7 +1829,8 @@ struct volume_options options[] = { "list all the NFS-clients that have connected " "through the MOUNT protocol. If this is on shared " "storage, all GlusterFS servers will update and " - "output (with 'showmount') the same list." + "output (with 'showmount') the same list. Set to " + "\"/-\" to disable." }, { .key = {OPT_SERVER_AUX_GIDS}, .type = GF_OPTION_TYPE_BOOL, -- cgit