Whamcloud - gitweb
b=21881 mdt_num_threads tuning
authorNicolas Williams <Nicolas.Williams@Sun.COM>
Thu, 11 Feb 2010 21:26:24 +0000 (15:26 -0600)
committerRobert Read <rread@sun.com>
Thu, 11 Feb 2010 22:37:56 +0000 (14:37 -0800)
Fixes to mdt_handler.c so that it honors mdt_num_threads in the same manner as
ost_handler.c does for oss_num_threads.  Enhancements to the test framework and
new tests (53a and b, to check for thread tunable behavior) for conf-sanity are
included.

i=adilger@sun.com
i=robert.read@sun.com

lustre/include/lustre_net.h
lustre/mdt/mdt_handler.c
lustre/tests/conf-sanity.sh
lustre/tests/functions.sh
lustre/tests/rpc.sh
lustre/tests/test-framework.sh

index c5f3b56..b91459f 100644 (file)
 #define MDT_NUM_THREADS max(min_t(unsigned long, MDT_MAX_THREADS, \
                                   cfs_num_physpages >> (25 - CFS_PAGE_SHIFT)), \
                                   2UL)
-#define FLD_NUM_THREADS max(min_t(unsigned long, MDT_MAX_THREADS, \
-                                  cfs_num_physpages >> (25 - CFS_PAGE_SHIFT)), \
-                                  2UL)
-#define SEQ_NUM_THREADS max(min_t(unsigned long, MDT_MAX_THREADS, \
-                                  cfs_num_physpages >> (25 - CFS_PAGE_SHIFT)), \
-                                  2UL)
 
 /* Absolute limits */
 #define MDS_THREADS_MIN 2
index 56fec6a..ff17a56 100644 (file)
@@ -99,7 +99,9 @@ ldlm_mode_t mdt_dlm_lock_modes[] = {
 /*
  * Initialized in mdt_mod_init().
  */
-unsigned long mdt_num_threads;
+static unsigned long mdt_num_threads;
+static unsigned long mdt_min_threads;
+static unsigned long mdt_max_threads;
 
 /* ptlrpc request handler for MDT. All handlers are
  * grouped into several slices - struct mdt_opc_slice,
@@ -3883,9 +3885,8 @@ static int mdt_start_ptlrpc_service(struct mdt_device *m)
                  * We'd like to have a mechanism to set this on a per-device
                  * basis, but alas...
                  */
-                .psc_min_threads    = min(max(mdt_num_threads, MDT_MIN_THREADS),
-                                          MDT_MAX_THREADS),
-                .psc_max_threads     = MDT_MAX_THREADS,
+                .psc_min_threads     = mdt_min_threads,
+                .psc_max_threads     = mdt_max_threads,
                 .psc_ctx_tags        = LCT_MD_THREAD
         };
 
@@ -3916,9 +3917,8 @@ static int mdt_start_ptlrpc_service(struct mdt_device *m)
                 .psc_req_portal      = MDS_READPAGE_PORTAL,
                 .psc_rep_portal      = MDC_REPLY_PORTAL,
                 .psc_watchdog_factor = MDT_SERVICE_WATCHDOG_FACTOR,
-                .psc_min_threads    = min(max(mdt_num_threads, MDT_MIN_THREADS),
-                                          MDT_MAX_THREADS),
-                .psc_max_threads     = MDT_MAX_THREADS,
+                .psc_min_threads     = mdt_min_threads,
+                .psc_max_threads     = mdt_max_threads,
                 .psc_ctx_tags        = LCT_MD_THREAD
         };
         m->mdt_readpage_service =
@@ -3944,9 +3944,8 @@ static int mdt_start_ptlrpc_service(struct mdt_device *m)
                 .psc_req_portal      = MDS_SETATTR_PORTAL,
                 .psc_rep_portal      = MDC_REPLY_PORTAL,
                 .psc_watchdog_factor = MDT_SERVICE_WATCHDOG_FACTOR,
-                .psc_min_threads   = min(max(mdt_num_threads, MDT_MIN_THREADS),
-                                         MDT_MAX_THREADS),
-                .psc_max_threads     = MDT_MAX_THREADS,
+                .psc_min_threads     = mdt_min_threads,
+                .psc_max_threads     = mdt_max_threads,
                 .psc_ctx_tags        = LCT_MD_THREAD
         };
 
@@ -3975,8 +3974,8 @@ static int mdt_start_ptlrpc_service(struct mdt_device *m)
                 .psc_req_portal      = SEQ_CONTROLLER_PORTAL,
                 .psc_rep_portal      = MDC_REPLY_PORTAL,
                 .psc_watchdog_factor = MDT_SERVICE_WATCHDOG_FACTOR,
-                .psc_min_threads     = SEQ_NUM_THREADS,
-                .psc_max_threads     = SEQ_NUM_THREADS,
+                .psc_min_threads     = mdt_min_threads,
+                .psc_max_threads     = mdt_max_threads,
                 .psc_ctx_tags        = LCT_MD_THREAD|LCT_DT_THREAD
         };
 
@@ -4004,8 +4003,8 @@ static int mdt_start_ptlrpc_service(struct mdt_device *m)
                 .psc_req_portal      = SEQ_METADATA_PORTAL,
                 .psc_rep_portal      = MDC_REPLY_PORTAL,
                 .psc_watchdog_factor = MDT_SERVICE_WATCHDOG_FACTOR,
-                .psc_min_threads     = SEQ_NUM_THREADS,
-                .psc_max_threads     = SEQ_NUM_THREADS,
+                .psc_min_threads     = mdt_min_threads,
+                .psc_max_threads     = mdt_max_threads,
                 .psc_ctx_tags        = LCT_MD_THREAD|LCT_DT_THREAD
         };
 
@@ -4036,8 +4035,8 @@ static int mdt_start_ptlrpc_service(struct mdt_device *m)
                 .psc_req_portal      = SEQ_DATA_PORTAL,
                 .psc_rep_portal      = OSC_REPLY_PORTAL,
                 .psc_watchdog_factor = MDT_SERVICE_WATCHDOG_FACTOR,
-                .psc_min_threads     = SEQ_NUM_THREADS,
-                .psc_max_threads     = SEQ_NUM_THREADS,
+                .psc_min_threads     = mdt_min_threads,
+                .psc_max_threads     = mdt_max_threads,
                 .psc_ctx_tags        = LCT_MD_THREAD|LCT_DT_THREAD
         };
 
@@ -4063,8 +4062,8 @@ static int mdt_start_ptlrpc_service(struct mdt_device *m)
                 .psc_req_portal      = FLD_REQUEST_PORTAL,
                 .psc_rep_portal      = MDC_REPLY_PORTAL,
                 .psc_watchdog_factor = MDT_SERVICE_WATCHDOG_FACTOR,
-                .psc_min_threads     = FLD_NUM_THREADS,
-                .psc_max_threads     = FLD_NUM_THREADS,
+                .psc_min_threads     = mdt_min_threads,
+                .psc_max_threads     = mdt_max_threads,
                 .psc_ctx_tags        = LCT_DT_THREAD|LCT_MD_THREAD
         };
 
@@ -4093,9 +4092,8 @@ static int mdt_start_ptlrpc_service(struct mdt_device *m)
                 .psc_req_portal      = MDS_MDS_PORTAL,
                 .psc_rep_portal      = MDC_REPLY_PORTAL,
                 .psc_watchdog_factor = MDT_SERVICE_WATCHDOG_FACTOR,
-                .psc_min_threads    = min(max(mdt_num_threads, MDT_MIN_THREADS),
-                                          MDT_MAX_THREADS),
-                .psc_max_threads     = MDT_MAX_THREADS,
+                .psc_min_threads     = mdt_min_threads,
+                .psc_max_threads     = mdt_max_threads,
                 .psc_ctx_tags        = LCT_MD_THREAD
         };
         m->mdt_xmds_service =
@@ -5769,7 +5767,19 @@ static int __init mdt_mod_init(void)
 
         llo_local_obj_register(&mdt_last_recv);
 
-        mdt_num_threads = MDT_NUM_THREADS;
+        if (mdt_num_threads > 0) {
+                if (mdt_num_threads > MDT_MAX_THREADS)
+                        mdt_num_threads = MDT_MAX_THREADS;
+                if (mdt_num_threads < MDT_MIN_THREADS)
+                        mdt_num_threads = MDT_MIN_THREADS;
+                mdt_max_threads = mdt_min_threads = mdt_num_threads;
+        } else {
+                mdt_max_threads = MDT_MAX_THREADS;
+                mdt_min_threads = MDT_MIN_THREADS;
+                if (mdt_min_threads < MDT_NUM_THREADS)
+                        mdt_min_threads = MDT_NUM_THREADS;
+        }
+
         lprocfs_mdt_init_vars(&lvars);
         rc = class_register_type(&mdt_obd_device_ops, NULL,
                                  lvars.module_vars, LUSTRE_MDT_NAME,
index 199c73c..1281d3f 100644 (file)
@@ -2212,6 +2212,102 @@ test_52() {
 }
 run_test 52 "check recovering objects from lost+found"
 
+# Checks threads_min/max/started for some service
+#
+# Arguments: service name (OST or MDT), facet (e.g., ost1, $SINGLEMDS), and a
+# parameter pattern prefix like 'ost.*.ost'.
+thread_sanity() {
+        local modname=$1
+        local facet=$2
+        local parampat=$3
+        local opts=$4
+        local tmin
+        local tmin2
+        local tmax
+        local tmax2
+        local tstarted
+        local paramp
+        local msg="Insane $modname thread counts"
+        shift 4
+
+        setup
+        check_mount || return 41
+
+        # We need to expand $parampat, but it may match multiple parameters, so
+        # we'll pick the first one
+        if ! paramp=$(do_facet $facet "lctl get_param -N ${parampat}.threads_min"|head -1); then
+                error "Couldn't expand ${parampat}.threads_min parameter name"
+                return 22
+        fi
+
+        # Remove the .threads_min part
+        paramp=${paramp%.threads_min}
+
+        # Check for sanity in defaults
+        tmin=$(do_facet $facet "lctl get_param -n ${paramp}.threads_min" || echo 0)
+        tmax=$(do_facet $facet "lctl get_param -n ${paramp}.threads_max" || echo 0)
+        tstarted=$(do_facet $facet "lctl get_param -n ${paramp}.threads_started" || echo 0)
+        lassert 23 "$msg (PDSH problems?)" '(($tstarted && $tmin && $tmax))' || return $?
+        lassert 24 "$msg" '(($tstarted >= $tmin && $tstarted <= tmax ))' || return $?
+
+        # Check that we can lower min/max
+        do_facet $facet "lctl set_param ${paramp}.threads_min=$((tmin - 1))"
+        do_facet $facet "lctl set_param ${paramp}.threads_max=$((tmax - 10))"
+        tmin2=$(do_facet $facet "lctl get_param -n ${paramp}.threads_min" || echo 0)
+        tmax2=$(do_facet $facet "lctl get_param -n ${paramp}.threads_max" || echo 0)
+        lassert 25 "$msg" '(($tmin2 == ($tmin - 1) && $tmax2 == ($tmax -10)))' || return $?
+
+        # Check that we can set min/max to the same value
+        do_facet $facet "lctl set_param ${paramp}.threads_min=$tmin"
+        do_facet $facet "lctl set_param ${paramp}.threads_max=$tmin"
+        tmin2=$(do_facet $facet "lctl get_param -n ${paramp}.threads_min" || echo 0)
+        tmax2=$(do_facet $facet "lctl get_param -n ${paramp}.threads_max" || echo 0)
+        lassert 26 "$msg" '(($tmin2 == $tmin && $tmax2 == $tmin))' || return $?
+
+        # Check that we can't set max < min
+        do_facet $facet "lctl set_param ${paramp}.threads_max=$((tmin - 1))"
+        tmin2=$(do_facet $facet "lctl get_param -n ${paramp}.threads_min" || echo 0)
+        tmax2=$(do_facet $facet "lctl get_param -n ${paramp}.threads_max" || echo 0)
+        lassert 27 "$msg" '(($tmin <= $tmax2))' || return $?
+
+        # We need to ensure that we get the module options desired; to do this
+        # we set LOAD_MODULES_REMOTE=true and we call setmodopts below.
+        LOAD_MODULES_REMOTE=true
+        cleanup
+        local oldvalue
+        setmodopts -a $modname "$opts" oldvalue
+
+        load_modules
+        setup
+        check_mount || return 41
+
+        # Restore previous setting of MODOPTS_*
+        setmodopts $modname "$oldvalue"
+
+        # Check that $opts took
+        tmin=$(do_facet $facet "lctl get_param -n ${paramp}.threads_min")
+        tmax=$(do_facet $facet "lctl get_param -n ${paramp}.threads_max")
+        tstarted=$(do_facet $facet "lctl get_param -n ${paramp}.threads_started")
+        lassert 28 "$msg" '(($tstarted == $tmin && $tstarted == $tmax ))' || return $?
+        cleanup
+
+        # Workaround a YALA bug where YALA expects that modules will remain
+        # loaded on the servers
+        LOAD_MODULES_REMOTE=false
+        setup
+        cleanup
+}
+
+test_53a() {
+        thread_sanity OST ost1 'ost.*.ost' 'oss_num_threads=64'
+}
+run_test 53a "check OSS thread count params"
+
+test_53b() {
+        thread_sanity MDT $SINGLEMDS 'mdt.*.*.' 'mdt_num_threads=64'
+}
+run_test 53b "check MDT thread count params"
+
 cleanup_gss
 equals_msg `basename $0`: test complete
 [ -f "$TESTSUITELOG" ] && cat $TESTSUITELOG && grep -q FAIL $TESTSUITELOG && exit 1 || true
index 1dd9ac5..9442145 100644 (file)
@@ -13,6 +13,150 @@ assert_env() {
     [ $failed ] && exit 1 || true
 }
 
+# lrepl - Lustre test Read-Eval-Print Loop.
+#
+# This function implements a REPL for the Lustre test framework.  It
+# doesn't exec an actual shell because the user may want to inspect
+# variables and use functions from the test framework.
+lrepl() {
+    local line
+    local rawline
+    local prompt
+
+    cat <<EOF
+        This is an interactive read-eval-print loop interactive shell
+        simulation that you can use to debug failing tests.  You can
+        enter most bash command lines (see notes below).
+
+        Use this REPL to inspect variables, set them, call test
+        framework shell functions, etcetera.
+
+        'exit' or EOF to exit this shell.
+
+        set \$retcode to 0 to cause the assertion failure that
+        triggered this REPL to be ignored.
+
+        Examples:
+            do_facet ost1 lctl get_param ost.*.ost.threads_*
+            do_rpc_nodes \$OSTNODES unload_modules
+
+        NOTES:
+            All but the last line of multi-line statements or blocks
+            must end in a backslash.
+
+            "Here documents" are not supported.
+
+            History is not supported, but command-line editing is.
+
+EOF
+
+    # Prompt escapes don't work in read -p, sadly.
+    prompt=":test_${testnum:-UNKNOWN}:$(uname -n):$(basename $PWD)% "
+
+    # We use read -r to get close to a shell experience
+    while read -e -r -p "$prompt" rawline; do
+        line=
+        case "$rawline" in
+        # Don't want to exit-exit, just exit the REPL
+        exit) break;;
+        # We need to handle continuations, and read -r doesn't do
+        # that for us.  Yet we need read -r.
+        #
+        # We also use case/esac to compare lines read to "*\\"
+        # because [ "$line" = *\\ ] and variants of that don't work.
+        *\\) line="$rawline"
+            while read -e -r -p '> ' rawline
+            do
+                line="$line"$'\n'"$rawline"
+                case "$rawline" in
+                # We could check for here documents by matching
+                # against *<<*, but who cares.
+                *\\) continue;;
+                *) break;;
+                esac
+            done
+            ;;
+        *) line=$rawline
+        esac
+
+        case "$line" in
+        *\\) break;;
+        esac
+
+        # Finally!  Time to eval.
+        eval "$line"
+    done
+
+    echo $'\n\tExiting interactive shell...\n'
+    return 0
+}
+
+# lassert - Lustre test framework assert
+#
+# Arguments: failure code, failure message, expression/statement
+#
+# lassert evaluates the expression given, and, if false, calls
+# error() to trigger test failure.  If REPL_ON_LASSERT is true then
+# lassert will call lrepl() to give the user an interactive shell.
+# If the REPL sets retcode=0 then the assertion failure will be
+# ignored.
+lassert() {
+    local retcode=$1
+    local msg=$2
+    shift 2
+
+    echo "checking $* ($(eval echo \""$*"\"))..."
+    eval "$@" && return 0;
+
+    if ${REPL_ON_LASSERT:-false}; then
+        echo "Assertion $retcode failed: $* (expanded: $(eval echo \""$*"\"))
+$msg"
+        lrepl
+    fi
+
+    error "Assertion $retcode failed: $* (expanded: $(eval echo \""$*"\"))
+$msg"
+    return $retcode
+}
+
+# setmodopts- set module options for subsequent calls to load_modules
+#
+# Usage: setmodopts module_name new_value [var_in_which_to_save_old_value]
+#        setmodopts -a module_name new_value [var_in_which_to_save_old_value]
+#
+# In the second usage the new value is appended to the old.
+setmodopts() {
+        local _append=false
+
+        if [ "$1" = -a ]; then
+            _append=true
+            shift
+        fi
+
+        local _var=MODOPTS_$1
+        local _newvalue=$2
+        local _savevar=$3
+        local _oldvalue
+
+        # Dynamic naming of variables is a pain in bash.  In ksh93 we could
+        # write "nameref opts_var=${modname}_MODOPTS" then assign directly
+        # to opts_var.  Associative arrays would also help, alternatively.
+        # Alas, we're stuck with eval until all distros move to a more recent
+        # version of bash.  Fortunately we don't need to eval unset and export.
+
+        if [ -z "$_newvalue" ]; then
+            unset $_var
+            return 0
+        fi
+
+        _oldvalue=${!var}
+        $_append && _newvalue="$_oldvalue $_newvalue"
+        export $_var="$_newvalue"
+        echo setmodopts: ${_var}=${_newvalue}
+
+        [ -n "$_savevar" ] && eval $_savevar=\""$_oldvalue"\"
+}
+
 echoerr () { echo "$@" 1>&2 ; }
 
 signaled() {
index e1127d2..d2914b6 100755 (executable)
@@ -9,11 +9,12 @@ if [ ! -f $LUSTRE/tests/rpc.sh ]; then
 fi
 
 . $LUSTRE/tests/test-framework.sh
-init_test_env $@
+init_test_env
 . ${CONFIG:=$LUSTRE/tests/cfg/$NAME.sh}
 
-cmd=$1
-shift
-$cmd $@
+# Reset the trap on ERR set by the framework.  Noticing this failure is the
+# framework's job.
+trap ERR
 
-exit $?
+# Execute the command
+"$@"
index 1f4a50c..be0a9ad 100644 (file)
@@ -34,6 +34,9 @@ if [ -f "$EXCEPT_LIST_FILE" ]; then
     . $EXCEPT_LIST_FILE
 fi
 
+[ -z "$MODPROBECONF" -a -f /etc/modprobe.conf ] && MODPROBECONF=/etc/modprobe.conf
+[ -z "$MODPROBECONF" -a -f /etc/modprobe.d/Lustre ] && MODPROBECONF=/etc/modprobe.d/Lustre
+
 assert_DIR () {
     local failed=""
     [[ $DIR/ = $MOUNT/* ]] || \
@@ -230,27 +233,64 @@ module_loaded () {
    /sbin/lsmod | grep -q $1
 }
 
+# Load a module on the system where this is running.
+#
+# Synopsis: load_module module_name [module arguments for insmod/modprobe]
+#
+# If module arguments are not given but MODOPTS_<MODULE> is set, then its value
+# will be used as the arguments.  Otherwise arguments will be obtained from
+# /etc/modprobe.conf, from /etc/modprobe.d/Lustre, or else none will be used.
+#
 load_module() {
+    local optvar
     EXT=".ko"
     module=$1
     shift
     BASE=`basename $module $EXT`
 
+    # If no module arguments were passed, get them from $MODOPTS_<MODULE>, else from
+    # modprobe.conf
+    if [ $# -eq 0 ]; then
+        # $MODOPTS_<MODULE>; we could use associative arrays, but that's not in
+        # Bash until 4.x, so we resort to eval.
+        optvar="MODOPTS_$(basename $module | tr a-z A-Z)"
+        eval set -- \$$optvar
+        if [ $# -eq 0 -a -n "$MODPROBECONF" ]; then
+            # Nothing in $MODOPTS_<MODULE>; try modprobe.conf
+            set -- $(grep "^options\\s*\<${module}\>" $MODPROBECONF)
+            # Get rid of "options $module"
+           (($# > 0)) && shift 2
+
+            # Ensure we have accept=all for lnet
+            if [ $module = lnet ]; then
+                # OK, this is a bit wordy...
+                local arg accept_all_present=false
+                for arg in "$@"; do
+                    [ "$arg" = accept=all ] && accept_all_present=true
+                done
+                $accept_all_present || set -- "$@" accept=all
+            fi
+        fi
+    fi
+
+    [ $# -gt 0 ] && echo "${module} options: '$*'"
+
     module_loaded ${BASE} && return
 
+    # Note that insmod will ignore anything in modprobe.conf, which is why we're
+    # passing options on the command-line.
     if [ "$BASE" == "lnet_selftest" ] && \
             [ -f ${LUSTRE}/../lnet/selftest/${module}${EXT} ]; then
         insmod ${LUSTRE}/../lnet/selftest/${module}${EXT}
-
     elif [ -f ${LUSTRE}/${module}${EXT} ]; then
-        insmod ${LUSTRE}/${module}${EXT} $@
+        insmod ${LUSTRE}/${module}${EXT} "$@"
     else
         # must be testing a "make install" or "rpm" installation
         # note failed to load ptlrpc_gss is considered not fatal
         if [ "$BASE" == "ptlrpc_gss" ]; then
-            modprobe $BASE $@ 2>/dev/null || echo "gss/krb5 is not supported"
+            modprobe $BASE "$@" 2>/dev/null || echo "gss/krb5 is not supported"
         else
-            modprobe $BASE $@
+            modprobe $BASE "$@"
         fi
     fi
 }
@@ -258,10 +298,12 @@ load_module() {
 load_modules_local() {
     if [ -n "$MODPROBE" ]; then
         # use modprobe
-    return 0
+        echo "Using modprobe to load modules"
+        return 0
     fi
     if [ "$HAVE_MODULES" = true ]; then
-    # we already loaded
+        # we already loaded
+        echo "Modules already loaded"
         return 0
     fi
     HAVE_MODULES=true
@@ -270,15 +312,7 @@ load_modules_local() {
     load_module ../libcfs/libcfs/libcfs
     [ "$PTLDEBUG" ] && lctl set_param debug="$PTLDEBUG"
     [ "$SUBSYSTEM" ] && lctl set_param subsystem_debug="${SUBSYSTEM# }"
-    local MODPROBECONF=
-    [ -f /etc/modprobe.conf ] && MODPROBECONF=/etc/modprobe.conf
-    [ ! "$MODPROBECONF" -a -d /etc/modprobe.d ] && MODPROBECONF=/etc/modprobe.d/Lustre
-    [ -z "$LNETOPTS" -a "$MODPROBECONF" ] && \
-        LNETOPTS=$(awk '/^options lnet/ { print $0}' $MODPROBECONF | sed 's/^options lnet //g')
-    echo $LNETOPTS | grep -q "accept=all"  || LNETOPTS="$LNETOPTS accept=all";
-    echo "lnet options: '$LNETOPTS'"
-    # note that insmod will ignore anything in modprobe.conf
-    load_module ../lnet/lnet/lnet $LNETOPTS
+    load_module ../lnet/lnet/lnet
     LNETLND=${LNETLND:-"socklnd/ksocklnd"}
     load_module ../lnet/klnds/$LNETLND
     load_module lvfs/lvfs
@@ -1505,6 +1539,17 @@ single_local_node () {
    [ "$1" = "$HOSTNAME" ]
 }
 
+# Outputs environment variable assignments that should be passed to remote nodes
+get_env_vars() {
+    local var
+    local value
+
+    for var in ${!MODOPTS_*}; do
+        value=${!var}
+        echo "${var}=\"$value\""
+    done
+}
+
 do_nodes() {
     local verbose=false
     # do not stripe off hostname if verbose, bug 19215
@@ -1537,9 +1582,9 @@ do_nodes() {
     fi
 
     if $verbose ; then
-        $myPDSH $rnodes "(PATH=\$PATH:$RLUSTRE/utils:$RLUSTRE/tests:/sbin:/usr/sbin; cd $RPWD; LUSTRE=\"$RLUSTRE\" sh -c \"$@\")"
+        $myPDSH $rnodes "(PATH=\$PATH:$RLUSTRE/utils:$RLUSTRE/tests:/sbin:/usr/sbin; cd $RPWD; LUSTRE=\"$RLUSTRE\" $(get_env_vars) sh -c \"$@\")"
     else
-        $myPDSH $rnodes "(PATH=\$PATH:$RLUSTRE/utils:$RLUSTRE/tests:/sbin:/usr/sbin; cd $RPWD; LUSTRE=\"$RLUSTRE\" sh -c \"$@\")" | sed -re "s/\w+:\s//g"
+        $myPDSH $rnodes "(PATH=\$PATH:$RLUSTRE/utils:$RLUSTRE/tests:/sbin:/usr/sbin; cd $RPWD; LUSTRE=\"$RLUSTRE\" $(get_env_vars) sh -c \"$@\")" | sed -re "s/\w+:\s//g"
     fi
     return ${PIPESTATUS[0]}
 }
@@ -2429,15 +2474,11 @@ error_noexit() {
 
 error() {
     error_noexit "$@"
-    if $FAIL_ON_ERROR; then
-        reset_fail_loc
-        exit 1
-    fi
+    exit 1
 }
 
 error_exit() {
-    error_noexit "$@"
-    exit 1
+    error "$@"
 }
 
 # use only if we are ignoring failures for this test, bugno required.