[SCM] Samba Shared Repository - branch master updated
Martin Schwenke
martins at samba.org
Tue Jun 25 04:25:02 UTC 2024
The branch, master has been updated
via 415f9f07456 ctdb-failover: Split statd_callout add-client/del-client
via 089aec2885f ctdb-doc: Drop unnecessary, broken attempt at rpc.statd stack trace
via 707e0ef55b8 ctdb-scripts: Fail monitoring after 1 x NFS-Ganesha not running
via 4766d4568bf ctdb-doc: Add example for NFS-Ganesha RPC checking
via d96078e263e ctdb-scripts: Implement NFS statistics retrieval for NFS-Ganesha
via 5b7d17d44d9 ctdb-scripts: Add service_stats_command variable to NFS checks
from d86f9ff7fdd smbd: Simplify callers of notify_filter_string
https://git.samba.org/?p=samba.git;a=shortlog;h=master
- Log -----------------------------------------------------------------
commit 415f9f07456e3fd24063e7508d8b2553df020c21
Author: Martin Schwenke <mschwenke at ddn.com>
Date: Fri May 10 11:42:26 2024 +1000
ctdb-failover: Split statd_callout add-client/del-client
rpc.statd is single-threaded and runs its HA callout synchronously. If
it is too slow then latency accumulates and rpc.statd's backlog grows.
Running a pair of add-client/del-client events with the current code
averages ~0.030s in my test environment. This mean that 1000 clients
reclaiming locks after failover can easily cause 10s of latency. This
could cause rpc.statd to become unresponsive, resulting in a time out
for an rpcinfo-based health check of the status service.
Split the add-client/del-client events out to a standalone
statd_callout executable, written in C, to be used as the HA callout
for rpc.statd. All other functions move to statd_callout_helper.
Now, running a pair of add-client/del-client events in my test
environment averages only ~0.002s. This seems less likely to cause
latency problems.
The standalone statd_callout executable needs to read a configuration
file, which is generated by statd_callout_helper from the "startup"
event. It also needs access to a list of currently assigned public
IPs.
For backward compatibility, during installation a symlink is created
from $CTDB_BASE/statd-callout to the new statd_callout, which is
installed in the helper directory.
Testing this as part of the eventscript unit tests starts to become
even more of a hack than it used to be. However, the dependency on
stubs and the corresponding setup of fake state makes it hard to move
this elsewhere.
Signed-off-by: Martin Schwenke <mschwenke at ddn.com>
Reviewed-by: Amitay Isaacs <amitay at gmail.com>
Autobuild-User(master): Martin Schwenke <martins at samba.org>
Autobuild-Date(master): Tue Jun 25 04:24:57 UTC 2024 on atb-devel-224
commit 089aec2885fecce4fa202ccc402688c394774dc9
Author: Martin Schwenke <mschwenke at ddn.com>
Date: Mon Mar 4 14:29:41 2024 +1100
ctdb-doc: Drop unnecessary, broken attempt at rpc.statd stack trace
There is a typo here, since there will be no process called "status".
Instead of fixing it, drop this because rpc.statd isn't the focus of
this monitoring check and when systemd is init rpc.statd isn't
restarted with nfs-ganesha. It stays running, so a confusing stack
trace for rpc.statd is always logged.
Signed-off-by: Martin Schwenke <mschwenke at ddn.com>
Reviewed-by: Amitay Isaacs <amitay at gmail.com>
commit 707e0ef55b89e328cadd0f4ac3ad33a83039e287
Author: Martin Schwenke <mschwenke at ddn.com>
Date: Fri May 24 11:55:59 2024 +1000
ctdb-scripts: Fail monitoring after 1 x NFS-Ganesha not running
If ganesha.nfsd is gone then a node can't provide an NFS service, so
should be marked unhealthy. A later restart may bring it back to
health.
Signed-off-by: Martin Schwenke <mschwenke at ddn.com>
Reviewed-by: Amitay Isaacs <amitay at gmail.com>
commit 4766d4568bf45d74cbf8d41f9853d0afef3e4c45
Author: Martin Schwenke <mschwenke at ddn.com>
Date: Mon Mar 4 14:28:11 2024 +1100
ctdb-doc: Add example for NFS-Ganesha RPC checking
This one does an rpcinfo check, along with statistics mitigation. It
can be used in combination with the existing 20.nfs_ganesha.check.
The equivalent kernel NFS file only restarts every 10 failures. This
one can be a little more proactive given that false positives are less
likely with the statistics mitigation.
Signed-off-by: Martin Schwenke <mschwenke at ddn.com>
Reviewed-by: Amitay Isaacs <amitay at gmail.com>
commit d96078e263ebc3119d646ddf154582351052c5b0
Author: Martin Schwenke <mschwenke at ddn.com>
Date: Mon Mar 4 13:52:10 2024 +1100
ctdb-scripts: Implement NFS statistics retrieval for NFS-Ganesha
Simplicity is preferred here over absolute correctness. If the
ganesha_stats command exits with an error or times out then no output
is produced so, implicitly, the statistics do not change. Also, the
statistics always change at startup. However, it is likely that the
statistics change when NFS makes progress and do not change when NFS
does not make progress.
Signed-off-by: Martin Schwenke <mschwenke at ddn.com>
Reviewed-by: Amitay Isaacs <amitay at gmail.com>
commit 5b7d17d44d9abdf2f3e15b225bd53b6d21ac41dd
Author: Martin Schwenke <mschwenke at ddn.com>
Date: Mon Feb 19 21:42:11 2024 +1100
ctdb-scripts: Add service_stats_command variable to NFS checks
When monitoring an RPC service, the rpcinfo command might time out
even though the service is making progress. In this case, it is just
slow, so counting the timeout as a failure and potentially restarting
the service will not help. The problem is determining if a service is
making progress.
Add a new NFS checks service_stats_command. This command is intended
to run a statistics command. The output is naively compared using
cmp(1). If the output changes then rpcinfo failures are converted to
successes.
Signed-off-by: Martin Schwenke <mschwenke at ddn.com>
Reviewed-by: Amitay Isaacs <amitay at gmail.com>
-----------------------------------------------------------------------
Summary of changes:
ctdb/config/README | 5 -
ctdb/config/events/legacy/60.nfs.script | 49 ++-
ctdb/config/functions | 2 +-
ctdb/doc/examples/20.nfs_ganesha.check | 4 +-
ctdb/doc/examples/21.nfs.check | 7 +
ctdb/doc/examples/README | 3 +-
ctdb/doc/examples/nfs-ganesha-callout | 26 +-
ctdb/failover/statd_callout.c | 327 +++++++++++++++++++++
ctdb/tests/UNIT/eventscripts/60.nfs.monitor.115.sh | 26 ++
ctdb/tests/UNIT/eventscripts/60.nfs.monitor.116.sh | 26 ++
ctdb/tests/UNIT/eventscripts/scripts/60.nfs.sh | 49 ++-
ctdb/tests/UNIT/eventscripts/scripts/local.sh | 43 +--
.../UNIT/eventscripts/scripts/statd-callout.sh | 60 +++-
ctdb/tests/UNIT/eventscripts/stubs/rm | 2 +-
.../UNIT/eventscripts/stubs/statd_callout_helper | 6 +
ctdb/tests/UNIT/shellcheck/base_scripts.sh | 3 +-
ctdb/tests/UNIT/shellcheck/ctdb_helpers.sh | 3 +-
.../statd-callout => tools/statd_callout_helper} | 46 +--
ctdb/wscript | 15 +-
19 files changed, 613 insertions(+), 89 deletions(-)
create mode 100644 ctdb/doc/examples/21.nfs.check
create mode 100644 ctdb/failover/statd_callout.c
create mode 100755 ctdb/tests/UNIT/eventscripts/60.nfs.monitor.115.sh
create mode 100755 ctdb/tests/UNIT/eventscripts/60.nfs.monitor.116.sh
create mode 100755 ctdb/tests/UNIT/eventscripts/stubs/statd_callout_helper
rename ctdb/{config/statd-callout => tools/statd_callout_helper} (89%)
Changeset truncated at 500 lines:
diff --git a/ctdb/config/README b/ctdb/config/README
index d28f4f0fc60..3fd1583ecee 100644
--- a/ctdb/config/README
+++ b/ctdb/config/README
@@ -14,11 +14,6 @@ Selected highlights:
Support functions, sourced by eventscripts and other scripts.
- statd-callout
-
- rpc.statd high-availability callout to support lock migration on
- failover.
-
Notes:
* All of these scripts are written in POSIX Bourne shell. Please
diff --git a/ctdb/config/events/legacy/60.nfs.script b/ctdb/config/events/legacy/60.nfs.script
index 6935ad9fadc..246a856bca8 100755
--- a/ctdb/config/events/legacy/60.nfs.script
+++ b/ctdb/config/events/legacy/60.nfs.script
@@ -21,8 +21,8 @@ service_reconfigure()
# Restart lock manager, notify clients
# shellcheck disable=SC2317
# Called indirectly via check_thresholds()
- if [ -x "${CTDB_BASE}/statd-callout" ]; then
- "${CTDB_BASE}/statd-callout" notify &
+ if [ -x "${CTDB_HELPER_BINDIR}/statd_callout_helper" ] ; then
+ "${CTDB_HELPER_BINDIR}/statd_callout_helper" notify &
fi >/dev/null 2>&1
}
@@ -80,6 +80,13 @@ nfs_check_services()
# traces of threads that have not exited, since
# they may be stuck doing I/O;
# no default, see also function program_stack_traces()
+# * service_stats_cmd - command to retrieve statistics for given service;
+# if this is set and RPC checks fail (or
+# $service_check_cmd fails), then statistics are
+# compared (using cmp) to see if the service is
+# making progress or is truly hung;
+# no default, failed service does not double-check
+# failure using statistics
#
# Quoting in values is not preserved
#
@@ -103,6 +110,7 @@ nfs_check_service()
service_start_cmd=""
service_check_cmd=""
service_debug_cmd=""
+ service_stats_cmd=""
# Eval line-by-line. Expands variable references in values.
# Also allows variable name checking, which seems useful.
@@ -113,7 +121,8 @@ nfs_check_service()
family=* | version=* | \
unhealthy_after=* | restart_every=* | \
service_stop_cmd=* | service_start_cmd=* | \
- service_check_cmd=* | service_debug_cmd=*)
+ service_check_cmd=* | service_debug_cmd=* | \
+ service_stats_cmd=*)
eval "$_line"
;;
@@ -144,6 +153,30 @@ nfs_check_service()
fi
fi
+ if [ -n "$service_stats_cmd" ]; then
+ # If configured, always update stats,
+ # regardless of RPC status...
+
+ # shellcheck disable=SC2154
+ # script_state_dir set by ctdb_setup_state_dir
+ _curr="${script_state_dir}/stats_${_progname}.out"
+ _prev="${_curr}.prev"
+
+ if [ -f "$_curr" ]; then
+ mv -f "$_curr" "$_prev"
+ fi
+ eval "$service_stats_cmd" >"$_curr" 2>&1
+
+ if ! $_ok &&
+ ! cmp "$_prev" "$_curr" >/dev/null 2>&1; then
+ # Stats always implicitly change on
+ # the first monitor event, since
+ # previous stats don't exists...
+ echo "WARNING: statistics changed but ${_err}"
+ _ok=true
+ fi
+ fi
+
if $_ok; then
if [ $unhealthy_after -ne 1 ] ||
[ $restart_every -ne 0 ]; then
@@ -248,12 +281,12 @@ nfs_check_rpcinfo()
}
##################################################################
-# use statd-callout to update NFS lock info
+# use statd_callout_helper to update NFS lock info
##################################################################
nfs_update_lock_info()
{
- if [ -x "$CTDB_BASE/statd-callout" ]; then
- "$CTDB_BASE/statd-callout" update
+ if [ -x "$CTDB_HELPER_BINDIR/statd_callout_helper" ] ; then
+ "$CTDB_HELPER_BINDIR/statd_callout_helper" update
fi
}
@@ -265,8 +298,8 @@ nfs_callout_init "$script_state_dir"
case "$1" in
startup)
- if [ -x "${CTDB_BASE}/statd-callout" ] ; then
- "${CTDB_BASE}/statd-callout" startup
+ if [ -x "${CTDB_HELPER_BINDIR}/statd_callout_helper" ] ; then
+ "${CTDB_HELPER_BINDIR}/statd_callout_helper" startup
fi
nfs_callout "$@" || exit $?
diff --git a/ctdb/config/functions b/ctdb/config/functions
index fbb1e284020..ef79dbf2162 100755
--- a/ctdb/config/functions
+++ b/ctdb/config/functions
@@ -270,7 +270,7 @@ ctdb_get_ip_address()
}
# Cache of public IP addresses assigned to this node. This function
-# exists mainly so statd-callout does not need to talk to ctdbd, so
+# exists mainly so statd_callout does not need to talk to ctdbd, so
# can be run as non-root, but it may be used in other places. This
# must be updated/refreshed on failover. This is done in
# 10.interface, but doing it in "ipreallocated" isn't enough because
diff --git a/ctdb/doc/examples/20.nfs_ganesha.check b/ctdb/doc/examples/20.nfs_ganesha.check
index 3288f16f93c..92112aff384 100644
--- a/ctdb/doc/examples/20.nfs_ganesha.check
+++ b/ctdb/doc/examples/20.nfs_ganesha.check
@@ -1,8 +1,6 @@
# nfs_ganesha
restart_every=2
-unhealthy_after=6
+unhealthy_after=1
service_stop_cmd="$CTDB_NFS_CALLOUT stop nfs"
service_start_cmd="$CTDB_NFS_CALLOUT start nfs"
service_check_cmd="$CTDB_NFS_CALLOUT check nfs"
-# Ganesha initscript restarts rpc.statd and stack trace is desirable!
-service_debug_cmd="program_stack_traces status 5"
diff --git a/ctdb/doc/examples/21.nfs.check b/ctdb/doc/examples/21.nfs.check
new file mode 100644
index 00000000000..0a230e7886a
--- /dev/null
+++ b/ctdb/doc/examples/21.nfs.check
@@ -0,0 +1,7 @@
+# nfs_ganesha RPC
+restart_every=6
+unhealthy_after=2
+service_stop_cmd="$CTDB_NFS_CALLOUT stop nfs"
+service_start_cmd="$CTDB_NFS_CALLOUT start nfs"
+service_stats_cmd="$CTDB_NFS_CALLOUT stats nfs"
+service_debug_cmd="program_stack_traces ganesha.nfsd"
diff --git a/ctdb/doc/examples/README b/ctdb/doc/examples/README
index a4ea2228b50..02f552aee32 100644
--- a/ctdb/doc/examples/README
+++ b/ctdb/doc/examples/README
@@ -7,9 +7,10 @@ Sample CTDB configuration files:
o 91.lvs.options - Options for the 91.lvs event script
Sample 60.nfs configuration for NFS ganesha - callout script and
-.check file
+.check files
o nfs-ganesha-callout
o 20.nfs_ganesha.check
+ o 21.nfs.check
See the comment at the top of nfs-ganesha-callout for instructions.
diff --git a/ctdb/doc/examples/nfs-ganesha-callout b/ctdb/doc/examples/nfs-ganesha-callout
index 8b7df88456e..74c3ad5c1f9 100755
--- a/ctdb/doc/examples/nfs-ganesha-callout
+++ b/ctdb/doc/examples/nfs-ganesha-callout
@@ -15,9 +15,9 @@
# copy of) this script, making sure it is executable.
#
# * Create a new directory alongside the nfs-checks.d directory, for
-# example nfs-checks-ganesha.d. Install 20.nfs-ganesha.check in
-# this directory. Symlink to any other check files from
-# nfs-checks.d that should still be used, such as
+# example nfs-checks-ganesha.d. Install 20.nfs_ganesha.check and
+# 21.nfs.check in this directory. Symlink to any other check files
+# from nfs-checks.d that should still be used, such as
# 00.portmapper.check. Set CTDB_NFS_CHECKS_DIR to point to this new
# directory of check files.
#
@@ -73,7 +73,7 @@ usage()
_c=$(basename "$0")
cat <<EOF
usage: $_c { shutdown | startup }
- $_c { stop | start | check } nfs
+ $_c { stop | start | check | stats } nfs
$_c { releaseip | takeip } <iface> <ip> <maskbits>
$_c { monitor-list-shares }
EOF
@@ -266,6 +266,22 @@ service_check()
return 0
}
+nfs_stats()
+{
+ _service="$1"
+
+ case "$_service" in
+ nfs)
+ timeout -v 5 ganesha_stats | grep '^Total NFSv.* ops:'
+ ;;
+ *)
+ # This will never change, so is intentionally
+ # unhelpful for avoiding an unhealthy service
+ echo "Not implemented" >&2
+ exit 1
+ esac
+}
+
#-------------------------------------------------
grace_period()
@@ -411,6 +427,7 @@ startup
stop
start
check
+stats
startipreallocate
releaseip
takeip
@@ -429,6 +446,7 @@ startup) nfs_startup ;;
stop) service_stop "$1" ;;
start) service_start "$1" ;;
check) service_check "$1" ;;
+stats) nfs_stats "$1" ;;
startipreallocate) nfs_startipreallocate ;;
releaseip) nfs_releaseip "$@" ;;
takeip) nfs_takeip "$@" ;;
diff --git a/ctdb/failover/statd_callout.c b/ctdb/failover/statd_callout.c
new file mode 100644
index 00000000000..9a85cad7fe2
--- /dev/null
+++ b/ctdb/failover/statd_callout.c
@@ -0,0 +1,327 @@
+/*
+ CTDB NFSv3 rpc.statd HA callout
+
+ Copyright 2023, DataDirect Networks, Inc. All rights reserved.
+ Author: Martin Schwenke <mschwenke at ddn.com>
+
+ This program is free software; you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation; either version 3 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program; if not, see <http://www.gnu.org/licenses/>.
+*/
+
+#include <limits.h>
+#include <stdbool.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <time.h>
+#include <unistd.h>
+
+/*
+ * A configuration file, created by statd_callout_helper, containing
+ * at least 1 line of text.
+ *
+ * The first line is the mode. Currently supported modes are:
+ *
+ * persistent_db
+ *
+ * In this mode, the file contains 2 subsequent lines of text:
+ *
+ * path: directory where files should be created
+ * ips_file: file containing node's currently assigned public IP addresses
+ */
+#define CONFIG_FILE CTDB_VARDIR "/scripts/statd_callout.conf"
+
+static const char *progname;
+
+struct {
+ enum {
+ CTDB_SC_MODE_PERSISTENT_DB,
+ } mode;
+ union {
+ struct {
+ char *path;
+ char *ips_file;
+ };
+ };
+} config;
+
+static bool getline_strip(char **restrict lineptr,
+ size_t *n,
+ FILE *restrict stream)
+{
+ bool was_null;
+ int ret;
+
+ was_null = *lineptr == NULL;
+
+ ret = getline(lineptr, n, stream);
+ if (ret == -1 || ret <= 2) {
+ if (was_null) {
+ free(*lineptr);
+ *lineptr = NULL;
+ *n = 0;
+ }
+ return false;
+ }
+
+ if ((*lineptr)[ret - 1] == '\n') {
+ (*lineptr)[ret - 1] = '\0';
+ }
+
+ return true;
+}
+
+static void free_config(void)
+{
+ switch (config.mode) {
+ case CTDB_SC_MODE_PERSISTENT_DB:
+ free(config.path);
+ config.path = NULL;
+ free(config.ips_file);
+ config.ips_file = NULL;
+ }
+}
+
+static void read_config(void)
+{
+ const char *config_file = NULL;
+ FILE *f = NULL;
+ char *mode = NULL;
+ size_t n = 0;
+ bool status;
+
+ /* For testing only */
+ config_file = getenv("CTDB_STATD_CALLOUT_CONFIG_FILE");
+ if (config_file == NULL || strlen(config_file) == 0) {
+ config_file = CONFIG_FILE;
+ }
+
+ f = fopen(config_file, "r");
+ if (f == NULL) {
+ fprintf(stderr,
+ "%s: unable to open config file (%s)\n",
+ progname,
+ config_file);
+ exit(1);
+ }
+
+ status = getline_strip(&mode, &n, f);
+ if (!status) {
+ fprintf(stderr,
+ "%s: error parsing mode in %s\n",
+ progname,
+ config_file);
+ exit(1);
+ }
+ if (strcmp(mode, "persistent_db") == 0) {
+ config.mode = CTDB_SC_MODE_PERSISTENT_DB;
+ } else {
+ fprintf(stderr,
+ "%s: unknown mode=%s in %s\n",
+ progname,
+ mode,
+ config_file);
+ free(mode);
+ exit(1);
+ }
+ free(mode);
+
+ switch (config.mode) {
+ case CTDB_SC_MODE_PERSISTENT_DB:
+ status = getline_strip(&config.path, &n, f);
+ if (!status) {
+ goto parse_error;
+ }
+
+ status = getline_strip(&config.ips_file, &n, f);
+ if (!status) {
+ goto parse_error;
+ }
+
+ break;
+ }
+
+ fclose(f);
+ return;
+
+parse_error:
+ fprintf(stderr,
+ "%s: error parsing contents of %s\n",
+ progname,
+ config_file);
+ free_config();
+ exit(1);
+}
+
+static void for_each_sip(void (*line_func)(const char *sip, const char *cip),
+ const char *cip)
+{
+ FILE *f = NULL;
+ char *line = NULL;
+ size_t n = 0;
+
+ f = fopen(config.ips_file, "r");
+ if (f == NULL) {
+ fprintf(stderr,
+ "%s: unable to open IPs file (%s)\n",
+ progname,
+ config.ips_file);
+ exit(1);
+ }
+
+ for (;;) {
+ bool status;
+
+ status = getline_strip(&line, &n, f);
+ if (!status) {
+ if (!feof(f)) {
+ fprintf(stderr,
+ "%s: error parsing contents of %s\n",
+ progname,
+ config.ips_file);
+ free(line);
+ exit(1);
+ }
+ break;
+ }
+
+ line_func(line, cip);
+ }
+
+ free(line);
+ fclose(f);
+ return;
+}
+
+static void make_path(char *buf,
+ size_t num,
+ const char *sip,
+ const char *cip)
+{
+ int ret = snprintf(buf,
+ num,
+ "%s/statd-state@%s@%s",
+ config.path,
+ sip,
+ cip);
+ if (ret < 0) {
+ /* Not possible for snprintf(3)? */
+ fprintf(stderr,
+ "%s: error constructing path %s/statd-state@%s@%s\n",
+ progname,
+ config.path,
+ sip,
+ cip);
+ exit(1);
+ }
+ if ((size_t)ret >= num) {
+ fprintf(stderr,
+ "%s: path too long %s/statd-state@%s@%s\n",
+ progname,
+ config.path,
+ sip,
+ cip);
+ exit(1);
+ }
+}
+
+static void add_client_persistent_db_line(const char *sip, const char *cip)
+{
+ char path[PATH_MAX];
+ FILE *f;
+ long long datetime;
+
+ make_path(path, sizeof(path), sip, cip);
+
+ datetime = (long long)time(NULL);
+
+ f = fopen(path, "w");
+ if (f == NULL) {
+ fprintf(stderr,
+ "%s: unable to open for writing %s\n",
+ progname,
--
Samba Shared Repository
More information about the samba-cvs
mailing list