Details
-
Bug
-
Status: Stalled (View Workflow)
-
Major
-
Resolution: Unresolved
-
10.5, 10.6, 10.11, 11.4, 11.7(EOL), 11.8
Description
When using START SLAVE UNTIL, the UNTIL grammar rule uses master_file_def (through slave_until_opts), which is also used by CHANGE MASTER TO. This means that a START SLAVE UNTIL statement can use any option available to the master_file_def rule (e.g. master_use_gtid, master_demote_to_slave), despite not being valid for START SLAVE. For example:
START SLAVE UNTIL relay_log_file='mariadb-bin.000002', relay_log_pos=50, master_demote_to_slave=1, master_use_gtid=no;
|
The only constraint is that the master/relay log file/pos have to be set:
| UNTIL_SYM slave_until_opts
|
{
|
LEX *lex=Lex;
|
if (unlikely(((lex->mi.log_file_name || lex->mi.pos) &&
|
(lex->mi.relay_log_name || lex->mi.relay_log_pos)) ||
|
!((lex->mi.log_file_name && lex->mi.pos) ||
|
(lex->mi.relay_log_name && lex->mi.relay_log_pos))))
|
my_yyabort_error((ER_BAD_SLAVE_UNTIL_COND, MYF(0)));
|
}
|
The valid options for START SLAVE UNTIL should be split from master_file_def to disallow this.
Attachments
Activity
| Field | Original Value | New Value |
|---|---|---|
| Summary | START SLAVE UNTIL uses CHANGE MASTER TO syntax | START SLAVE UNTIL uses CHANGE MASTER TO options |
| Summary | START SLAVE UNTIL uses CHANGE MASTER TO options | START SLAVE UNTIL allows CHANGE MASTER TO options |
Can I work on this issue ?
From what I have seen the only thing that needs to be done is to separate this
slave_until_opts:
|
master_file_def
|
| slave_until_opts ',' master_file_def
|
;
|
into a separate slave_until_file_def instead of getting everything in the master_file_def and just allow these only instead of everything in master_file_def
MASTER_LOG_FILE_SYM, MASTER_LOG_POS_SYM, RELAY_LOG_FILE_SYM, RELAY_LOG_POS_SYM
Also probably there needs to be a new test case for trying out that query where it should fail
START SLAVE UNTIL relay_log_file='mariadb-bin.000002', relay_log_pos=50, master_demote_to_slave=1, master_use_gtid=no; |
What do you think ? @Brandon Nesterenko
Please do, Mohanad Khaled! Your plan sounds good to me 
Note too, as this is just parsing, you don't really need to put the test in the rpl suite, as those tests actually deal with multiple server instances. Just one server should suffice for this, so putting the test in main should be good.
| Labels | beginner-friendly |
Great .. and how about the fix version ? should this be applied to 12 or 11.9 ?
I'd say to just target the main branch. So when it is ready, it will just be a part of the next release cycle. Most likely, this will end up being 12.1, as 12.0 has just had its preview release.
| Fix Version/s | 12.1 [ 29992 ] | |
| Fix Version/s | 12.0 [ 29945 ] |
| Labels | beginner-friendly | beginner-friendly contribution |
| Status | Open [ 1 ] | In Progress [ 3 ] |
| Status | In Progress [ 3 ] | In Review [ 10002 ] |
| Assignee | Brandon Nesterenko [ JIRAUSER48702 ] | Susil Behera [ JIRAUSER40751 ] |
Hi susil.behera!
I've given this a round of review and approved. When you have a chance, could you look over the MTR tests in the patch and give your thoughts as well? It should be quick.
| Assignee | Susil Behera [ JIRAUSER40751 ] | Brandon Nesterenko [ JIRAUSER48702 ] |
| Assignee | Brandon Nesterenko [ JIRAUSER48702 ] | Susil Behera [ JIRAUSER40751 ] |
| Assignee | Susil Behera [ JIRAUSER40751 ] | Brandon Nesterenko [ JIRAUSER48702 ] |
| Status | In Review [ 10002 ] | Stalled [ 10000 ] |
Targeted non-GA (11.9) because there aren't actually any negative side-effects of this "bug", as the change_master() function is never invoked. So it is effectively just a refactoring.