Skip to content
/ server Public

Comments

MDEV-7394 : Making slave_skip_errors writable at runtime when slave is stopped #4634

Open
Mahmoud-kh1 wants to merge 4 commits intoMariaDB:mainfrom
Mahmoud-kh1:dynamic-slave-skip-error
Open

MDEV-7394 : Making slave_skip_errors writable at runtime when slave is stopped #4634
Mahmoud-kh1 wants to merge 4 commits intoMariaDB:mainfrom
Mahmoud-kh1:dynamic-slave-skip-error

Conversation

@Mahmoud-kh1
Copy link

@Mahmoud-kh1 Mahmoud-kh1 commented Feb 8, 2026

Now we can update slave_skip_errors at runtime when slaves stopped

  • This feature makes the slave_skip_errors system variable dynamic allowing it
    to be changed at runtime when the replication slave is stopped.
  • Previously slave_skip_errors was read only at runtime and required a
    server restart to be changed.
  • Runtime updates are now validated and safely rejected when the slave is
    running preventing inconsistent replication state.

Key Changes

  • Added ON_CHECK handler to verify that updates are only allowed while the
    slave is stopped.
  • Added ON_UPDATE handler to reinitialize the internal skip error state
    when the variable is changed.
  • Added an rpl mtr test that verifies slave_skip_errors can be changed
    dynamically when the slave is stopped and verified that updates are rejected while the slave is running.

behavior now is like that :
test slave2

Feature :
MDEV-7394

@CLAassistant
Copy link

CLAassistant commented Feb 8, 2026

CLA assistant check
All committers have signed the CLA.

@Mahmoud-kh1 Mahmoud-kh1 force-pushed the dynamic-slave-skip-error branch 6 times, most recently from 87dbe8f to 3fa1017 Compare February 9, 2026 07:34
@Mahmoud-kh1
Copy link
Author

Mahmoud-kh1 commented Feb 9, 2026

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
sys_vars.sysvars_server_notembedded
main.variables-notembedded
sys_vars.slave_skip_errors_basic

@gkodinov gkodinov added the External Contribution All PRs from entities outside of MariaDB Foundation, Corporation, Codership agreements. label Feb 9, 2026
Copy link
Member

@gkodinov gkodinov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@grooverdan
Copy link
Member

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.
@Mahmoud-kh1 Mahmoud-kh1 force-pushed the dynamic-slave-skip-error branch from 3fa1017 to 440eec2 Compare February 12, 2026 12:41
Copy link
Member

@gkodinov gkodinov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.
@Mahmoud-kh1 Mahmoud-kh1 force-pushed the dynamic-slave-skip-error branch 7 times, most recently from b28dbed to 4b94518 Compare February 19, 2026 14:59
@Mahmoud-kh1 Mahmoud-kh1 requested a review from gkodinov February 20, 2026 11:42
Copy link
Member

@gkodinov gkodinov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please also address the buildbot failure. It seems related.

@Mahmoud-kh1 Mahmoud-kh1 force-pushed the dynamic-slave-skip-error branch 3 times, most recently from d21fd5b to 5af34c9 Compare February 20, 2026 14:07
@Mahmoud-kh1 Mahmoud-kh1 requested a review from gkodinov February 20, 2026 15:46
@Mahmoud-kh1 Mahmoud-kh1 force-pushed the dynamic-slave-skip-error branch 2 times, most recently from da3e178 to b294419 Compare February 21, 2026 14:17
@Mahmoud-kh1 Mahmoud-kh1 force-pushed the dynamic-slave-skip-error branch from b294419 to 450161b Compare February 21, 2026 14:46
Copy link
Member

@gkodinov gkodinov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

here and below: why the change?

Copy link
Author

@Mahmoud-kh1 Mahmoud-kh1 Feb 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Image

so I opened the .opt file where the the initial values is defined and I found it in the same order they appear now

Image

so I suppose it shows the values in the same order as they were set.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

External Contribution All PRs from entities outside of MariaDB Foundation, Corporation, Codership agreements.

Development

Successfully merging this pull request may close these issues.

4 participants