-
Notifications
You must be signed in to change notification settings - Fork 149
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
PXC-4469: Implement CLONE method for SST #2046
PXC-4469: Implement CLONE method for SST #2046
Conversation
https://perconadev.atlassian.net/browse/PXC-4469 Implemented SST method using clone plugin.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Clang-Tidy
found issue(s) with the introduced code (1/2)
@@ -73,6 +74,7 @@ extern const char *wsrep_defaults_group_suffix; | |||
#define WSREP_SST_ONLY_IST "ist_only" | |||
#define WSREP_SST_XTRABACKUP "xtrabackup" | |||
#define WSREP_SST_XTRABACKUP_V2 "xtrabackup-v2" | |||
#define WSREP_SST_CLONE "clone" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
macro WSREP_SST_CLONE
used to declare a constant; consider using a constexpr
constant
struct sst_thread_arg { | ||
const char *cmd; | ||
char **env; | ||
const sst_auth &auth_container; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
member variable auth_container
has public visibility
struct sst_thread_arg { | ||
const char *cmd; | ||
char **env; | ||
const sst_auth &auth_container; | ||
char *ret_str; | ||
int err; | ||
|
||
mysql_mutex_t LOCK_wsrep_sst_thread; | ||
mysql_cond_t COND_wsrep_sst_thread; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
constructor does not initialize these fields: COND_wsrep_sst_thread
mysql_cond_t COND_wsrep_sst_thread; | |
mysql_cond_t COND_wsrep_sst_thread{}; |
char *ret_str; | ||
int err; | ||
|
||
mysql_mutex_t LOCK_wsrep_sst_thread; | ||
mysql_cond_t COND_wsrep_sst_thread; | ||
|
||
sst_thread_arg(const char *c, char **e) | ||
: cmd(c), env(e), ret_str(0), err(-1) { | ||
sst_thread_arg(const char *c, char **e, sst_auth &auth) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
parameter name c
is too short, expected at least 2 characters
char *ret_str; | ||
int err; | ||
|
||
mysql_mutex_t LOCK_wsrep_sst_thread; | ||
mysql_cond_t COND_wsrep_sst_thread; | ||
|
||
sst_thread_arg(const char *c, char **e) | ||
: cmd(c), env(e), ret_str(0), err(-1) { | ||
sst_thread_arg(const char *c, char **e, sst_auth &auth) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
parameter name e
is too short, expected at least 2 characters
sst_thread_arg(const char *c, char **e) | ||
: cmd(c), env(e), ret_str(0), err(-1) { | ||
sst_thread_arg(const char *c, char **e, sst_auth &auth) | ||
: cmd(c), env(e), auth_container(auth), ret_str(0), err(-1) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use nullptr
: cmd(c), env(e), auth_container(auth), ret_str(0), err(-1) { | |
: cmd(c), env(e), auth_container(auth), ret_str(nullptr), err(-1) { |
nullptr, | ||
"DROP USER IF EXISTS 'mysql.pxc.sst.user'@localhost;", | ||
nullptr, | ||
"CREATE USER 'mysql.pxc.sst.user'@localhost " |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suspicious string literal, probably missing a comma
@@ -1254,6 +1282,7 @@ int wsrep_remove_sst_user(bool initialize_thread) { | |||
|
|||
static void *sst_donor_thread(void *a) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
function sst_donor_thread
has cognitive complexity of 121 (threshold 50)
@@ -1318,6 +1347,18 @@ static void *sst_donor_thread(void *a) { | |||
err = (ret < 0 ? ret : -EMSGSIZE); | |||
} | |||
|
|||
// if remote user is defined we will pass the user-name/password pair | |||
if (auth.remote_name_.length()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
implicit conversion std::basic_string<char>::size_type
(aka unsigned long
) -> bool
if (auth.remote_name_.length()) { | |
if (auth.remote_name_.length() != 0u) { |
*/ | ||
const char *addr = strrchr(data, '@'); | ||
wsp::string remote_auth; | ||
if (addr) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
implicit conversion const char *
-> bool
if (addr) { | |
if (addr != nullptr) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Clang-Tidy
found issue(s) with the introduced code (2/2)
|
||
/* Set up auth info (from <user>:<password> strings) */ | ||
sst_auth auth; | ||
if (remote_auth()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
implicit conversion char *
-> bool
if (remote_auth()) { | |
if (remote_auth() != nullptr) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approved
Merge pull request percona#2046 from kamil-holubicki/PXC-4469-squashed Cherry pick commit 5993775 # Conflicts: # mysql-test/r/grant_dynamic.result # mysql-test/r/mysql_upgrade.result # mysql-test/suite/perfschema/r/privilege_table_io.result # scripts/mysql_system_tables_fix.sql
https://perconadev.atlassian.net/browse/PXC-4469
Implemented SST method using clone plugin.