MDEV-26479: Add support for slave_skip_errors="ddl_exist_errors"#4780
MDEV-26479: Add support for slave_skip_errors="ddl_exist_errors"#4780Mahmoud-kh1 wants to merge 1 commit intoMariaDB:mainfrom
Conversation
gkodinov
added
the
External Contribution
gkodinov
changed the title
gkodinov
left a comment
gkodinov
left a comment
There was a problem hiding this comment.
Thank you for your contribution! This is a preliminary review.
sql/slave.cc
Outdated
| bitmap_set_all(&slave_error_mask); | ||
| goto end; | ||
| } | ||
| if (!system_charset_info->strnncoll((uchar*)arg,16,(const uchar*)"ddl_exist_errors",16)) |
There was a problem hiding this comment.
please use STRING_WITH_LEN here.
sql/slave.cc
Outdated
| ER_CANT_DROP_FIELD_OR_KEY, | ||
| ER_NO_SUCH_TABLE | ||
| }; | ||
| uint n_errors = sizeof(ddl_errors) / sizeof(ddl_errors[0]); |
Implement ddl_exist_errors alias option for slave_skip_errors which allows slave to skip DDL errors such as: ER_DB_CREATE_EXISTS ER_DB_DROP_EXISTS ER_TABLE_EXISTS_ERROR ER_BAD_TABLE_ERROR ER_BAD_FIELD_ERROR ER_DUP_FIELDNAME ER_DUP_KEYNAME ER_MULTIPLE_PRI_KEY ER_CANT_DROP_FIELD_OR_KEY ER_NO_SUCH_TABLE Test: Added rpl.rpl_skip_errors_ddl test to verify all errors are correctly skipped.
Mahmoud-kh1
force-pushed
the
ddl_exist_errors_alias
branch
from
cd8c851 to
4859684
Compare
| bitmap_set_all(&slave_error_mask); | ||
| goto end; | ||
| } | ||
|
|
There was a problem hiding this comment.
This will only work if the ddl_exist_errors is the first in the list. How about "1234, ddl_exist_errors, 456" ?
Move the check inside the for() loop and make sure to skip over leading and trailing spaces, if any. I'd put the whole thing into a small utility function, similar to str2int().
There was a problem hiding this comment.
yeah it will fail for "1234, ddl_exist_errors, 456" I just make it like mysql but I agree with you we shouldn't say we treat it like any error codes and this is not valid.
also making a separate function will be more cleaner if we added more aliases in future so I will make one that take the string and return array that holds error codes , I think I will move all logic of parsing including aliases there making init function just set the bitmap with error codes , is this what you mean ?
There was a problem hiding this comment.
I will just move the check inside the loop I think it doesn't worth to make a another function and If we needed to add more aliases in future it will be better to go with on of your approaches which you introduced in zulip.
what do you think ?
| } | ||
|
|
||
| if (!system_charset_info->strnncoll((uchar*)arg,sizeof("ddl_exist_errors")-1, | ||
| (const uchar*)STRING_WITH_LEN("ddl_exist_errors"))) |
2 participants
Description:
This PR add the ddl_exist_errors alias option for the slave_skip_errors which allows slaves to skip DDL errors.
Errors skipped:
ER_DB_CREATE_EXISTS
ER_DB_DROP_EXISTS
ER_TABLE_EXISTS_ERROR
ER_BAD_TABLE_ERROR
ER_BAD_FIELD_ERROR
ER_DUP_FIELDNAME
ER_DUP_KEYNAME
ER_MULTIPLE_PRI_KEY
ER_CANT_DROP_FIELD_OR_KEY
ER_NO_SUCH_TABLE
How the pr can be tested
we run this test
./mysql-test-run.pl rpl.rpl_slave_skip_errors_ddlthat verify all errors skipped correctly