Skip to content

M
MariaDB
Commit de0f93fb authored by Julius Goryavsky's avatar Julius Goryavsky
Browse files
Options

MDEV-20420: SST failed after MDEV-18863 in some test configurations 

After applying MDEV-18863, in some test configurations, SST
may fails due to duplication of some parameters (in particular
"--port") in the main part of the command line and after
"--mysqld-args", as well as due to incorrect interpretation
of the parameter "--port" passed after "--mysqld-args" when
the SST script is invoked without explicitly specifying a port
for SST. In addition, it is necessary to correctly handle spaces,
quotation marks and special characters when copying original
arguments from the argv[] array to a new command line (after
"--mysqld-args"). This patch resolves these shortcomings.
parent 4a9fb905
Hide whitespace changes
Inline Side-by-side
Showing with 324 additions and 57 deletions
scripts/wsrep_sst_common.sh
View file @ de0f93fb
......@@ -166,7 +166,11 @@ case "$1" in
shift
while [ $# -gt 0 ]; do
option=${1%%=*}
if [ "$option" != "--defaults-file" ]; then
if [[ "$option" != "--defaults-file" && \
"$option" != "--defaults-extra-file" && \
"$option" != "--defaults-group-suffix" && \
"$option" != "--port" && \
"$option" != "--socket" ]]; then
value=${1#*=}
case "$option" in
'--innodb-data-home-dir')
......@@ -249,7 +253,15 @@ else
MY_PRINT_DEFAULTS=$(which my_print_defaults)
fi
readonly WSREP_SST_OPT_CONF="$WSREP_SST_OPT_DEFAULT $WSREP_SST_OPT_EXTRA_DEFAULT $WSREP_SST_OPT_SUFFIX_DEFAULT"
wsrep_defaults="$WSREP_SST_OPT_DEFAULT"
if [ -n "$wsrep_defaults" ]; then
wsrep_defaults="$wsrep_defaults "
fi
wsrep_defaults="$wsrep_defaults$WSREP_SST_OPT_EXTRA_DEFAULT"
if [ -n "$wsrep_defaults" ]; then
wsrep_defaults="$wsrep_defaults "
fi
readonly WSREP_SST_OPT_CONF="$wsrep_defaults$WSREP_SST_OPT_SUFFIX_DEFAULT"
readonly MY_PRINT_DEFAULTS="$MY_PRINT_DEFAULTS $WSREP_SST_OPT_CONF"
wsrep_auth_not_set()
......
scripts/wsrep_sst_mariabackup.sh
View file @ de0f93fb
......@@ -60,7 +60,7 @@ rebuild=0
rebuildcmd=""
payload=0
pvformat="-F '%N => Rate:%r Avg:%a Elapsed:%t %e Bytes: %b %p' "
pvopts="-f -i 10 -N $WSREP_SST_OPT_ROLE "
pvopts="-f -i 10 -N $WSREP_SST_OPT_ROLE "
STATDIR=""
uextra=0
disver=""
......@@ -130,7 +130,7 @@ get_keys()
if [[ $encrypt -eq 0 ]];then
if $MY_PRINT_DEFAULTS xtrabackup | grep -q encrypt;then
wsrep_log_error "Unexpected option combination. SST may fail. Refer to http://www.percona.com/doc/percona-xtradb-cluster/manual/xtrabackup_sst.html "
wsrep_log_error "Unexpected option combination. SST may fail. Refer to http://www.percona.com/doc/percona-xtradb-cluster/manual/xtrabackup_sst.html"
fi
return
fi
......@@ -465,7 +465,7 @@ cleanup_donor()
if [[ -n ${XTRABACKUP_PID:-} ]];then
if check_pid $XTRABACKUP_PID
then
wsrep_log_error "xtrabackup process is still running. Killing... "
wsrep_log_error "xtrabackup process is still running. Killing..."
kill_xtrabackup
fi
......@@ -567,7 +567,7 @@ check_extra()
# Xtrabackup works only locally.
# Hence, setting host to 127.0.0.1 unconditionally.
wsrep_log_info "SST through extra_port $eport"
INNOEXTRA+=" --host=127.0.0.1 --port=$eport "
INNOEXTRA+=" --host=127.0.0.1 --port=$eport"
use_socket=0
else
wsrep_log_error "Extra port $eport null, failing"
......@@ -577,8 +577,8 @@ check_extra()
wsrep_log_info "Thread pool not set, ignore the option use_extra"
fi
fi
if [[ $use_socket -eq 1 ]] && [[ -n "${WSREP_SST_OPT_SOCKET}" ]];then
INNOEXTRA+=" --socket=${WSREP_SST_OPT_SOCKET}"
if [[ $use_socket -eq 1 ]] && [[ -n "$WSREP_SST_OPT_SOCKET" ]];then
INNOEXTRA+=" --socket=$WSREP_SST_OPT_SOCKET"
fi
}
......@@ -745,8 +745,8 @@ if [[ $ssyslog -eq 1 ]];then
logger -p daemon.info -t ${ssystag}wsrep-sst-$WSREP_SST_OPT_ROLE "$@"
}
INNOAPPLY="${INNOBACKUPEX_BIN} --innobackupex $disver $iapts \$INNOEXTRA --apply-log \$rebuildcmd \${DATA} 2>&1 | logger -p daemon.err -t ${ssystag}innobackupex-apply "
INNOMOVE="${INNOBACKUPEX_BIN} --innobackupex ${WSREP_SST_OPT_CONF} $disver $impts --move-back --force-non-empty-directories \${DATA} 2>&1 | logger -p daemon.err -t ${ssystag}innobackupex-move "
INNOAPPLY="${INNOBACKUPEX_BIN} --innobackupex $disver $iapts \$INNOEXTRA --apply-log \$rebuildcmd \${DATA} 2>&1 | logger -p daemon.err -t ${ssystag}innobackupex-apply"
INNOMOVE="${INNOBACKUPEX_BIN} --innobackupex ${WSREP_SST_OPT_CONF} $disver $impts --move-back --force-non-empty-directories \${DATA} 2>&1 | logger -p daemon.err -t ${ssystag}innobackupex-move"
INNOBACKUP="${INNOBACKUPEX_BIN} --innobackupex ${WSREP_SST_OPT_CONF} $disver $iopts \$tmpopts \$INNOEXTRA --galera-info --stream=\$sfmt \$itmpdir 2> >(logger -p daemon.err -t ${ssystag}innobackupex-backup)"
fi
......@@ -856,13 +856,12 @@ then
get_keys
if [[ $encrypt -eq 1 ]];then
if [[ -n $ekey ]];then
INNOEXTRA+=" --encrypt=$ealgo --encrypt-key=$ekey "
INNOEXTRA+=" --encrypt=$ealgo --encrypt-key=$ekey"
else
INNOEXTRA+=" --encrypt=$ealgo --encrypt-key-file=$ekeyfile "
INNOEXTRA+=" --encrypt=$ealgo --encrypt-key-file=$ekeyfile"
fi
fi
check_extra
wsrep_log_info "Streaming GTID file before SST"
......@@ -883,7 +882,6 @@ then
tcmd=" $scomp | $tcmd "
fi
send_donor $DATA "${stagemsg}-gtid"
tcmd="$ttcmd"
......
scripts/wsrep_sst_rsync.sh
View file @ de0f93fb
......@@ -23,7 +23,7 @@ RSYNC_CONF= # rsync configuration file
RSYNC_REAL_PID= # rsync process id
OS=$(uname)
[ "$OS" == "Darwin" ] && export -n LD_LIBRARY_PATH
[ "$OS" = "Darwin" ] && export -n LD_LIBRARY_PATH
# Setting the path for lsof on CentOS
export PATH="/usr/sbin:/sbin:$PATH"
......@@ -69,11 +69,6 @@ check_pid_and_port()
exit 2 # ENOENT
fi
if ! which lsof > /dev/null; then
wsrep_log_error "lsof tool not found in PATH! Make sure you have it installed."
exit 2 # ENOENT
fi
local port_info=$(lsof -i :$rsync_port -Pn 2>/dev/null | \
grep "(LISTEN)")
local is_rsync=$(echo $port_info | \
......@@ -318,8 +313,8 @@ EOF
cd $WSREP_SST_OPT_DATA
count=1
[ "$OS" == "Linux" ] && count=$(grep -c processor /proc/cpuinfo)
[ "$OS" == "Darwin" -o "$OS" == "FreeBSD" ] && count=$(sysctl -n hw.ncpu)
[ "$OS" = "Linux" ] && count=$(grep -c processor /proc/cpuinfo)
[ "$OS" = "Darwin" -o "$OS" = "FreeBSD" ] && count=$(sysctl -n hw.ncpu)
find . -maxdepth 1 -mindepth 1 -type d -not -name "lost+found" \
-print0 | xargs -I{} -0 -P $count \
......@@ -439,7 +434,7 @@ EOF
RSYNC_PID=$STUNNEL_PID
fi
until check_pid_and_port $RSYNC_PID $RSYNC_REAL_PID $RSYNC_PORT
until check_pid_and_port "$RSYNC_PID" "$RSYNC_REAL_PID" "$RSYNC_PORT"
do
sleep 0.2
done
......
scripts/wsrep_sst_xtrabackup-v2.sh
View file @ de0f93fb
......@@ -56,7 +56,7 @@ rebuild=0
rebuildcmd=""
payload=0
pvformat="-F '%N => Rate:%r Avg:%a Elapsed:%t %e Bytes: %b %p' "
pvopts="-f -i 10 -N $WSREP_SST_OPT_ROLE "
pvopts="-f -i 10 -N $WSREP_SST_OPT_ROLE "
STATDIR=""
uextra=0
disver=""
......@@ -675,7 +675,7 @@ check_extra()
# Xtrabackup works only locally.
# Hence, setting host to 127.0.0.1 unconditionally.
wsrep_log_info "SST through extra_port $eport"
INNOEXTRA+=" --host=127.0.0.1 --port=$eport "
INNOEXTRA+=" --host=127.0.0.1 --port=$eport"
use_socket=0
else
wsrep_log_error "Extra port $eport null, failing"
......@@ -685,8 +685,8 @@ check_extra()
wsrep_log_info "Thread pool not set, ignore the option use_extra"
fi
fi
if [[ $use_socket -eq 1 ]] && [[ -n "${WSREP_SST_OPT_SOCKET}" ]];then
INNOEXTRA+=" --socket=${WSREP_SST_OPT_SOCKET}"
if [[ $use_socket -eq 1 ]] && [[ -n "$WSREP_SST_OPT_SOCKET" ]];then
INNOEXTRA+=" --socket=$WSREP_SST_OPT_SOCKET"
fi
}
......
scripts/wsrep_sst_xtrabackup.sh
View file @ de0f93fb
......@@ -49,7 +49,7 @@ rebuild=0
rebuildcmd=""
payload=0
pvformat="-F '%N => Rate:%r Avg:%a Elapsed:%t %e Bytes: %b %p' "
pvopts="-f -i 10 -N $WSREP_SST_OPT_ROLE "
pvopts="-f -i 10 -N $WSREP_SST_OPT_ROLE "
uextra=0
if which pv &>/dev/null && pv --help | grep -q FORMAT;then
......@@ -380,7 +380,7 @@ check_extra()
# Xtrabackup works only locally.
# Hence, setting host to 127.0.0.1 unconditionally.
wsrep_log_info "SST through extra_port $eport"
INNOEXTRA+=" --host=127.0.0.1 --port=$eport "
INNOEXTRA+=" --host=127.0.0.1 --port=$eport"
use_socket=0
else
wsrep_log_error "Extra port $eport null, failing"
......@@ -390,8 +390,8 @@ check_extra()
wsrep_log_info "Thread pool not set, ignore the option use_extra"
fi
fi
if [[ $use_socket -eq 1 ]] && [[ -n "${WSREP_SST_OPT_SOCKET}" ]];then
INNOEXTRA+=" --socket=${WSREP_SST_OPT_SOCKET}"
if [[ $use_socket -eq 1 ]] && [[ -n "$WSREP_SST_OPT_SOCKET" ]];then
INNOEXTRA+=" --socket=$WSREP_SST_OPT_SOCKET"
fi
}
......@@ -439,14 +439,14 @@ then
get_keys
if [[ $encrypt -eq 1 ]];then
if [[ -n $ekey ]];then
INNOEXTRA+=" --encrypt=$ealgo --encrypt-key=$ekey "
INNOEXTRA+=" --encrypt=$ealgo --encrypt-key=$ekey"
else
INNOEXTRA+=" --encrypt=$ealgo --encrypt-key-file=$ekeyfile "
INNOEXTRA+=" --encrypt=$ealgo --encrypt-key-file=$ekeyfile"
fi
fi
if [[ -n $lsn ]];then
INNOEXTRA+=" --incremental --incremental-lsn=$lsn "
INNOEXTRA+=" --incremental --incremental-lsn=$lsn"
fi
check_extra
......
sql/wsrep_sst.cc
View file @ de0f93fb
......@@ -16,6 +16,7 @@
#include "wsrep_sst.h"
#include <inttypes.h>
#include <ctype.h>
#include <mysqld.h>
#include <m_ctype.h>
#include <my_sys.h>
......@@ -379,7 +380,31 @@ static char* my_fgets (char* buf, size_t buf_len, FILE* stream)
}
/*
Generate opt_binlog_opt_val for sst_donate_other(), sst_prepare_other().
Generate "name 'value'" string.
*/
static char* generate_name_value(const char* name, const char* value)
{
size_t name_len= strlen(name);
size_t value_len= strlen(value);
char* buf=
(char*) my_malloc((name_len + value_len + 5) * sizeof(char), MYF(0));
if (buf)
{
char* ref= buf;
*ref++ = ' ';
memcpy(ref, name, name_len * sizeof(char));
ref += name_len;
*ref++ = ' ';
*ref++ = '\'';
memcpy(ref, value, value_len * sizeof(char));
ref += value_len;
*ref++ = '\'';
*ref = 0;
}
return buf;
}
/*
Generate binlog option string for sst_donate_other(), sst_prepare_other().
Returns zero on success, negative error code otherwise.
......@@ -395,7 +420,9 @@ static int generate_binlog_opt_val(char** ret)
{
assert(opt_bin_logname);
*ret= strcmp(opt_bin_logname, "0") ?
my_strdup(opt_bin_logname, MYF(0)) : my_strdup("", MYF(0));
generate_name_value(WSREP_SST_OPT_BINLOG,
opt_bin_logname) :
my_strdup("", MYF(0));
}
else
{
......@@ -411,7 +438,9 @@ static int generate_binlog_index_opt_val(char** ret)
*ret= NULL;
if (opt_binlog_index_name) {
*ret= strcmp(opt_binlog_index_name, "0") ?
my_strdup(opt_binlog_index_name, MYF(0)) : my_strdup("", MYF(0));
generate_name_value(WSREP_SST_OPT_BINLOG_INDEX,
opt_binlog_index_name) :
my_strdup("", MYF(0));
}
else
{
......@@ -608,7 +637,86 @@ static size_t estimate_cmd_len (bool* extra_args)
{
for (int i = 1; i < orig_argc; i++)
{
cmd_len += strlen(orig_argv[i]);
const char* arg= orig_argv[i];
size_t n= strlen(arg);
if (n == 0) continue;
cmd_len += n;
bool quotation= false;
char c;
while ((c = *arg++) != 0)
{
/* A whitespace or a single quote requires double quotation marks: */
if (isspace(c) || c == '\'')
{
quotation= true;
}
/*
If the equals symbol is encountered, then we need to separately
process the right side:
*/
else if (c == '=')
{
/* Perhaps we need to quote the left part of the argument: */
if (quotation)
{
cmd_len += 2;
/*
Reset the quotation flag, since now the status for
the right side of the expression will be saved here:
*/
quotation= false;
}
while ((c = *arg++) != 0)
{
/*
A whitespace or a single quote requires double
quotation marks:
*/
if (isspace(c) || c == '\'')
{
quotation= true;
}
/*
Double quotation mark or backslash symbol requires backslash
prefixing:
*/
#ifdef __WIN__
else if (c == '"' || c == '\\')
#else
/*
The dollar symbol is used to substitute a variable, therefore
it also requires escaping:
*/
else if (c == '"' || c == '\\' || c == '$')
#endif
{
cmd_len++;
}
}
break;
}
/*
Double quotation mark or backslash symbol requires backslash
prefixing:
*/
#ifdef __WIN__
else if (c == '"' || c == '\\')
#else
/*
The dollar symbol is used to substitute a variable, therefore
it also requires escaping:
*/
else if (c == '"' || c == '\\' || c == '$')
#endif
{
cmd_len++;
}
}
/* Perhaps we need to quote the entire argument or its right part: */
if (quotation)
{
cmd_len += 2;
}
}
extra = true;
cmd_len += strlen(WSREP_SST_OPT_MYSQLD);
......@@ -636,10 +744,171 @@ static void copy_orig_argv (char* cmd_str)
for (int i = 1; i < orig_argc; i++)
{
char* arg= orig_argv[i];
*cmd_str++ = ' ';
n = strlen(arg);
memcpy(cmd_str, arg, n * sizeof(char));
cmd_str += n;
if (n == 0) continue;
*cmd_str++ = ' ';
bool quotation= false;
bool plain= true;
char *arg_scan= arg;
char c;
while ((c = *arg_scan++) != 0)
{
/* A whitespace or a single quote requires double quotation marks: */
if (isspace(c) || c == '\'')
{
quotation= true;
}
/*
If the equals symbol is encountered, then we need to separately
process the right side:
*/
else if (c == '=')
{
/* Calculate length of the Left part of the argument: */
size_t m = (size_t) (arg_scan - arg) - 1;
if (m)
{
/* Perhaps we need to quote the left part of the argument: */
if (quotation)
{
*cmd_str++ = '"';
}
/*
If there were special characters inside, then we can use
the fast memcpy function:
*/
if (plain)
{
memcpy(cmd_str, arg, m * sizeof(char));
cmd_str += m;
/* Left part of the argument has already been processed: */
n -= m;
arg += m;
}
/* Otherwise we need to prefix individual characters: */
else
{
n -= m;
while (m)
{
c = *arg++;
#ifdef __WIN__
if (c == '"' || c == '\\')
#else
if (c == '"' || c == '\\' || c == '$')
#endif
{
*cmd_str++ = '\\';
}
*cmd_str++ = c;
m--;
}
/*
Reset the plain string flag, since now the status for
the right side of the expression will be saved here:
*/
plain= true;
}
/* Perhaps we need to quote the left part of the argument: */
if (quotation)
{
*cmd_str++ = '"';
/*
Reset the quotation flag, since now the status for
the right side of the expression will be saved here:
*/
quotation= false;
}
}
/* Copy equals symbol: */
*cmd_str++ = '=';
arg++;
n--;
/* Let's deal with the left side of the expression: */
while ((c = *arg_scan++) != 0)
{
/*
A whitespace or a single quote requires double
quotation marks:
*/
if (isspace(c) || c == '\'')
{
quotation= true;
}
/*
Double quotation mark or backslash symbol requires backslash
prefixing:
*/
#ifdef __WIN__
else if (c == '"' || c == '\\')
#else
/*
The dollar symbol is used to substitute a variable, therefore
it also requires escaping:
*/
else if (c == '"' || c == '\\' || c == '$')
#endif
{
plain= false;
}
}
break;
}
/*
Double quotation mark or backslash symbol requires backslash
prefixing:
*/
#ifdef __WIN__
else if (c == '"' || c == '\\')
#else
/*
The dollar symbol is used to substitute a variable, therefore
it also requires escaping:
*/
else if (c == '"' || c == '\\' || c == '$')
#endif
{
plain= false;
}
}
if (n)
{
/* Perhaps we need to quote the entire argument or its right part: */
if (quotation)
{
*cmd_str++ = '"';
}
/*
If there were no special characters inside, then we can use
the fast memcpy function:
*/
if (plain)
{
memcpy(cmd_str, arg, n * sizeof(char));
cmd_str += n;
}
/* Otherwise we need to prefix individual characters: */
else
{
while ((c = *arg++) != 0)
{
#ifdef __WIN__
if (c == '"' || c == '\\')
#else
if (c == '"' || c == '\\' || c == '$')
#endif
{
*cmd_str++ = '\\';
}
*cmd_str++ = c;
}
}
/* Perhaps we need to quote the entire argument or its right part: */
if (quotation)
{
*cmd_str++ = '"';
}
}
}
/*
Add a terminating null character (not counted in the length,
......@@ -666,8 +935,6 @@ static ssize_t sst_prepare_other (const char* method,
return -ENOMEM;
}
const char* binlog_opt= "";
const char* binlog_index_opt= "";
char* binlog_opt_val= NULL;
char* binlog_index_opt_val= NULL;
......@@ -685,9 +952,6 @@ static ssize_t sst_prepare_other (const char* method,
ret);
}
if (strlen(binlog_opt_val)) binlog_opt= WSREP_SST_OPT_BINLOG;
if (strlen(binlog_index_opt_val)) binlog_index_opt= WSREP_SST_OPT_BINLOG_INDEX;
make_wsrep_defaults_file();
ret= snprintf (cmd_str(), cmd_len,
......@@ -695,14 +959,14 @@ static ssize_t sst_prepare_other (const char* method,
WSREP_SST_OPT_ROLE " 'joiner' "
WSREP_SST_OPT_ADDR " '%s' "
WSREP_SST_OPT_DATA " '%s' "
" %s "
"%s"
WSREP_SST_OPT_PARENT " '%d'"
" %s '%s'"
" %s '%s'",
"%s"
"%s",
method, addr_in, mysql_real_data_home,
wsrep_defaults_file,
(int)getpid(), binlog_opt, binlog_opt_val,
binlog_index_opt, binlog_index_opt_val);
(int)getpid(),
binlog_opt_val, binlog_index_opt_val);
my_free(binlog_opt_val);
my_free(binlog_index_opt_val);
......@@ -999,7 +1263,7 @@ static int sst_donate_mysqldump (const char* addr,
WSREP_SST_OPT_PORT " '%d' "
WSREP_SST_OPT_LPORT " '%u' "
WSREP_SST_OPT_SOCKET " '%s' "
" %s "
"%s"
WSREP_SST_OPT_GTID " '%s:%lld' "
WSREP_SST_OPT_GTID_DOMAIN_ID " '%d'"
"%s",
......@@ -1347,7 +1611,6 @@ static int sst_donate_other (const char* method,
return -ENOMEM;
}
const char* binlog_opt= "";
char* binlog_opt_val= NULL;
int ret;
......@@ -1356,7 +1619,6 @@ static int sst_donate_other (const char* method,
WSREP_ERROR("sst_donate_other(): generate_binlog_opt_val() failed: %d",ret);
return ret;
}
if (strlen(binlog_opt_val)) binlog_opt= WSREP_SST_OPT_BINLOG;
make_wsrep_defaults_file();
......@@ -1366,15 +1628,15 @@ static int sst_donate_other (const char* method,
WSREP_SST_OPT_ADDR " '%s' "
WSREP_SST_OPT_SOCKET " '%s' "
WSREP_SST_OPT_DATA " '%s' "
" %s "
" %s '%s' "
"%s"
WSREP_SST_OPT_GTID " '%s:%lld' "
WSREP_SST_OPT_GTID_DOMAIN_ID " '%d'"
"%s"
"%s",
method, addr, mysqld_unix_port, mysql_real_data_home,
wsrep_defaults_file,
binlog_opt, binlog_opt_val,
uuid, (long long) seqno, wsrep_gtid_domain_id,
binlog_opt_val,
bypass ? " " WSREP_SST_OPT_BYPASS : "");
my_free(binlog_opt_val);
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment
GitLab Nexedi Edition | About GitLab | About Nexedi | 沪ICP备2021021310号-2 | 沪ICP备2021021310号-7