MDEV-32947 Async conflicts with semi-sync in multi-source - #5615
Open
ParadoxV5 wants to merge 3 commits into
Open
MDEV-32947 Async conflicts with semi-sync in multi-source#5615ParadoxV5 wants to merge 3 commits into
ParadoxV5 wants to merge 3 commits into
Conversation
ParadoxV5
commented
Aug 29, 2026
ParadoxV5
left a comment
ParadoxV5
left a comment
Contributor
Author
There was a problem hiding this comment.
Repl_semi_sync_slave is merely a singleton that does not give enough value as a namespace.
Maybe a main-branch refactor is worth to dissolve it into Master_info…
(Relates to MDEV-40941)
Comment on lines
46
to
60
| { | ||
| local_semi_sync_enabled= repl_semisync_slave.get_slave_enabled(); | ||
| var->type= SHOW_BOOL; | ||
| var->value= (char*) &local_semi_sync_enabled; | ||
| if (Master_info *mi= | ||
| get_master_info(&thd->variables.default_master_connection, | ||
| Sql_condition::WARN_LEVEL_NOTE)) | ||
| { | ||
| *static_cast<bool *>(buff)= | ||
| repl_semisync_slave.get_slave_enabled(mi); | ||
| mi->release(); | ||
| var->type= SHOW_BOOL; | ||
| var->value= buff; | ||
| } | ||
| else | ||
| var->type= SHOW_UNDEF; | ||
| return 0; | ||
| } |
Contributor
Author
There was a problem hiding this comment.
matches Slave_running (show_slave_running()) 🤔 ¯\_(ツ)_/¯
ParadoxV5
marked this pull request as draft
August 30, 2026 07:37
`@@rpl_semi_sync_slave_enabled` can be changed without stopping replication, because it takes effect when a connection is established. But when it takes effect, it applies to *all* replication connections. This means that starting an async connection will switch any ongoing semi-sync connections to async as well, and vice versa, which will cause those connections to fail to replicate in the wrong mode. This fix resolves this oversight by changing the value application to specific to only the establishing connection (technically, moving the singleton’s field `Repl_semi_sync_slave::m_slave_enabled` to the instance variable `Master_info::semi_sync_enabled`). Note, `Master_info::semi_sync_reply_enabled` must remain separate, because it only disables ACK replying in async slave fallback, whereas the semi-sync slave setting also controls whether the IO thread should expect additional semi-sync flags in event packets. Consequently, rather than leaving the status variable `Rpl_semi_sync_slave_status` ambiguous, this commit refines it to show `@@default_master_connection`’s status, matching `Slave_running` & co.. These designs build toward MDEV-40941, which proposes migrating `@@rpl_semi_sync_slave_enabled` to a proper per-connection configuration.
ParadoxV5
marked this pull request as ready for review
August 31, 2026 03:13
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
@@rpl_semi_sync_slave_enabledcan be changed without stopping replication, because it takes effect when a connection is established.But when it takes effect, it applies to all replication connections.
This means that starting an async connection will switch any ongoing semi-sync connections to async as well, and vice versa, which will cause those connections to fail to replicate in the wrong mode.
This fix resolves this oversight by changing the value application to specific to only the establishing connection (technically, moving the singleton’s field
Repl_semi_sync_slave::m_slave_enabledto the instance variableMaster_info::semi_sync_enabled).Note,
Master_info::semi_sync_reply_enabledmust remain separate, because it only disables ACK replying in async slave fallback (whether that’s the right move is a separate topic), whereas the semi-sync slave setting also controls whether the IO thread should expect additional semi-sync flags in event packets.(It’s also a possible design to ACK in async replication as slave heartbeating.)
Consequently, rather than leaving the status variable
Rpl_semi_sync_slave_statusambiguous, this commit refines it to show@@default_master_connection’s status, matchingSlave_running& co..These designs build toward MDEV-40941, which proposes migrating
@@rpl_semi_sync_slave_enabledto a proper per-connection configuration.