MDEV-7394 : Making slave_skip_errors writable at runtime when slave is stopped #4634
MDEV-7394 : Making slave_skip_errors writable at runtime when slave is stopped #4634Mahmoud-kh1 wants to merge 4 commits intoMariaDB:mainfrom
Conversation
87dbe8f to
3fa1017
Compare
|
The following test cases fail because they assume that slave_skip_errors is read only which is no longer true which making their checks fail also |
gkodinov
left a comment
There was a problem hiding this comment.
This is a preliminary review. I'd like to request changes mostly because there are tests that need to be re-recorded (and possibly fixed too).
The rest of the comments are just my own limited take on the change. Feel free to ignore and leave for the final review.
|
Thank you so much for implementing my 11 year old bug report. I'd be very grateful if you stick through the review process on this. There's a lot to keep correct in the server to implement this change. |
Make slave_skip_errors dynamic so it can be changed while the slave is stopped. Attempts to change it while the slave is running are rejected with a clear error.
3fa1017 to
440eec2
Compare
gkodinov
left a comment
There was a problem hiding this comment.
Please just fix the spacing issues. The rest is pretty much as one expects it to be. Aside from some polishing. Once the space changes are gone I will do another round and approve it.
Removed unnecessary blank lines before the conditional check.
b28dbed to
4b94518
Compare
gkodinov
left a comment
There was a problem hiding this comment.
Please also address the buildbot failure. It seems related.
d21fd5b to
5af34c9
Compare
da3e178 to
b294419
Compare
b294419 to
450161b
Compare
gkodinov
left a comment
There was a problem hiding this comment.
LGTM. Please do not do changes that are not relevant to the fix (like the one outlined below).
Please stand by for the final review.
| show variables like 'slave_skip_errors'; | ||
| Variable_name Value | ||
| slave_skip_errors 0,3,100,137,643,1752 | ||
| slave_skip_errors 3,100,137,0,643,1752 |
There was a problem hiding this comment.
here and below: why the change?
There was a problem hiding this comment.
I also suspected that the issue might be related to the modifications I made. however, I run the same test on the main branch and it appears the same order.
The test is failing in main branch because the variable is now global and the test has not fixed for that I think (I can make an issue for that and pr to fix it).
so I opened the .opt file where the the initial values is defined and I found it in the same order they appear now
so I suppose it shows the values in the same order as they were set.
Now we can update slave_skip_errors at runtime when slaves stopped
to be changed at runtime when the replication slave is stopped.
server restart to be changed.
running preventing inconsistent replication state.
Key Changes
slave is stopped.
when the variable is changed.
dynamically when the slave is stopped and verified that updates are rejected while the slave is running.
behavior now is like that :

Feature :
MDEV-7394