MDEV-7394 : Making slave_skip_errors writable at runtime when slave is stopped #4634
Conversation
|
|
Mahmoud-kh1
force-pushed
the
dynamic-slave-skip-error
branch
5 times, most recently
from
e972236 to
87dbe8f
Compare
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
force-pushed
the
dynamic-slave-skip-error
branch
from
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
added
the
External Contribution
gkodinov
left a comment
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.
| /* Make @@slave_skip_errors show the nice human-readable value. */ | ||
| opt_slave_skip_errors= slave_skip_error_names; | ||
| /* we should not touch opt_slave_skip_errors here. we just build the printable string only. */ | ||
| (void) opt_slave_skip_errors; |
There was a problem hiding this comment.
remove all these lines please.
| { | ||
| bool res= false; | ||
| mysql_mutex_unlock(&LOCK_global_system_variables); | ||
| mysql_mutex_lock(&LOCK_active_mi); |
There was a problem hiding this comment.
I believe you have a locking issue to sort out. LOCK_active_mi is definitely not the right lock to take for this. I'm not going to pursue on this one and will leave it to the final review. But I believe you need to find a better lock and not just reuse a random existing one.
| bool update_slave_skip_errors(sys_var *self, THD *thd, enum_var_type type) | ||
| { | ||
| bool res= false; | ||
| mysql_mutex_unlock(&LOCK_global_system_variables); |
There was a problem hiding this comment.
Why have you decided to unlock this lock?
| res= true; | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
here you can do what you removed from slave.cc: assign the printable names to the variable so it's more readable.
| START SLAVE; | ||
| --echo # should reduce error because slave is not stopped | ||
| --error ER_SLAVE_MUST_STOP | ||
| SET GLOBAL slave_skip_errors = "1040"; |
There was a problem hiding this comment.
please also add actual functionality tests for the newly set variable values.
3 participants
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