Pxc 4469 sst clone #1961

Tusamarco · 2024-10-07T17:08:10Z

Adding preview POC for the Clone method for SST.
The script is based on the one from codership but then need to refactor almost in full to have it working with PXC.
birdeye flow

#              ┌──────────────────┐                      
#              │  Joiner starts   │                      
#              └────────┬─────────┘                      
#              ┌────────▼─────────┐                      
#              │open NetCat listnr│                      
#              └────────┬─────────┘                      
#              ┌────────▼─────────┐                      
#              │Send message to Dn│                      
#              └────────┬─────────┘                      
#              ┌────────▼─────────┐                      
#              │Donor decide IST  │                      
#              │or SST. Send msg  │                      
#              │through NC and    │                      
#              │waits for Joiner  │                      
#              │Clone instance    │                      
#              └────────┬─────────┘                      
#              ┌────────▼─────────┐                      
#              │Joiner get IST    │                      
#              │or SST.If IST     │                      
#              │Bypass all and    │                      
#              │Wait for IST.     │                      
#              │SST Start clone   │                      
#              │Instance and wait │                      
#              │for donor to clone│                      
#              └─────────┬────────┘                      
#              ┌─────────▼─────────────┐                 
#              │Clone process is       │                 
#              │reported.              │                 
#              │When done Dn waits     │                 
#              │Joiner close instance  │                 
#              │And performs 3 restarts│                 
#              └──────────┬────────────┘                 
#              ┌──────────▼──────────────┐               
#              │1) To fix dictionary     │               
#              │2) To recover position   │               
#              │3) Final cleanup         │               
#              └──────────┬──────────────┘               
#              ┌──────────▼──────────────┐               
#              │Send final ready signal  │               
#              │Waits for IST            │               
#              └─────────────────────────┘

This version has a patch to go straight to ist so is not fully functional unless removing that bypass

SSL is NOT supported

bug. Wrong comma

fixing config missed wsrep allowed methods

…tcat is not running

it-percona-cla · 2024-10-07T17:08:15Z

All committers have signed the CLA.

Tusamarco · 2024-10-07T17:18:36Z

I have a question...
the two file wsrep_clone and wsrep_common_clone are not copied over when I do make install where is that defined?
@kamil-holubicki @venkatesh-prasad-v

Need to ad extension or make script will not identify them correctly

…the user Some edit/cleanup as suggested by Kamil

mysql-test/suite/galera/include/galera_st_kill_slave_ddl.inc

mysql-test/suite/galera/t/galera_sst_clone.cnf

mysql-test/suite/galera/t/galera_sst_clone.test

scripts/wsrep_sst_common_clone.sh

scripts/wsrep_sst_clone.sh

scripts/mysql_system_users.sql

sql/wsrep_sst.cc

scripts/mysql_system_users.sql

sql/wsrep_sst.cc

scripts/wsrep_sst_common_clone.sh

kamil-holubicki · 2024-11-12T19:08:36Z

As we spoke today, we need to test it with encrypted tables and keyring component/plugin enabled

- 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.

Tusamarco · 2024-11-20T14:58:26Z

@kamil-holubicki

==============================================================================
                  TEST NAME                       RESULT  TIME (ms) COMMENT
------------------------------------------------------------------------------
[ 33%] galera.galera_sst_clone 'enc_off'         [ pass ]  28381
[ 66%] galera.galera_sst_clone 'enc_on'          [ pass ]  28789
[100%] shutdown_report                           [ pass ]       
------------------------------------------------------------------------------
The servers were restarted 1 times
The servers were reinitialized 0 times
Spent 57.170 of 168 seconds executing testcases

Completed: All 3 tests were successful.

If you outline what kind of operation are you expecting to test when talking about data encryption I can see how clone deal with it. Also if think this can be step 2

scripts/wsrep_sst_clone.sh

mysql-test/suite/galera/r/galera_sst_clone.result

scripts/wsrep_sst_clone.sh

venkatesh-prasad-v · 2024-11-22T09:08:09Z

scripts/wsrep_sst_clone.sh

+        CLONE_EXECUTE_SQL=$(wsrep_mktemp_in_dir "$WSREP_SST_OPT_DATA" --suffix=.sql clone_execute_XXXX)
+        CLONE_PREPARE_SQL=$(wsrep_mktemp_in_dir "$WSREP_SST_OPT_DATA" --suffix=.sql clone_prepare_XXXX)
+
+        cleanup_donor # Remove potentially existing clone user


Here we call cleanup_donor to remove the potentially existing clone user, however this will run a DROP USER IF EXISTS 'clone_sst'@'%' in TOI and is replicated to other nodes as well. Is this something we want?

2024-11-22T08:58:32.975616Z 0 [Note] [MY-000000] [WSREP-SST] (debug) Cleanup MySQL ADMIN_PSWD: yx9!A-ezgMrQz5Q7HdKLwD7rEqHzJ0Ot 2024-11-22T08:58:32.975760Z 0 [Note] [MY-000000] [WSREP-SST] (debug) Cleanup MySQL MYSQL_ACLIENT: /home/venki/work/pxc/merge/80/bld/bin/mysql --no-defaults -umysql.pxc.sst.user -S/home/venki/work/pxc/merge/80/bld/mysql-test/var/tmp/mysqld.1.sock --batch --skip_column_names --silent 2024-11-22T08:58:32.986956Z 13 [Note] [MY-000000] [WSREP] wsrep_sync_wait: thd->variables.wsrep_sync_wait= 15, mask= 1, thd->variables.wsrep_on= 1 2024-11-22T08:58:32.987820Z 0 [Note] [MY-000000] [WSREP-SST] ERROR 1193 (HY000) at line 1: Unknown system variable 'clone_ssl_cert' 2024-11-22T08:58:32.997468Z 14 [Note] [MY-000000] [WSREP] wsrep_sync_wait: thd->variables.wsrep_sync_wait= 15, mask= 1, thd->variables.wsrep_on= 1 2024-11-22T08:58:33.004851Z 14 [Note] [MY-000000] [WSREP] Executing Query (DROP USER IF EXISTS 'clone_sst'@'%') with write-set (-1) and exec_mode: local in TO Isolation mode 2024-11-22T08:58:33.004913Z 14 [Note] [MY-000000] [WSREP] wsrep: initiating TOI for write set (-1) 2024-11-22T08:58:33.005813Z 14 [Note] [MY-000000] [WSREP] Query (DROP USER IF EXISTS 'clone_sst'@'%') with write-set (3) and exec_mode: toi replicated in TO Isolation mode 2024-11-22T08:58:33.005835Z 14 [Note] [MY-000000] [WSREP] wsrep: TO isolation initiated for write set (3) 2024-11-22T08:58:33.011278Z 14 [Note] [MY-000000] [WSREP] TO END: 3: DROP USER IF EXISTS 'clone_sst'@'%' 2024-11-22T08:58:33.011306Z 14 [Note] [MY-000000] [WSREP] wsrep: completed TOI write set (3) 2024-11-22T08:58:33.011323Z 14 [Note] [MY-000000] [WSREP] Setting WSREPXid (InnoDB): f0d030d8-a8af-11ef-927b-272a1375415c:3 2024-11-22T08:58:33.011372Z 14 [Note] [MY-000000] [WSREP] Updating WSREPXid: f0d030d8-a8af-11ef-927b-272a1375415c:3 2024-11-22T08:58:33.013101Z 14 [Note] [MY-000000] [WSREP] TO END: -1 2024-11-22T08:58:33.013812Z 14 [Warning] [MY-000000] [WSREP] Toggling wsrep_on to OFF will affect sql_log_bin. Check manual for more details 2024-11-22T08:58:33.019913Z 0 [Note] [MY-000000] [WSREP-SST] Cleanup DONOR DONE. 2024-11-22T08:58:33.019989Z 0 [Note] [MY-000000] [WSREP-SST] (debug) -> PREPARE DONOR 2024-11-22T08:58:33.033988Z 15 [Note] [MY-000000] [WSREP] wsrep_sync_wait: thd->variables.wsrep_sync_wait= 15, mask= 1, thd->variables.wsrep_on= 1 2024-11-22T08:58:33.034750Z 15 [Note] [MY-000000] [WSREP] wsrep_sync_wait: thd->variables.wsrep_sync_wait= 15, mask= 1, thd->variables.wsrep_on= 1 2024-11-22T08:58:33.040082Z 0 [Note] [MY-000000] [WSREP-SST] Installing CLONE plugin

good catch!
This is probably better to run this on the donor only!

ON the other hand user is created and replicated, so it makes also sense to delete from other nodes. What it should not replicated (and is not with wsrep_on=OFF) is the plugin installation.
Need to think more about this. For the moment I keep it as it is

Keep this open as reminder.

scripts/wsrep_sst_clone.sh

mysql-test/suite/galera/t/galera_sst_clone.cnf

scripts/wsrep_sst_clone.sh

kamil-holubicki · 2024-11-22T09:51:02Z

scripts/wsrep_sst_clone.sh

+    WSREP_SST_OPT_REMOTE_JOINER_PSWD=""
+
+    echo "continue" # donor can resume updating data
+    WSREP_SST_OPT_REMOTE_AUTH=$(echo $WSREP_SST_OPT_ADDR_LOCAL | cut -d '@' -f 1)


Right now --address parameter is in format clone_user:clone_passwd@joiner_ip:joiner:port
Why does it not conform to the format expected by --address in sst_common (ip:port/path)?
We are doing the full parsing here instead of taking the advantage of
what we already have in wsrep_sst_common.sh
If it was like joiner_ip:joiner_port/clone_user:clone_passwd
wsrep_sst_common, parses this into tokens
WSREP_SST_OPT_HOST -> joiner_ip
WSREP_SST_OPT_PORT -> joiner_port
WSREP_SST_OPT_PATH -> clone_user:clone_passwd
Then we just need to split WSREP_SST_OPT_PATH here.

Hi @Tusamarco , As I explained in this and other related comments, there is no reason for changing the SST request string protocol. Using the existing protocol will allow us to reuse existing parsing code and will keep the solution consistent with pxb-sst. Please change.

@kamil-holubicki I don't know from where you get that ip:port/path is the current standard for galera.
The docs here https://galeracluster.com/library/documentation/scriptable-sst.htm indicate to report ready <address>:port\n
The standard in sharing this kind of URI say <credential>@<address>:<port>/<attrinbutes> as already reported in another comment.
So I am following the standard and best practice.
Actually if we have something different in the code we should correct that and do the right thing not deviating from common standard creating a different approach.
That part will not change.

OK, as we discussed on Slack, I think we have a bigger issue here than format of the string.
user:password is passed via command line parameter to clone-sst script, like --address user:pass@host:port. This causes that the password is visible in the log output and in the process list output (ps). So this is a security issue.
In pxb-sst the donor needs to be informed about the user:password used by PXB to do a backup. And we send this information to the pxb-sst script via pipe.
If we look at what upstream does, they indeed use ready user:pass@host:port. I don't like it, don't see a single reason for doing this, but OK, let it be. But then they extract user:pass part on the server level and pass this information to the clone-sst script via pipe. Command line contains only host:port part (like --address host:port) and then the clone script uses the standard (the same as for pxb) way of parsing --address value).

Again, I will not insist on SST string format if we pass credentials in a secure way. The reason is that there needs to be some part of the logic on the server side anyway to parse sst string and detect that it includes credentials. Another reason is that they use @ character to detect credentials sent in SST string, and this is generic implementation on the server side (not specific to clone-sst) so I agree, In such a case let's use user:pass@host:port.

scripts/wsrep_sst_clone.sh

kamil-holubicki · 2024-11-22T09:57:15Z

scripts/wsrep_sst_clone.sh

+
+    # Report clone credentials/address to the caller
+    wsrep_log_debug "-> ready passing string |$CLONE_USER:$CLONE_PSWD]$JOINER_CLONE_HOST:$JOINER_CLONE_PORT|"
+    echo "ready $CLONE_USER:$CLONE_PSWD]$JOINER_CLONE_HOST:$JOINER_CLONE_PORT"


Let's just pass it as $JOINER_CLONE_HOST:$JOINER_CLONE_PORT/$CLONE_USER:$CLONE_PSWD
This way it will be a lot easier to parse it on donor side (already in sst_common). Please see other related comments.

see other comments on same topic.

kamil-holubicki · 2024-11-22T09:58:33Z

scripts/wsrep_sst_clone.sh

+
+    wsrep_log_debug "-> WAIT DIR DONOR MESSAGE DONE"
+
+    if [[ ! `cat $WSREP_SST_OPT_DATA/XST_FILE.txt` =~ "SST@" ]];then


We still didn't solve the problem of not full message in file
My proposal is to send from Donor such a string:
SST@${WSREP_SST_OPT_GTID}<EOF>
and then wait in a loop above until we receive <EOF> substring in a file
then we can test the content of the file.
BTW. In the case of IST, shouldn't Donor signal by sending something?

Here we have 2 messages.
SST or IST, there is NO other. So if not SST is IST, or as you fear a broken one. No need to add code to check the obvious (IST)

@kamil-holubicki, the check is already there because the reg expression checks for the message SST terminated by @ that's it. We do not need another terminator like <EOF>. The only relevant part of the message is SST after that the GTID:POS received is a plus to resolve the position in case the recovery position is not able to recover it.

There is synchronization issue here.
We are waiting in while [ ! -s $WSREP_SST_OPT_DATA/XST_FILE.txt ];do loop. Once the file is not empty, we exit the loop. Note that the content of the file may be SS (not all data written to the file)
Then we go to if [[ ! cat $WSREP_SST_OPT_DATA/XST_FILE.txt =~ "SST@" ]];then. This if will detect that no full SST is needed, as the file contains only SS

About IST only. I don't see any place where donor sends the info about IST. It sends info only if full SST is needed.
The point is, that we have to wait in the loop until we receive the whole message, then we can parse this message.

Whatever message is not SST@ script should try IST and exit, that is the correct behaviour.
If you can build a test case where only one "s" is sent (from the original action in the DONOR section) I will consider to implement a more sophisticated check on the string.
At the moment in all test done, also using qdisk to generate network interference, never report that as an issue.

About IST as you know you have the bypass instructions from galera on donor side and the joiner exit from SST script on bypass.
So not really need to do anything else.

where only one "s" is sent - It is not about only "s" being sent. The full message is always sent. I agree. The point is about the receiver side. If you show me the justification that netcat writes data to the file in the atomic way, I will agree. For now, on the receiver side, you may have "SS" written to the file, while "T@" still being in the netcat buffer. Then sst script tests if the file is empty -> no it is not empty, because it contains "SS". Then the decision is made that we should go for IST, not SST, but it is wrong.
I agree that it might work in all tests, because there is a very short time window when we have only "SS" stored in file. This can lead to very hard to investigate bugs in the future. We can avoid this in one of the following ways:

Prove that netcat writes the whole message atomically into the file (I don't think it is the case)

Design the protocol in the way that protects us

As for bypass, you are right. I missed this line:
echo $WSREP_SST_OPT_GTID | nc -w 1 $SST_HOST_STRIPPED $WSREP_SST_OPT_REMOTE_HOSTPORT || :
so the joiner is informed.

This PR fixes the above issue
Tusamarco#2

kamil-holubicki · 2024-11-22T09:59:59Z

scripts/wsrep_sst_clone.sh

+    wsrep_log_debug "-> WAIT DIR DONOR MESSAGE DONE"
+
+    if [[ ! `cat $WSREP_SST_OPT_DATA/XST_FILE.txt` =~ "SST@" ]];then
+        wsrep_log_info "DONOR SAY IST"


I don't see anything being sent by Donor in case of IST. We should send something like
IST<EOF>. This will allow the Joiner to immediately leave SST script.

Whatever is not SST you need to exit. That's it.
no need to check for other cases

Where the Donor sends the information that it is not SST? Without sending it, the joiner will wait in the while loop above, no?

Donor send a message on bypass in the same way it does for SST using netcat. So joiner will get the message exit the loop and exit the SST process given is not an SST message.

kamil-holubicki · 2024-11-22T10:01:49Z

sql/wsrep_sst.cc

@@ -1540,7 +1542,7 @@ static bool is_sst_request_valid(const std::string &msg) {
      Instead of this we will just allow alpha-num + a few special characters
      (colon, slash, dot, underscore, square brackets, hyphen). */
    std::string data = msg.substr(method_len + 1, data_len);
-    static const std::regex allowed_chars_regex("[\\w:/.[\\]-]+");
+    static const std::regex allowed_chars_regex("[\\w:/.[\\]@-]+");


I think we don't need this change if we pass the address in the form ip:port/path

I still think that having the path as you mention is not ok at least in standard cases.
If you check the Glaera documentation they say:

To signal that the node is ready to receive the state transfer, print the following string to standard output: ready <address>:port\n. Use the IP address and port at which the node is waiting for the state snapshot. For example: ready 192.168.1.1:4444

https://galeracluster.com/library/documentation/scriptable-sst.html

So using the standard uri for db connection host:port\termination which in this case evolves to user:pw@host:port\n The use of uri/path was probably added for XB and other attributes

As said in another comment, I inisist on using existing protocol, there is no reason for changing it.
The standard URI for the db connection is good thing, however, does not apply here.

As said above, no.
standard is clear also in codership documentation they follow the <address>:<ip>\n
If in any part of the code we are deviationg from what is the real standard we should correct that.

Few minor refactoring to accomodate comments on PR

Tusamarco and others added 20 commits August 2, 2024 15:26

inserting clone scripts

8f5dfa0

Working version of clone

13e77e4

This version has a patch to go straight to ist so is not fully functional unless removing that bypass

First version that works SST and IST

876f274

Working copy of wsrep_clone with PXC

0c5887c

SSL is NOT supported

adding needed GRANTS

b721c41

adding MTR tests

d44cab3

commenting unneeded debug

93315c9

fixing debug setting

1c2799b

Update mysql_system_users.sql

b92a948

bug. Wrong comma

INFO not DEBUG

3795899

trying to identify why is not reporting the clone status in the mtr test

9c852c8

fixing config missed wsrep allowed methods

trying to identify why is not reporting the clone status in the mtr test

d7bab63

fixing config missed wsrep allowed methods

trying to identify why is not reporting the clone status in the mtr test

f5ff5cc

fixing config missed wsrep allowed methods

Working version also on MTR. I still need to find a way to be sure ne…

e307fc3

…tcat is not running

SST clone with mtr successful

b8ff4e3

pre release

a971965

fix mtr

09d9a78

fixing result

87b3971

clean up and netcat removal

caa7bb1

Adding some help description

c7fdebb

Tusamarco assigned kamil-holubicki Oct 7, 2024

kamil-holubicki requested review from venkatesh-prasad-v and kamil-holubicki October 7, 2024 17:14

Tusamarco and others added 5 commits October 18, 2024 11:39

Change needed by different NetCat implementation on Debian

ab1fa17

Add the clone script to CMake to install them in the right location.

b74a565

Need to ad extension or make script will not identify them correctly

fix in the message

f9cf9d8

adjusting value to 1 given NC RH implementation does not support 0

b28c4d7

Merge branch 'percona:8.0' into PXC-4469-sst-clone

91938a8

Tusamarco and others added 3 commits October 30, 2024 17:51

fix permission adding DELETE

5832eb8

Merge branch 'percona:8.0' into PXC-4469-sst-clone

0f1dae1

Adding revoke on SST user to remove the role association before drop …

db53e22

…the user Some edit/cleanup as suggested by Kamil

kamil-holubicki reviewed Nov 12, 2024

View reviewed changes

scripts/wsrep_sst_common_clone.sh Outdated Show resolved Hide resolved

scripts/wsrep_sst_common_clone.sh Outdated Show resolved Hide resolved

Tusamarco added 2 commits November 19, 2024 19:11

Forgot to change the Cmake list

76663b0

venkatesh-prasad-v requested changes Nov 22, 2024

View reviewed changes

kamil-holubicki requested changes Nov 22, 2024

View reviewed changes

Tusamarco added 3 commits December 3, 2024 16:29

Enable SSL

2aad82e

Few minor refactoring to accomodate comments on PR

Fixing a bug for Debian netcat implementation

3bf9d1e

removing orphan code.

77bb2a7

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Pxc 4469 sst clone #1961

Pxc 4469 sst clone #1961

Tusamarco commented Oct 7, 2024

it-percona-cla commented Oct 7, 2024 •

edited

Loading

Tusamarco commented Oct 7, 2024

kamil-holubicki commented Nov 12, 2024

Tusamarco commented Nov 20, 2024

venkatesh-prasad-v Nov 22, 2024

Tusamarco Nov 22, 2024

Tusamarco Dec 2, 2024

Tusamarco Dec 3, 2024

kamil-holubicki Nov 22, 2024 •

edited

Loading

kamil-holubicki Dec 5, 2024

Tusamarco Dec 5, 2024

kamil-holubicki Dec 5, 2024 •

edited

Loading

kamil-holubicki Nov 22, 2024

Tusamarco Dec 3, 2024

kamil-holubicki Nov 22, 2024 •

edited

Loading

Tusamarco Nov 22, 2024

Tusamarco Dec 2, 2024

kamil-holubicki Dec 5, 2024

Tusamarco Dec 5, 2024

kamil-holubicki Dec 5, 2024

kamil-holubicki Dec 5, 2024

kamil-holubicki Nov 22, 2024

Tusamarco Dec 2, 2024

kamil-holubicki Dec 5, 2024

Tusamarco Dec 5, 2024

kamil-holubicki Nov 22, 2024

Tusamarco Dec 2, 2024 •

edited

Loading

kamil-holubicki Dec 5, 2024 •

edited

Loading

Tusamarco Dec 5, 2024


		wsrep_log_debug "-> WAIT DIR DONOR MESSAGE DONE"

		if [[ ! `cat $WSREP_SST_OPT_DATA/XST_FILE.txt` =~ "SST@" ]];then

Pxc 4469 sst clone #1961

Are you sure you want to change the base?

Pxc 4469 sst clone #1961

Conversation

Tusamarco commented Oct 7, 2024

it-percona-cla commented Oct 7, 2024 • edited Loading

Tusamarco commented Oct 7, 2024

kamil-holubicki commented Nov 12, 2024

Tusamarco commented Nov 20, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kamil-holubicki Nov 22, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kamil-holubicki Dec 5, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kamil-holubicki Nov 22, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Tusamarco Dec 2, 2024 • edited Loading

Choose a reason for hiding this comment

kamil-holubicki Dec 5, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

it-percona-cla commented Oct 7, 2024 •

edited

Loading

kamil-holubicki Nov 22, 2024 •

edited

Loading

kamil-holubicki Dec 5, 2024 •

edited

Loading

kamil-holubicki Nov 22, 2024 •

edited

Loading

Tusamarco Dec 2, 2024 •

edited

Loading

kamil-holubicki Dec 5, 2024 •

edited

Loading