MDEV-28423 continuation: Galera progress reporting + rolling updatesbb-10.9-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, 104 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 84e676f32cd..ab6b4e8a37f 100644
--- a/scripts/wsrep_sst_mariabackup.sh
+++ b/scripts/wsrep_sst_mariabackup.sh
@@ -87,7 +87,6 @@ encrypt_chunk=""
 
 readonly SECRET_TAG='secret'
 readonly TOTAL_TAG='total'
-readonly TOTAL_TAG_SST="$SECRET_TAG /$TOTAL_TAG"
 
 # Required for backup locks
 # For backup locks it is 1 sent by joiner
@@ -434,7 +433,7 @@ get_footprint()
     wsrep_log_info \
         "SST footprint estimate: data: $payload_data, undo: $payload_undo"
 
-    payload=$(( $payload_data + $payload_undo ))
+    payload=$(( payload_data + payload_undo ))
 
     if [ "$compress" != 'none' ]; then
         # QuickLZ has around 50% compression ratio
@@ -442,28 +441,20 @@ get_footprint()
         payload=$(( payload*1/2 ))
     fi
 
-    # report to parent the total footprint of the SST
-    echo "$TOTAL_TAG $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 [ "$progress" = 'none' ]; then
-        wsrep_log_info "All progress/rate-limiting disabled by configuration"
-        pcmd=""
-        rlimit=""
-        return
-    fi
+    pcmd=""
+    rcmd=""
 
-    if [ -z "$(commandex pv)" ]; then
-        wsrep_log_info "Progress reporting tool pv not found in path: $PATH"
-        wsrep_log_info "Disabling all progress/rate-limiting"
-        pcmd=""
-        rlimit=""
-        progress='none'
-        return
-    fi
+    [ $progress = 'none' ] && return
 
     rlimitopts=""
     if [ -n "$rlimit" -a "$WSREP_SST_OPT_ROLE" = 'donor' ]; then
@@ -472,17 +463,22 @@ adjust_progress()
     fi
 
     if [ -n "$progress" ]; then
-        # Backward compatibility: user configured progress output
-        if pv --help | grep -qw -F -- '-F'; then
-            pvopts="$pvopts $pvformat"
+
+        # 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
-            pvopts="$pvopts -s $payload"
+            pcmd="$pcmd -s $payload"
         fi
 
-        pcmd="pv $pvopts$rlimitopts"
-
         if [ "$progress" != '1' ]; then
             if [ -e "$progress" ]; then
                 pcmd="$pcmd 2>>'$progress'"
@@ -491,15 +487,20 @@ adjust_progress()
             fi
         fi
 
-        rcmd=':'
-    else
+    elif [ $WSREP_SST_OPT_PROGRESS -eq 1 ]; then
+
         # Default progress output parseable by parent
-        pvopts="-f -i 1 -n -b"
-        pcmd="pv $pvopts$rlimitopts"
+        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
 }
 
@@ -809,27 +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
 
-        # check total SST footprint
-        total=$(grep -m1 -E "^$TOTAL_TAG_SST[[:space:]]" \
-                     -- "$MAGIC_FILE" || :)
-        total=$(trim_string "${total#$TOTAL_TAG_SST}")
-        if [ $total -ge 0 ]; then
-            # report to parent
-            echo "$TOTAL_TAG $total"
+        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
 }
@@ -878,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
@@ -1033,9 +1042,13 @@ if [ "$WSREP_SST_OPT_ROLE" = 'donor' ]; then
 
         check_extra
 
-        wsrep_log_info "Estimating total transfer size"
-        get_footprint
-        wsrep_log_info "To transfer: $payload"
+        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"
 
@@ -1043,14 +1056,16 @@ if [ "$WSREP_SST_OPT_ROLE" = 'donor' ]; then
         # (separated by a space).
         echo "$WSREP_SST_OPT_GTID $WSREP_SST_OPT_GTID_DOMAIN_ID" > "$MAGIC_FILE"
 
-        # Tell joiner what to expect:
-        echo "$TOTAL_TAG_SST $payload" >> "$MAGIC_FILE"
-
         if [ -n "$WSREP_SST_OPT_REMOTE_PSWD" ]; then
             # Let joiner know that we know its secret
             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
@@ -1068,7 +1083,7 @@ if [ "$WSREP_SST_OPT_ROLE" = 'donor' ]; then
         tcmd="$ttcmd"
 
         if [ -n "$pcmd" ]; then
-            if [ "$rcmd" != ':' ]; 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
@@ -1276,8 +1291,6 @@ else # joiner
         MY_SECRET="" # for check down in recv_joiner()
     fi
 
-    trap cleanup_at_exit EXIT
-
     get_keys
     if [ $encrypt -eq 1 ]; then
         strmcmd="$ecmd | $strmcmd"
@@ -1287,19 +1300,10 @@ else # joiner
         strmcmd="$sdecomp | $strmcmd"
     fi
 
-    adjust_progress
-    if [ -n "$pcmd" ]; then
-        if [ "$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
-
     check_sockets_utils
 
+    trap cleanup_at_exit EXIT
+
     STATDIR="$(mktemp -d)"
     MAGIC_FILE="$STATDIR/$INFO_FILE"
 
@@ -1313,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:" \
@@ -1333,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
@@ -1508,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 9f7ec93450a..f8c0be9565f 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 1a3df7c676e..0676ce79dfd 100644
--- a/sql/wsrep_sst.cc
+++ b/sql/wsrep_sst.cc
@@ -1149,12 +1149,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(),
+                 wsrep_debug ? 1 : 0,
                  binlog_opt_val, binlog_index_opt_val);
 
   my_free(binlog_opt_val);
@@ -1957,16 +1959,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,
+                 wsrep_debug ? 1 : 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