MDEV-28423 continuation: Galera progress reporting + rolling updatesbb-10.6-MDEV-28423-v2

A new feature to support progress reporting (including progress
reporting in the JSON format) that was added in 10.9 can crash the
rolling update scenario if the old version donor node does not pass
the "total" tag in the MAGIC_FILE to the joiner node. The "grep"
utility that is used to extract this tag in this case will return
an empty string, which will then be substituted into the "if"
statement, resulting in a syntactically invalid construct.

This commit corrects this issue.

Also, when using the previous fixes (from MDEV-28423 and from
MDEV-28593), authentication errors are possible in case of mixing
versions at the opposite sides - if the donor is newer than joiner,
due to incorrect reading of the new MAGIC_FILE format by older node
versions (from the joiners side). This problem is also fixed here,
taking advantage of the fact that older versions are still able
to read GUID coordinates even if multiple strings are passed to
them at the end.

In addition, the new progress reporting exchanges the length of
the transmitted data between the donor and the joiner before the
start of the transfer, but does not use this information on the
joiner side, which can lead to incorrect calculation of progress
by the joiner (not synchronous with calculations from the donor
side) if from the joiner side reporting works in compatibility
mode with older versions. This commit moves the construction
of the command line for progress reporting to a point where
we already know the size and can use that knowledge.

Also, to facilitate SST script support in the future, the
script code now has been made unified between all versions
from 10.3 to 10.9.

5 files changed, 169 insertions, 82 deletions

diff --git a/scripts/wsrep_sst_common.sh b/scripts/wsrep_sst_common.sh
index 9d69ddf63a1..6b9a31881a9 100644
--- a/scripts/wsrep_sst_common.sh
+++ b/scripts/wsrep_sst_common.sh
@@ -80,6 +80,7 @@ to_minuses()
 }
 
 WSREP_SST_OPT_BYPASS=0
+WSREP_SST_OPT_PROGRESS=0
 WSREP_SST_OPT_BINLOG=""
 WSREP_SST_OPT_BINLOG_INDEX=""
 WSREP_SST_OPT_LOG_BASENAME=""
@@ -187,6 +188,10 @@ case "$1" in
     '--bypass')
         readonly WSREP_SST_OPT_BYPASS=1
         ;;
+    '--progress')
+        readonly WSREP_SST_OPT_PROGRESS=$(( $2 ))
+        shift
+        ;;
     '--datadir')
         # Let's remove the trailing slash:
         readonly WSREP_SST_OPT_DATA=$(trim_dir "$2")
@@ -258,7 +263,7 @@ case "$1" in
         shift
         ;;
     '--port')
-        readonly WSREP_SST_OPT_PORT="$2"
+        readonly WSREP_SST_OPT_PORT=$(( $2 ))
         shift
         ;;
     '--role')
@@ -531,6 +536,8 @@ else
     readonly WSREP_SST_OPT_ROLE='donor'
 fi
 
+readonly WSREP_SST_OPT_PROGRESS
+
 # The same argument can be present on the command line several
 # times, in this case we must take its last value:
 if [ -n "${MYSQLD_OPT_INNODB_DATA_HOME_DIR:-}" -a \
diff --git a/scripts/wsrep_sst_mariabackup.sh b/scripts/wsrep_sst_mariabackup.sh
index 8027245d364..ab6b4e8a37f 100644
--- a/scripts/wsrep_sst_mariabackup.sh
+++ b/scripts/wsrep_sst_mariabackup.sh
@@ -86,15 +86,12 @@ encrypt_threads=""
 encrypt_chunk=""
 
 readonly SECRET_TAG='secret'
+readonly TOTAL_TAG='total'
 
 # Required for backup locks
 # For backup locks it is 1 sent by joiner
 sst_ver=1
 
-if [ -n "$(commandex pv)" ] && pv --help | grep -qw -F -- '-F'; then
-    pvopts="$pvopts $pvformat"
-fi
-pcmd="pv $pvopts"
 declare -a RC
 
 BACKUP_BIN=$(commandex 'mariabackup')
@@ -121,18 +118,19 @@ timeit()
 
     if [ $ttime -eq 1 ]; then
         x1=$(date +%s)
-        wsrep_log_info "Evaluating $cmd"
-        eval "$cmd"
-        extcode=$?
+    fi
+
+    wsrep_log_info "Evaluating $cmd"
+    eval $cmd
+    extcode=$?
+
+    if [ $ttime -eq 1 ]; then
         x2=$(date +%s)
         took=$(( x2-x1 ))
         wsrep_log_info "NOTE: $stage took $took seconds"
         totime=$(( totime+took ))
-    else
-        wsrep_log_info "Evaluating $cmd"
-        eval "$cmd"
-        extcode=$?
     fi
+
     return $extcode
 }
 
@@ -419,44 +417,90 @@ get_transfer()
 get_footprint()
 {
     cd "$DATA_DIR"
-    payload=$(find . -regex '.*\.ibd$\|.*\.MYI$\|.*\.MYD$\|.*ibdata1$' \
-              -type f -print0 | du --files0-from=- --block-size=1 -c -s | \
-              awk 'END { print $1 }')
+    local payload_data=$(find . \
+        -regex '.*undo[0-9]+$\|.*\.ibd$\|.*\.MYI$\|.*\.MYD$\|.*ibdata1$' \
+        -type f -print0 | du --files0-from=- --block-size=1 -c -s | \
+        awk 'END { print $1 }')
+
+    local payload_undo=0
+    if [ -n "$ib_undo_dir" -a -d "$ib_undo_dir" ]; then
+        cd "$ib_undo_dir"
+        payload_undo=$(find . -regex '.*undo[0-9]+$' -type f -print0 | \
+            du --files0-from=- --block-size=1 -c -s | awk 'END { print $1 }')
+    fi
+    cd "$OLD_PWD"
+
+    wsrep_log_info \
+        "SST footprint estimate: data: $payload_data, undo: $payload_undo"
+
+    payload=$(( payload_data + payload_undo ))
+
     if [ "$compress" != 'none' ]; then
         # QuickLZ has around 50% compression ratio
         # When compression/compaction used, the progress is only an approximate.
         payload=$(( payload*1/2 ))
     fi
-    cd "$OLD_PWD"
-    pcmd="$pcmd -s $payload"
+
+    if [ $WSREP_SST_OPT_PROGRESS -eq 1 ]; then
+        # report to parent the total footprint of the SST
+        echo "$TOTAL_TAG $payload"
+    fi
+
     adjust_progress
 }
 
 adjust_progress()
 {
-    if [ -z "$(commandex pv)" ]; then
-        wsrep_log_error "pv not found in path: $PATH"
-        wsrep_log_error "Disabling all progress/rate-limiting"
-        pcmd=""
-        rlimit=""
-        progress=""
-        return
-    fi
+    pcmd=""
+    rcmd=""
 
-    if [ -n "$progress" -a "$progress" != '1' ]; then
-        if [ -e "$progress" ]; then
-            pcmd="$pcmd 2>>'$progress'"
-        else
-            pcmd="$pcmd 2>'$progress'"
-        fi
-    elif [ -z "$progress" -a -n "$rlimit" ]; then
-            # When rlimit is non-zero
-            pcmd='pv -q'
-    fi
+    [ $progress = 'none' ] && return
 
+    rlimitopts=""
     if [ -n "$rlimit" -a "$WSREP_SST_OPT_ROLE" = 'donor' ]; then
         wsrep_log_info "Rate-limiting SST to $rlimit"
-        pcmd="$pcmd -L \$rlimit"
+        rlimitopts=" -L $rlimit"
+    fi
+
+    if [ -n "$progress" ]; then
+
+        # Backward compatibility: user-configured progress output
+        pcmd="pv $pvopts$rlimitopts"
+
+        if [ -z "${PV_FORMAT+x}" ]; then
+           PV_FORMAT=0
+           pv --help | grep -qw -F -- '-F' && PV_FORMAT=1
+        fi
+        if [ $PV_FORMAT -eq 1 ]; then
+            pcmd="$pcmd $pvformat"
+        fi
+
+        if [ $payload -ne 0 ]; then
+            pcmd="$pcmd -s $payload"
+        fi
+
+        if [ "$progress" != '1' ]; then
+            if [ -e "$progress" ]; then
+                pcmd="$pcmd 2>>'$progress'"
+            else
+                pcmd="$pcmd 2>'$progress'"
+            fi
+        fi
+
+    elif [ $WSREP_SST_OPT_PROGRESS -eq 1 ]; then
+
+        # Default progress output parseable by parent
+        pcmd="pv -f -i 1 -n -b$rlimitopts"
+
+        # read progress data, add tag and post to stdout
+        # for the parent
+        rcmd="stdbuf -oL tr '\r' '\n' | xargs -n1 echo complete"
+
+    elif [ -n "$rlimitopts" ]; then
+
+        # Rate-limiting only, when rlimit is non-zero
+        pcmd="pv -q$rlimitopts"
+
     fi
 }
 
@@ -766,18 +810,27 @@ recv_joiner()
             exit 32
         fi
 
-        # Select the "secret" tag whose value does not start
-        # with a slash symbol. All new tags must to start with
-        # the space and the slash symbol after the word "secret" -
-        # to be removed by older versions of the SST scripts:
-        SECRET=$(grep -m1 -E "^$SECRET_TAG[[:space:]]+[^/]" \
-                      -- "$MAGIC_FILE" || :)
-        # Check donor supplied secret:
-        SECRET=$(trim_string "${SECRET#$SECRET_TAG}")
-        if [ "$SECRET" != "$MY_SECRET" ]; then
-            wsrep_log_error "Donor does not know my secret!"
-            wsrep_log_info "Donor: '$SECRET', my: '$MY_SECRET'"
-            exit 32
+        if [ -n "$MY_SECRET" ]; then
+            # Check donor supplied secret:
+            SECRET=$(grep -m1 -E "^$SECRET_TAG[[:space:]]" "$MAGIC_FILE" || :)
+            SECRET=$(trim_string "${SECRET#$SECRET_TAG}")
+            if [ "$SECRET" != "$MY_SECRET" ]; then
+                wsrep_log_error "Donor does not know my secret!"
+                wsrep_log_info "Donor: '$SECRET', my: '$MY_SECRET'"
+                exit 32
+            fi
+        fi
+
+        if [ $WSREP_SST_OPT_PROGRESS -eq 1 ]; then
+            # check total SST footprint
+            payload=$(grep -m1 -E "^$TOTAL_TAG[[:space:]]" "$MAGIC_FILE" || :)
+            if [ -n "$payload" ]; then
+                payload=$(trim_string "${payload#$TOTAL_TAG}")
+                if [ $payload -ge 0 ]; then
+                    # report to parent
+                    echo "$TOTAL_TAG $payload"
+                fi
+            fi
         fi
     fi
 }
@@ -826,6 +879,14 @@ monitor_process()
 read_cnf
 setup_ports
 
+if [ "$progress" = 'none' ]; then
+    wsrep_log_info "All progress/rate-limiting disabled by configuration"
+elif [ -z "$(commandex pv)" ]; then
+    wsrep_log_info "Progress reporting tool pv not found in path: $PATH"
+    wsrep_log_info "Disabling all progress/rate-limiting"
+    progress='none'
+fi
+
 if "$BACKUP_BIN" --help 2>/dev/null | grep -qw -F -- '--version-check'; then
     disver=' --no-version-check'
 fi
@@ -981,6 +1042,14 @@ if [ "$WSREP_SST_OPT_ROLE" = 'donor' ]; then
 
         check_extra
 
+        if [ -n "$progress" -o $WSREP_SST_OPT_PROGRESS -eq 1 ]; then
+            wsrep_log_info "Estimating total transfer size"
+            get_footprint
+            wsrep_log_info "To transfer: $payload"
+        else
+            adjust_progress
+        fi
+
         wsrep_log_info "Streaming GTID file before SST"
 
         # Store donor's wsrep GTID (state ID) and wsrep_gtid_domain_id
@@ -992,6 +1061,11 @@ if [ "$WSREP_SST_OPT_ROLE" = 'donor' ]; then
             echo "$SECRET_TAG $WSREP_SST_OPT_REMOTE_PSWD" >> "$MAGIC_FILE"
         fi
 
+        if [ $WSREP_SST_OPT_PROGRESS -eq 1 ]; then
+            # Tell joiner what to expect:
+            echo "$TOTAL_TAG $payload" >> "$MAGIC_FILE"
+        fi
+
         ttcmd="$tcmd"
 
         if [ -n "$scomp" ]; then
@@ -1008,12 +1082,14 @@ if [ "$WSREP_SST_OPT_ROLE" = 'donor' ]; then
         # Restore the transport commmand to its original state
         tcmd="$ttcmd"
 
-        if [ -n "$progress" ]; then
-            get_footprint
-            tcmd="$pcmd | $tcmd"
-        elif [ -n "$rlimit" ]; then
-            adjust_progress
-            tcmd="$pcmd | $tcmd"
+        if [ -n "$pcmd" ]; then
+            if [ -n "$rcmd" ]; then
+                # redirect pv stderr to rcmd for tagging and output to parent
+                tcmd="{ $pcmd 2>&3 | $tcmd; } 3>&1 | $rcmd"
+            else
+                # use user-configured pv output
+                tcmd="$pcmd | $tcmd"
+            fi
         fi
 
         wsrep_log_info "Sleeping before data transfer for SST"
@@ -1215,13 +1291,6 @@ else # joiner
         MY_SECRET="" # for check down in recv_joiner()
     fi
 
-    trap cleanup_at_exit EXIT
-
-    if [ -n "$progress" ]; then
-        adjust_progress
-        tcmd="$tcmd | $pcmd"
-    fi
-
     get_keys
     if [ $encrypt -eq 1 ]; then
         strmcmd="$ecmd | $strmcmd"
@@ -1233,6 +1302,8 @@ else # joiner
 
     check_sockets_utils
 
+    trap cleanup_at_exit EXIT
+
     STATDIR="$(mktemp -d)"
     MAGIC_FILE="$STATDIR/$INFO_FILE"
 
@@ -1246,6 +1317,17 @@ else # joiner
 
     if [ ! -r "$STATDIR/$IST_FILE" ]; then
 
+        adjust_progress
+        if [ -n "$pcmd" ]; then
+            if [ -n "$rcmd" ]; then
+                # redirect pv stderr to rcmd for tagging and output to parent
+                strmcmd="{ $pcmd 2>&3 | $strmcmd; } 3>&1 | $rcmd"
+            else
+                # use user-configured pv output
+                strmcmd="$pcmd | $strmcmd"
+            fi
+        fi
+
         if [ -d "$DATA/.sst" ]; then
             wsrep_log_info \
                 "WARNING: Stale temporary SST directory:" \
@@ -1266,7 +1348,7 @@ else # joiner
             cd "$DATA"
             wsrep_log_info "Cleaning the old binary logs"
             # If there is a file with binlogs state, delete it:
-            [ -f "$binlog_base.state" ] && rm "$binlog_base.state" >&2
+            # [ -f "$binlog_base.state" ] && rm "$binlog_base.state" >&2
             # Clean up the old binlog files and index:
             if [ -f "$binlog_index" ]; then
                 while read bin_file || [ -n "$bin_file" ]; do
@@ -1336,16 +1418,14 @@ else # joiner
 
             dcmd="xargs -n 2 qpress -dT$nproc"
 
-            if [ -n "$progress" ] && \
+            if [ -n "$progress" -a "$progress" != 'none' ] && \
                pv --help | grep -qw -F -- '--line-mode'
             then
-                count=$(find "$DATA" -type f -name '*.qp' | wc -l)
+                count=$(find "$DATA" -maxdepth 1 -type f -name '*.qp' | wc -l)
                 count=$(( count*2 ))
-                pvopts="-f -s $count -l -N Decompression"
-                if pv --help | grep -qw -F -- '-F'; then
-                    pvopts="$pvopts -F '%N => Rate:%r Elapsed:%t %e Progress: [%b/$count]'"
-                fi
-                pcmd="pv $pvopts"
+                pvopts='-f -l -N Decompression'
+                pvformat="-F '%N => Rate:%r Elapsed:%t %e Progress: [%b/$count]'"
+                payload=$count
                 adjust_progress
                 dcmd="$pcmd | $dcmd"
             fi
@@ -1443,7 +1523,7 @@ else # joiner
     fi
 
     # Remove special tags from the magic file, and from the output:
-    coords=$(grep -v -E "^$SECRET_TAG[[:space:]]" -- "$MAGIC_FILE")
+    coords=$(head -n1 "$MAGIC_FILE")
     wsrep_log_info "Galera co-ords from recovery: $coords"
     echo "$coords" # Output : UUID:seqno wsrep_gtid_domain_id
 
diff --git a/scripts/wsrep_sst_rsync.sh b/scripts/wsrep_sst_rsync.sh
index 4207c951ddb..acd5ea1626f 100644
--- a/scripts/wsrep_sst_rsync.sh
+++ b/scripts/wsrep_sst_rsync.sh
@@ -318,7 +318,7 @@ if [ -n "$SSLMODE" -a "$SSLMODE" != 'DISABLED' ]; then
 fi
 
 readonly SECRET_TAG='secret'
-readonly BYPASS_TAG='secret /bypass'
+readonly BYPASS_TAG='bypass'
 
 SST_PID="$WSREP_SST_OPT_DATA/wsrep_sst.pid"
 
@@ -829,13 +829,8 @@ EOF
     fi
 
     if [ -n "$MY_SECRET" ]; then
-        # Select the "secret" tag whose value does not start
-        # with a slash symbol. All new tags must to start with
-        # the space and the slash symbol after the word "secret" -
-        # to be removed by older versions of the SST scripts:
-        SECRET=$(grep -m1 -E "^$SECRET_TAG[[:space:]]+[^/]" \
-                      -- "$MAGIC_FILE" || :)
         # Check donor supplied secret:
+        SECRET=$(grep -m1 -E "^$SECRET_TAG[[:space:]]" "$MAGIC_FILE" || :)
         SECRET=$(trim_string "${SECRET#$SECRET_TAG}")
         if [ "$SECRET" != "$MY_SECRET" ]; then
             wsrep_log_error "Donor does not know my secret!"
@@ -845,7 +840,7 @@ EOF
     fi
 
     if [ $WSREP_SST_OPT_BYPASS -eq 0 ]; then
-        if grep -m1 -qE "^$BYPASS_TAG([[:space:]]+.*)?\$" -- "$MAGIC_FILE"
+        if grep -m1 -qE "^$BYPASS_TAG([[:space:]]+.*)?\$" "$MAGIC_FILE"
         then
             readonly WSREP_SST_OPT_BYPASS=1
             readonly WSREP_TRANSFER_TYPE='IST'
@@ -930,7 +925,7 @@ EOF
     fi
 
     # Remove special tags from the magic file, and from the output:
-    coords=$(grep -v -E "^$SECRET_TAG[[:space:]]" -- "$MAGIC_FILE")
+    coords=$(head -n1 "$MAGIC_FILE")
     wsrep_log_info "Galera co-ords from recovery: $coords"
     echo "$coords" # Output : UUID:seqno wsrep_gtid_domain_id
 fi
diff --git a/sql/wsrep_sst.cc b/sql/wsrep_sst.cc
index d2f74e57559..ca40bc79386 100644
--- a/sql/wsrep_sst.cc
+++ b/sql/wsrep_sst.cc
@@ -1069,12 +1069,14 @@ static ssize_t sst_prepare_other (const char*  method,
                  WSREP_SST_OPT_ADDR " '%s' "
                  WSREP_SST_OPT_DATA " '%s' "
                  "%s"
-                 WSREP_SST_OPT_PARENT " '%d'"
+                 WSREP_SST_OPT_PARENT " %d "
+                 WSREP_SST_OPT_PROGRESS " %d"
                  "%s"
                  "%s",
                  method, addr_in, mysql_real_data_home,
                  wsrep_defaults_file,
                  (int)getpid(),
+                 0,
                  binlog_opt_val, binlog_index_opt_val);
 
   my_free(binlog_opt_val);
@@ -1854,16 +1856,18 @@ static int sst_donate_other (const char*        method,
                  "wsrep_sst_%s "
                  WSREP_SST_OPT_ROLE " 'donor' "
                  WSREP_SST_OPT_ADDR " '%s' "
-                 WSREP_SST_OPT_LPORT " '%u' "
+                 WSREP_SST_OPT_LPORT " %u "
                  WSREP_SST_OPT_SOCKET " '%s' "
+                 WSREP_SST_OPT_PROGRESS " %d "
                  WSREP_SST_OPT_DATA " '%s' "
                  "%s"
                  WSREP_SST_OPT_GTID " '%s:%lld' "
-                 WSREP_SST_OPT_GTID_DOMAIN_ID " '%d'"
+                 WSREP_SST_OPT_GTID_DOMAIN_ID " %d"
                  "%s"
                  "%s"
                  "%s",
                  method, addr, mysqld_port, mysqld_unix_port,
+                 0,
                  mysql_real_data_home,
                  wsrep_defaults_file,
                  uuid_oss.str().c_str(), gtid.seqno().get(), wsrep_gtid_server.domain_id,
diff --git a/sql/wsrep_sst.h b/sql/wsrep_sst.h
index 2389db4abe7..462db7a159e 100644
--- a/sql/wsrep_sst.h
+++ b/sql/wsrep_sst.h
@@ -32,6 +32,7 @@
 #define WSREP_SST_OPT_PARENT   "--parent"
 #define WSREP_SST_OPT_BINLOG   "--binlog"
 #define WSREP_SST_OPT_BINLOG_INDEX "--binlog-index"
+#define WSREP_SST_OPT_PROGRESS "--progress"
 #define WSREP_SST_OPT_MYSQLD   "--mysqld-args"
 
 // mysqldump-specific options