Skip to content
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 #1996

Draft
wants to merge 60 commits into
base: 8.0
Choose a base branch
from
Draft

Conversation

kamil-holubicki
Copy link
Contributor

Implement CLONE method for SST
Work in progress

Tusamarco and others added 30 commits August 2, 2024 15:26
This version has a patch to go straight to ist so is not fully functional unless removing that bypass
Need to ad extension or make script will not identify them correctly
…the user

Some edit/cleanup as suggested by Kamil
- removed the need to perform the restart to clean up the final instance.
- use wsrep-sst-receive-address to get IP and port definition
- use one port for Netcat and MySQL
- some code cleanup
- Add message suppression when reporting clone Status %. It will be reported only if different from previous value.
@kamil-holubicki kamil-holubicki marked this pull request as draft January 10, 2025 12:34
@kamil-holubicki kamil-holubicki changed the title PXC-4469: cImplement CLONE method for SST PXC-4469: Implement CLONE method for SST Jan 28, 2025
https://perconadev.atlassian.net/browse/PXC-4620

fixed:
galera.galera_sst_clone - aligned with galera_sst_xtrabackup-v2 test

Missing privileges granted / privileges cleanup:
main.grant_dynamic_flush_upgrade
main.roles-upgrade
main.mysql_upgrade_slave_master_info
main.plugin_auth
main.mysql_57_inplace_upgrade
main.mysql_80_inplace_upgrade
main.component-upgrade
main.grant_dynamic

re-recorded:
perfschema.privilege_table_io
funcs_1.is_table_privileges
funcs_1.is_schema_privileges
main.transactional_acl_tables
main.ps_sys_upgrade
Copy link

@github-actions github-actions bot left a 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/3)

struct sst_thread_arg {
const char *cmd;
char **env;
const sst_auth& auth_container;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ misc-non-private-member-variables-in-classes ⚠️
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;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ cppcoreguidelines-pro-type-member-init ⚠️
constructor does not initialize these fields: COND_wsrep_sst_thread

Suggested change
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)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ readability-identifier-length ⚠️
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)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ readability-identifier-length ⚠️
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) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ modernize-use-nullptr ⚠️
use nullptr

Suggested change
: cmd(c), env(e), auth_container(auth), ret_str(0), err(-1) {
: cmd(c), env(e), auth_container(auth), ret_str(nullptr), err(-1) {

@@ -700,8 +710,7 @@
static void reset_ld_preload(wsp::env &env) { env.append("LD_PRELOAD="); }
#endif

static ssize_t sst_prepare_other(const char *method, const char *addr_in,
const char **addr_out) {
static ssize_t sst_prepare_other(const char *method, const char *addr_in, const char **addr_out) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ readability-function-cognitive-complexity ⚠️
function sst_prepare_other has cognitive complexity of 80 (threshold 50)

@@ -1254,6 +1266,7 @@

static void *sst_donor_thread(void *a) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ readability-function-cognitive-complexity ⚠️
function sst_donor_thread has cognitive complexity of 121 (threshold 50)

@@ -1318,6 +1331,21 @@
err = (ret < 0 ? ret : -EMSGSIZE);
}

// if remote user is defined we will pass the user-name/password pair
if (auth.remote_name_.length())
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ readability-implicit-bool-conversion ⚠️
implicit conversion std::basic_string<char>::size_type (aka unsigned long) -> bool

Suggested change
if (auth.remote_name_.length())
if (auth.remote_name_.length() != 0u)

*/
const char* addr= strrchr(data, '@');
wsp::string remote_auth;
if (addr) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ readability-implicit-bool-conversion ⚠️
implicit conversion const char * -> bool

Suggested change
if (addr) {
if (addr != nullptr) {


/* Set up auth info (from <user>:<password> strings) */
sst_auth auth;
if (remote_auth()) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ readability-implicit-bool-conversion ⚠️
implicit conversion char * -> bool

Suggested change
if (remote_auth()) {
if (remote_auth() != nullptr) {

Copy link

@github-actions github-actions bot left a 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/3)

@@ -0,0 +1,119 @@
--echo Performing State Transfer on a server that has been killed and restarted
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ clang-diagnostic-error ⚠️
expected unqualified-id


--connection node_1
CREATE TABLE t1 (f1 CHAR(255)) ENGINE=InnoDB;
SET AUTOCOMMIT=OFF;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ clang-diagnostic-error ⚠️
unknown type name SET


--connection node_1
CREATE TABLE t1 (f1 CHAR(255)) ENGINE=InnoDB;
SET AUTOCOMMIT=OFF;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ clang-diagnostic-error ⚠️
use of undeclared identifier OFF

--connection node_1
CREATE TABLE t1 (f1 CHAR(255)) ENGINE=InnoDB;
SET AUTOCOMMIT=OFF;
START TRANSACTION;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ clang-diagnostic-error ⚠️
unknown type name START

CREATE TABLE t1 (f1 CHAR(255)) ENGINE=InnoDB;
SET AUTOCOMMIT=OFF;
START TRANSACTION;
INSERT INTO t1 VALUES ('node1_committed_before');
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ clang-diagnostic-error ⚠️
unknown type name INSERT

CREATE TABLE t1 (f1 CHAR(255)) ENGINE=InnoDB;
SET AUTOCOMMIT=OFF;
START TRANSACTION;
INSERT INTO t1 VALUES ('node1_committed_before');
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ clang-diagnostic-error ⚠️
expected ; after top level declarator

Suggested change
INSERT INTO t1 VALUES ('node1_committed_before');
INSERT INTO; t1 VALUES ('node1_committed_before');

SET AUTOCOMMIT=OFF;
START TRANSACTION;
INSERT INTO t1 VALUES ('node1_committed_before');
INSERT INTO t1 VALUES ('node1_committed_before');
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ clang-diagnostic-error ⚠️
unknown type name INSERT

SET AUTOCOMMIT=OFF;
START TRANSACTION;
INSERT INTO t1 VALUES ('node1_committed_before');
INSERT INTO t1 VALUES ('node1_committed_before');
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ clang-diagnostic-error ⚠️
expected ; after top level declarator

Suggested change
INSERT INTO t1 VALUES ('node1_committed_before');
INSERT INTO; t1 VALUES ('node1_committed_before');

START TRANSACTION;
INSERT INTO t1 VALUES ('node1_committed_before');
INSERT INTO t1 VALUES ('node1_committed_before');
INSERT INTO t1 VALUES ('node1_committed_before');
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ clang-diagnostic-error ⚠️
unknown type name INSERT

START TRANSACTION;
INSERT INTO t1 VALUES ('node1_committed_before');
INSERT INTO t1 VALUES ('node1_committed_before');
INSERT INTO t1 VALUES ('node1_committed_before');
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ clang-diagnostic-error ⚠️
expected ; after top level declarator

Suggested change
INSERT INTO t1 VALUES ('node1_committed_before');
INSERT INTO; t1 VALUES ('node1_committed_before');

Copy link

@github-actions github-actions bot left a 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 (3/3)

INSERT INTO t1 VALUES ('node1_committed_before');
INSERT INTO t1 VALUES ('node1_committed_before');
INSERT INTO t1 VALUES ('node1_committed_before');
INSERT INTO t1 VALUES ('node1_committed_before');
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ clang-diagnostic-error ⚠️
unknown type name INSERT

INSERT INTO t1 VALUES ('node1_committed_before');
INSERT INTO t1 VALUES ('node1_committed_before');
INSERT INTO t1 VALUES ('node1_committed_before');
INSERT INTO t1 VALUES ('node1_committed_before');
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ clang-diagnostic-error ⚠️
expected ; after top level declarator

Suggested change
INSERT INTO t1 VALUES ('node1_committed_before');
INSERT INTO; t1 VALUES ('node1_committed_before');

INSERT INTO t1 VALUES ('node1_committed_before');
INSERT INTO t1 VALUES ('node1_committed_before');
INSERT INTO t1 VALUES ('node1_committed_before');
INSERT INTO t1 VALUES ('node1_committed_before');
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ clang-diagnostic-error ⚠️
unknown type name INSERT

INSERT INTO t1 VALUES ('node1_committed_before');
INSERT INTO t1 VALUES ('node1_committed_before');
INSERT INTO t1 VALUES ('node1_committed_before');
INSERT INTO t1 VALUES ('node1_committed_before');
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ clang-diagnostic-error ⚠️
expected ; after top level declarator

Suggested change
INSERT INTO t1 VALUES ('node1_committed_before');
INSERT INTO; t1 VALUES ('node1_committed_before');

INSERT INTO t1 VALUES ('node1_committed_before');
INSERT INTO t1 VALUES ('node1_committed_before');

--connection node_2
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ clang-diagnostic-error ⚠️
expected unqualified-id


--connection node_2
START TRANSACTION;
INSERT INTO t1 VALUES ('node2_committed_before');
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ clang-diagnostic-error ⚠️
unknown type name INSERT


--connection node_2
START TRANSACTION;
INSERT INTO t1 VALUES ('node2_committed_before');
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ clang-diagnostic-error ⚠️
expected ; after top level declarator

Suggested change
INSERT INTO t1 VALUES ('node2_committed_before');
INSERT INTO; t1 VALUES ('node2_committed_before');

--connection node_2
START TRANSACTION;
INSERT INTO t1 VALUES ('node2_committed_before');
INSERT INTO t1 VALUES ('node2_committed_before');
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ clang-diagnostic-error ⚠️
unknown type name INSERT

--connection node_2
START TRANSACTION;
INSERT INTO t1 VALUES ('node2_committed_before');
INSERT INTO t1 VALUES ('node2_committed_before');
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ clang-diagnostic-error ⚠️
expected ; after top level declarator

Suggested change
INSERT INTO t1 VALUES ('node2_committed_before');
INSERT INTO; t1 VALUES ('node2_committed_before');

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants