Skip to content
/ server Public

Commit d299cd2

Browse files
ParadoxV5ParadoxV5
committed
MDEV-28302: address review
* Improve `DEFAULT` test results * Expand comments * `field_functor` → `mi_functor`
1 parent b5c22ed commit d299cd2

File tree

3 files changed

+226
-54
lines changed

3 files changed

+226
-54
lines changed

‎mysql-test/main/change_master_default.result‎

Lines changed: 181 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
# Start of main.change_master_default
12
CREATE PROCEDURE show_defaultable_fields()
23
SELECT connection_name,
34
connect_retry,
@@ -46,16 +47,93 @@ master_retry_count= 150000,
4647
master_heartbeat_period= 45,
4748
master_host='127.0.0.1';
4849
CALL show_defaultable_fields();
49-
connection_name connect_retry master_ssl_allowed master_ssl_ca_file master_ssl_ca_path master_ssl_cert master_ssl_cipher master_ssl_key master_ssl_verify_server_cert master_ssl_crl master_ssl_crlpath using_gtid master_retry_count slave_heartbeat_period
50-
90 No specified_ca specified_capath specified_cert specified_cipher specified_key No specified_crl specified_crlpath No 150000 45.000
51-
defaulted 60 Yes Yes Slave_Pos 100000 60.000
52-
unset 60 Yes Yes Slave_Pos 100000 60.000
50+
connection_name
51+
connect_retry 90
52+
master_ssl_allowed No
53+
master_ssl_ca_file specified_ca
54+
master_ssl_ca_path specified_capath
55+
master_ssl_cert specified_cert
56+
master_ssl_cipher specified_cipher
57+
master_ssl_key specified_key
58+
master_ssl_verify_server_cert No
59+
master_ssl_crl specified_crl
60+
master_ssl_crlpath specified_crlpath
61+
using_gtid No
62+
master_retry_count 150000
63+
slave_heartbeat_period 45.000
64+
connection_name defaulted
65+
connect_retry 60
66+
master_ssl_allowed Yes
67+
master_ssl_ca_file
68+
master_ssl_ca_path
69+
master_ssl_cert
70+
master_ssl_cipher
71+
master_ssl_key
72+
master_ssl_verify_server_cert Yes
73+
master_ssl_crl
74+
master_ssl_crlpath
75+
using_gtid Slave_Pos
76+
master_retry_count 100000
77+
slave_heartbeat_period 60.000
78+
connection_name unset
79+
connect_retry 60
80+
master_ssl_allowed Yes
81+
master_ssl_ca_file
82+
master_ssl_ca_path
83+
master_ssl_cert
84+
master_ssl_cipher
85+
master_ssl_key
86+
master_ssl_verify_server_cert Yes
87+
master_ssl_crl
88+
master_ssl_crlpath
89+
using_gtid Slave_Pos
90+
master_retry_count 100000
91+
slave_heartbeat_period 60.000
92+
# Those set or left as `DEFAULT` should pick up changes to defaults.
5393
# restart: --skip-slave-start --master-connect-retry=30 --skip-master-ssl --master-ssl-ca=default_ca --master-ssl-capath=default_capath --master-ssl-cert=default_cert --master-ssl-cipher=default_cipher --master-ssl-key=default_key --skip-master-ssl-verify-server-cert --master-ssl-crl=default_crl --master-ssl-crlpath=default_crlpath --master-use-gtid=CURRENT_POS --master-retry-count=50000 --master-heartbeat-period=15
5494
CALL show_defaultable_fields();
55-
connection_name connect_retry master_ssl_allowed master_ssl_ca_file master_ssl_ca_path master_ssl_cert master_ssl_cipher master_ssl_key master_ssl_verify_server_cert master_ssl_crl master_ssl_crlpath using_gtid master_retry_count slave_heartbeat_period
56-
90 No specified_ca specified_capath specified_cert specified_cipher specified_key No specified_crl specified_crlpath No 150000 45.000
57-
defaulted 30 No default_ca default_capath default_cert default_cipher default_key No default_crl default_crlpath Current_Pos 50000 15.000
58-
unset 30 No default_ca default_capath default_cert default_cipher default_key No default_crl default_crlpath Current_Pos 50000 15.000
95+
connection_name
96+
connect_retry 90
97+
master_ssl_allowed No
98+
master_ssl_ca_file specified_ca
99+
master_ssl_ca_path specified_capath
100+
master_ssl_cert specified_cert
101+
master_ssl_cipher specified_cipher
102+
master_ssl_key specified_key
103+
master_ssl_verify_server_cert No
104+
master_ssl_crl specified_crl
105+
master_ssl_crlpath specified_crlpath
106+
using_gtid No
107+
master_retry_count 150000
108+
slave_heartbeat_period 45.000
109+
connection_name defaulted
110+
connect_retry 30
111+
master_ssl_allowed No
112+
master_ssl_ca_file default_ca
113+
master_ssl_ca_path default_capath
114+
master_ssl_cert default_cert
115+
master_ssl_cipher default_cipher
116+
master_ssl_key default_key
117+
master_ssl_verify_server_cert No
118+
master_ssl_crl default_crl
119+
master_ssl_crlpath default_crlpath
120+
using_gtid Current_Pos
121+
master_retry_count 50000
122+
slave_heartbeat_period 15.000
123+
connection_name unset
124+
connect_retry 30
125+
master_ssl_allowed No
126+
master_ssl_ca_file default_ca
127+
master_ssl_ca_path default_capath
128+
master_ssl_cert default_cert
129+
master_ssl_cipher default_cipher
130+
master_ssl_key default_key
131+
master_ssl_verify_server_cert No
132+
master_ssl_crl default_crl
133+
master_ssl_crlpath default_crlpath
134+
using_gtid Current_Pos
135+
master_retry_count 50000
136+
slave_heartbeat_period 15.000
59137
SET @@GLOBAL.slave_net_timeout= 100;
60138
SELECT connection_name, slave_heartbeat_period
61139
FROM information_schema.slave_status ORDER BY connection_name;
@@ -78,25 +156,111 @@ master_use_gtid= DEFAULT,
78156
master_retry_count= DEFAULT,
79157
master_heartbeat_period= DEFAULT;
80158
CALL show_defaultable_fields();
81-
connection_name connect_retry master_ssl_allowed master_ssl_ca_file master_ssl_ca_path master_ssl_cert master_ssl_cipher master_ssl_key master_ssl_verify_server_cert master_ssl_crl master_ssl_crlpath using_gtid master_retry_count slave_heartbeat_period
82-
30 No default_ca default_capath default_cert default_cipher default_key No default_crl default_crlpath Current_Pos 50000 15.000
83-
defaulted 30 No default_ca default_capath default_cert default_cipher default_key No default_crl default_crlpath Current_Pos 50000 15.000
84-
unset 30 No default_ca default_capath default_cert default_cipher default_key No default_crl default_crlpath Current_Pos 50000 15.000
159+
connection_name
160+
connect_retry 30
161+
master_ssl_allowed No
162+
master_ssl_ca_file default_ca
163+
master_ssl_ca_path default_capath
164+
master_ssl_cert default_cert
165+
master_ssl_cipher default_cipher
166+
master_ssl_key default_key
167+
master_ssl_verify_server_cert No
168+
master_ssl_crl default_crl
169+
master_ssl_crlpath default_crlpath
170+
using_gtid Current_Pos
171+
master_retry_count 50000
172+
slave_heartbeat_period 15.000
173+
connection_name defaulted
174+
connect_retry 30
175+
master_ssl_allowed No
176+
master_ssl_ca_file default_ca
177+
master_ssl_ca_path default_capath
178+
master_ssl_cert default_cert
179+
master_ssl_cipher default_cipher
180+
master_ssl_key default_key
181+
master_ssl_verify_server_cert No
182+
master_ssl_crl default_crl
183+
master_ssl_crlpath default_crlpath
184+
using_gtid Current_Pos
185+
master_retry_count 50000
186+
slave_heartbeat_period 15.000
187+
connection_name unset
188+
connect_retry 30
189+
master_ssl_allowed No
190+
master_ssl_ca_file default_ca
191+
master_ssl_ca_path default_capath
192+
master_ssl_cert default_cert
193+
master_ssl_cipher default_cipher
194+
master_ssl_key default_key
195+
master_ssl_verify_server_cert No
196+
master_ssl_crl default_crl
197+
master_ssl_crlpath default_crlpath
198+
using_gtid Current_Pos
199+
master_retry_count 50000
200+
slave_heartbeat_period 15.000
85201
RESET REPLICA 'unset' ALL;
86202
CHANGE MASTER 'unset' TO master_host='127.0.1.3';
203+
# Validate command line options
87204
# restart_abort: --master-heartbeat-period=''
88205
# restart_abort: --master-heartbeat-period=123abc
89206
# restart_abort: --master-heartbeat-period=-1
90207
# restart_abort: --master-heartbeat-period=4294967.296
91208
# restart: --skip-slave-start --master-heartbeat-period=0.000499
92-
CALL mtr.add_suppression('.*master-heartbeat-period.+0.* disabl.+');
209+
SELECT connection_name, slave_heartbeat_period
210+
FROM information_schema.slave_status ORDER BY connection_name;
211+
connection_name slave_heartbeat_period
212+
0.000
213+
defaulted 0.000
214+
unset 0.000
215+
CALL mtr.add_suppression('.*master-heartbeat-period.+0.*disabl.+');
216+
FOUND 1 /\[Warning\] .*master-heartbeat-period.+0.*disabl.+/ in mysqld.1.err
93217
# restart: --skip-slave-start --skip-master-ssl --master-ssl --skip-master-ssl-verify-server-cert --master-ssl-verify-server-cert --master-use-gtid=NO --autoset-master-use-gtid --master-heartbeat-period=45 --autoset-master-heartbeat-period
94218
CALL show_defaultable_fields();
95-
connection_name connect_retry master_ssl_allowed master_ssl_ca_file master_ssl_ca_path master_ssl_cert master_ssl_cipher master_ssl_key master_ssl_verify_server_cert master_ssl_crl master_ssl_crlpath using_gtid master_retry_count slave_heartbeat_period
96-
60 Yes Yes Slave_Pos 100000 60.000
97-
defaulted 60 Yes Yes Slave_Pos 100000 60.000
98-
unset 60 Yes Yes Slave_Pos 100000 60.000
219+
connection_name
220+
connect_retry 60
221+
master_ssl_allowed Yes
222+
master_ssl_ca_file
223+
master_ssl_ca_path
224+
master_ssl_cert
225+
master_ssl_cipher
226+
master_ssl_key
227+
master_ssl_verify_server_cert Yes
228+
master_ssl_crl
229+
master_ssl_crlpath
230+
using_gtid Slave_Pos
231+
master_retry_count 100000
232+
slave_heartbeat_period 60.000
233+
connection_name defaulted
234+
connect_retry 60
235+
master_ssl_allowed Yes
236+
master_ssl_ca_file
237+
master_ssl_ca_path
238+
master_ssl_cert
239+
master_ssl_cipher
240+
master_ssl_key
241+
master_ssl_verify_server_cert Yes
242+
master_ssl_crl
243+
master_ssl_crlpath
244+
using_gtid Slave_Pos
245+
master_retry_count 100000
246+
slave_heartbeat_period 60.000
247+
connection_name unset
248+
connect_retry 60
249+
master_ssl_allowed Yes
250+
master_ssl_ca_file
251+
master_ssl_ca_path
252+
master_ssl_cert
253+
master_ssl_cipher
254+
master_ssl_key
255+
master_ssl_verify_server_cert Yes
256+
master_ssl_crl
257+
master_ssl_crlpath
258+
using_gtid Slave_Pos
259+
master_retry_count 100000
260+
slave_heartbeat_period 60.000
261+
# Clean-up
99262
DROP PROCEDURE show_defaultable_fields;
100263
RESET REPLICA 'unset' ALL;
101264
RESET REPLICA 'defaulted' ALL;
102265
RESET REPLICA ALL;
266+
# End of main.change_master_default

‎mysql-test/main/change_master_default.test‎

Lines changed: 36 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
--echo # Start of main.change_master_default
2+
13
# MDEV-28302: Test how `CHANGE MASTER [named]` reads server
24
# options for its default values and the lack of those options.
35
# The test creates multiple CHANGE MASTER connections,
@@ -44,7 +46,6 @@ CHANGE MASTER 'defaulted' TO
4446
master_heartbeat_period= DEFAULT,
4547
master_host= '127.0.1.2';
4648

47-
# This subject does not stay constant.
4849
CHANGE MASTER TO # Default master does not replace named masters
4950
master_connect_retry= 90,
5051
master_ssl= FALSE,
@@ -62,27 +63,21 @@ CHANGE MASTER TO # Default master does not replace named masters
6263
master_host='127.0.0.1';
6364

6465
# Show freshly created configurations
65-
CALL show_defaultable_fields();
66+
--query_vertical CALL show_defaultable_fields()
6667

6768

68-
# Those set or left as `DEFAULT` should pick up changes to defaults.
69+
--echo # Those set or left as `DEFAULT` should pick up changes to defaults.
6970

7071
--let $restart_parameters= --skip-slave-start --master-connect-retry=30 --skip-master-ssl --master-ssl-ca=default_ca --master-ssl-capath=default_capath --master-ssl-cert=default_cert --master-ssl-cipher=default_cipher --master-ssl-key=default_key --skip-master-ssl-verify-server-cert --master-ssl-crl=default_crl --master-ssl-crlpath=default_crlpath --master-use-gtid=CURRENT_POS --master-retry-count=50000 --master-heartbeat-period=15
7172
--source include/restart_mysqld.inc # not_embedded
72-
73-
CALL show_defaultable_fields();
74-
73+
--query_vertical CALL show_defaultable_fields()
7574

7675
# The `DEFAULT` of `master_heartbeat_period` changes with `@@slave_net_timeout`.
77-
7876
SET @@GLOBAL.slave_net_timeout= 100;
79-
8077
SELECT connection_name, slave_heartbeat_period
8178
FROM information_schema.slave_status ORDER BY connection_name;
8279

83-
84-
# `DEFAULT` overwrites the existing configs only for the CHANGEd connection.
85-
80+
# `DEFAULT` overwrites the existing configs for only the CHANGEd connection.
8681
CHANGE MASTER TO
8782
master_connect_retry= DEFAULT,
8883
master_ssl= DEFAULT,
@@ -97,49 +92,58 @@ CHANGE MASTER TO
9792
master_use_gtid= DEFAULT,
9893
master_retry_count= DEFAULT,
9994
master_heartbeat_period= DEFAULT;
95+
--query_vertical CALL show_defaultable_fields()
10096

101-
CALL show_defaultable_fields();
102-
103-
104-
# Recreate 'unset' to test that it saves `DEFAULT`, not the options' values
97+
# Recreate 'unset' to see later that it saves `DEFAULT`, not the options' values
10598
--disable_warnings
10699
RESET REPLICA 'unset' ALL;
107100
--enable_warnings
108101
CHANGE MASTER 'unset' TO master_host='127.0.1.3';
109102

103+
104+
--echo # Validate command line options
105+
110106
# Invalid `--master-heartbeat-period` values should abort the server
107+
# (`restart_abort` includes a wait for the server to exit on its own,
108+
# e.g., due to a startup error.)
111109
--source include/shutdown_mysqld.inc
112-
--let $restart_parameters= --master-heartbeat-period=''
113-
--echo # restart_abort: $restart_parameters
114-
--write_line "restart_abort: $restart_parameters" $_expect_file_name
115-
--let $restart_parameters= --master-heartbeat-period=123abc
116-
--echo # restart_abort: $restart_parameters
117-
--write_line "restart_abort: $restart_parameters" $_expect_file_name
118-
--let $restart_parameters= --master-heartbeat-period=-1
119-
--echo # restart_abort: $restart_parameters
120-
--write_line "restart_abort: $restart_parameters" $_expect_file_name
121-
--let $restart_parameters= --master-heartbeat-period=4294967.296
122-
--echo # restart_abort: $restart_parameters
123-
--write_line "restart_abort: $restart_parameters" $_expect_file_name
110+
--let $restart_parameters= restart_abort: --master-heartbeat-period=''
111+
--echo # $restart_parameters
112+
--write_line "$restart_parameters" $_expect_file_name
113+
--let $restart_parameters= restart_abort: --master-heartbeat-period=123abc
114+
--echo # $restart_parameters
115+
--write_line "$restart_parameters" $_expect_file_name
116+
--let $restart_parameters= restart_abort: --master-heartbeat-period=-1
117+
--echo # $restart_parameters
118+
--write_line "$restart_parameters" $_expect_file_name
119+
--let $restart_parameters= restart_abort: --master-heartbeat-period=4294967.296
120+
--echo # $restart_parameters
121+
--write_line "$restart_parameters" $_expect_file_name
124122

125123
# Numbers between 0 and 0.5 exclusive should warn about rounding to 0 (disabled)
126124
--let $restart_parameters= --skip-slave-start --master-heartbeat-period=0.000499
127125
--source include/start_mysqld.inc
128-
CALL mtr.add_suppression('.*master-heartbeat-period.+0.* disabl.+');
126+
SELECT connection_name, slave_heartbeat_period
127+
FROM information_schema.slave_status ORDER BY connection_name;
128+
--let $regexp= .*master-heartbeat-period.+0.*disabl.+
129+
--eval CALL mtr.add_suppression('$regexp')
130+
--let SEARCH_FILE= `SELECT @@log_error`
131+
--let SEARCH_PATTERN= \[Warning\] $regexp
132+
--source include/search_pattern_in_file.inc
129133

130134
# Test prefixes: adding `auto-` and omitting `skip-`
131135
--let $restart_parameters= --skip-slave-start --skip-master-ssl --master-ssl --skip-master-ssl-verify-server-cert --master-ssl-verify-server-cert --master-use-gtid=NO --autoset-master-use-gtid --master-heartbeat-period=45 --autoset-master-heartbeat-period
132136
--source include/restart_mysqld.inc
133137

134-
CALL show_defaultable_fields();
138+
--query_vertical CALL show_defaultable_fields()
135139

136140

137-
# Clean-up
141+
--echo # Clean-up
138142

139143
DROP PROCEDURE show_defaultable_fields;
140144

141145
RESET REPLICA 'unset' ALL;
142146
RESET REPLICA 'defaulted' ALL;
143147
RESET REPLICA ALL;
144-
# check-testcase
145-
#CHANGE MASTER TO master_user= 'root', master_ssl_verify_server_cert= FALSE;
148+
149+
--echo # End of main.change_master_default

‎sql/sql_lex.h‎

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -382,13 +382,17 @@ struct LEX_MASTER_INFO
382382
repl_do_domain_ids_opt, repl_ignore_domain_ids_opt;
383383

384384
/**TODO
385-
It is possible to apply CHANGE MASTER configs during parsing
386-
(in sql_yacc.yy) without stashing them in a @ref LEX_MASTER_INFO.
387-
For now, lambdas in the parser code demonstrates this concept while
385+
Going through this struct means it must contain a repeated set of CHANGE
386+
MASTER and START SLAVE variables that additionally knows which values are
387+
not changing, not to mention support for `CHANGE MASTER ...= DEFAULT`.
388+
This creates complexity and leads to inconsistency.
389+
Instead, it is possible to track and apply CHANGE MASTER configs during
390+
parsing (in `sql_yacc.yy`) without stashing them in a @ref LEX_MASTER_INFO.
391+
But for now, lambdas in `sql_yacc.yy` demonstrates this concept while
388392
keeping them deferred to the "post-processing" in change_master().
389393
*/
390-
using field_functor= std::function<void(Master_info_file *mi)>;
391-
field_functor connect_retry, heartbeat_period, ssl,
394+
using mi_functor= std::function<void(Master_info_file *mi)>;
395+
mi_functor connect_retry, heartbeat_period, ssl,
392396
ssl_key, ssl_cert, ssl_ca, ssl_capath, ssl_cipher, ssl_crl, ssl_crlpath,
393397
ssl_verify_server_cert, retry_count, use_gtid;
394398

0 commit comments

Comments
 (0)