Fix AWS RDS Blue/Green discovery for Aurora-only hostgroup configurations - #6158
Fix AWS RDS Blue/Green discovery for Aurora-only hostgroup configurations#6158Hyunho-Jung wants to merge 1 commit into
Conversation
…ions ProxySQL currently builds Hostgroup_Server_Mapping only from mysql_replication_hostgroups and mysql_servers. This leaves a gap for AWS Aurora configurations that use mysql_aws_aurora_hostgroups instead of mysql_replication_hostgroups. In that case, Aurora writer/reader servers are not present in hostgroup_server_mapping, even though they are valid backend servers managed by ProxySQL. AWS RDS Blue/Green Deployment handling relies on hostgroup_server_mapping when creating and applying Blue/Green hostgroup entries, so Aurora-only configurations can fail to provide the mapping needed to perform writer transitions correctly. This commit: - Extends the Hostgroup_Server_Mapping rebuild query to also include active mysql_aws_aurora_hostgroups entries (both writer_hostgroup and reader_hostgroup members), alongside the existing mysql_replication_hostgroups-based mapping. - Adds checksum tracking for mysql_aws_aurora_hostgroups so Hostgroup_Server_Mapping is rebuilt whenever the Aurora hostgroup configuration changes. - Adds a guard to update_aws_aurora_set_writer() and update_aws_aurora_set_reader() so Aurora writer/reader auto-movement is skipped while an AWS RDS Blue/Green switchover is already in progress for the same server. This prevents the Aurora monitor from reverting hostgroup state (for example, undoing a shun issued by the B/G switchover logic) while a switchover is in progress. Together these changes let Aurora-only configurations (mysql_aws_aurora_hostgroups without mysql_replication_hostgroups) participate correctly in AWS RDS Blue/Green Deployment handling, and prevent the Aurora monitor and the B/G switchover logic from racing over the same server's hostgroup membership during a transition.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (4)
Included review availability: Your plan provides up to 8 included reviews per hour; 6 remain after this review. 📜 Recent review details⏰ Context from checks skipped due to timeout. (1)
🧰 Additional context used📓 Path-based instructions (2)Header include guards use the `#ifndef __CLASS_*_H` convention.📄 CodeRabbit inference engine (CLAUDE.md) Files:
Class names must use `PascalCase` with protocol prefixes such as `MySQL_`, `PgSQL_`, and `ProxySQL_`.📄 CodeRabbit inference engine (CLAUDE.md) Files:
🪛 Cppcheck (2.21.0)lib/MySQL_HostGroups_Manager.cpp[warning] 6983-6983: If memory allocation fails, then there is a possible null pointer dereference (nullPointerOutOfMemory) [warning] 7218-7218: If memory allocation fails, then there is a possible null pointer dereference (nullPointerOutOfMemory) 🔇 Additional comments (4)
📝 WalkthroughWalkthroughThe change adds Aurora hostgroup checksum tracking, includes active Aurora writer and reader members in ChangesAurora hostgroup mapping
Aurora BGD update guards
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to The PR expands Aurora hostgroup discovery and prevents monitor movement during an in-progress Blue/Green switchover; no actionable merge-blocking risk remains after normal checks and review. Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant MySQL_HostGroups_Manager
participant AuroraHostgroups
participant MySQLServers
participant hostgroup_server_mapping
MySQL_HostGroups_Manager->>AuroraHostgroups: Check checksum and select active writer/reader members
AuroraHostgroups->>MySQLServers: Join member server records
MySQL_HostGroups_Manager->>hostgroup_server_mapping: Rebuild mapping
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
3 issues found across 4 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="include/Base_HostGroups_Manager.h">
<violation number="1" location="include/Base_HostGroups_Manager.h:733">
P3: The new member `hgsm_mysql_aws_aurora_hostgroups_checksum` is added inside the `#if 0` block (lines 605–965) of `Base_HostGroups_Manager.h`, which contains a legacy, never-compiled copy of the `MySQL_HostGroups_Manager` class. This addition has no effect; the live class is `MySQL_HostGroups_Manager` in `include/MySQL_HostGroups_Manager.h`, where the same member was added. Keeping the dead copy in sync is misleading and the change here is dead code — remove it (or drop the whole `#if 0` block).</violation>
</file>
<file name="lib/MySQL_HostGroups_Manager.cpp">
<violation number="1" location="lib/MySQL_HostGroups_Manager.cpp:6984">
P2: A BGD switchover can begin after this check and before the Aurora movement commits, so the guard does not eliminate the race it is intended to prevent. Coordinate the flag with the movement, or recheck under a lock held through the update.</violation>
<violation number="2" location="lib/MySQL_HostGroups_Manager.cpp:6990">
P3: When BGD is already in progress, this return skips the writer transition but the caller still records an Aurora failover. Return whether movement occurred and suppress the failover log when it did not.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| } | ||
|
|
||
| const std::string aurora_hostname { std::string(_server_id) + std::string(domain_name) }; | ||
| if (GloMyMon != nullptr && GloMyMon->is_aws_rds_bgd_server_in_progress(aurora_hostname, aurora_port)) { |
There was a problem hiding this comment.
P2: A BGD switchover can begin after this check and before the Aurora movement commits, so the guard does not eliminate the race it is intended to prevent. Coordinate the flag with the movement, or recheck under a lock held through the update.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/MySQL_HostGroups_Manager.cpp, line 6984:
<comment>A BGD switchover can begin after this check and before the Aurora movement commits, so the guard does not eliminate the race it is intended to prevent. Coordinate the flag with the movement, or recheck under a lock held through the update.</comment>
<file context>
@@ -6966,6 +6980,16 @@ void MySQL_HostGroups_Manager::update_aws_aurora_set_writer(int _whid, int _rhid
}
+ const std::string aurora_hostname { std::string(_server_id) + std::string(domain_name) };
+ if (GloMyMon != nullptr && GloMyMon->is_aws_rds_bgd_server_in_progress(aurora_hostname, aurora_port)) {
+ proxy_info(
+ "AWS Aurora: skipping writer update for %s:%d because AWS RDS BGD switchover is in progress\n",
</file context>
| * @details Used during 'commit' to determine if config has changed for 'MYSQL_AWS_AURORA_HOSTGROUPS', | ||
| * and 'hostgroup_server_mapping' should be rebuild. | ||
| */ | ||
| uint64_t hgsm_mysql_aws_aurora_hostgroups_checksum = 0; |
There was a problem hiding this comment.
P3: The new member hgsm_mysql_aws_aurora_hostgroups_checksum is added inside the #if 0 block (lines 605–965) of Base_HostGroups_Manager.h, which contains a legacy, never-compiled copy of the MySQL_HostGroups_Manager class. This addition has no effect; the live class is MySQL_HostGroups_Manager in include/MySQL_HostGroups_Manager.h, where the same member was added. Keeping the dead copy in sync is misleading and the change here is dead code — remove it (or drop the whole #if 0 block).
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At include/Base_HostGroups_Manager.h, line 733:
<comment>The new member `hgsm_mysql_aws_aurora_hostgroups_checksum` is added inside the `#if 0` block (lines 605–965) of `Base_HostGroups_Manager.h`, which contains a legacy, never-compiled copy of the `MySQL_HostGroups_Manager` class. This addition has no effect; the live class is `MySQL_HostGroups_Manager` in `include/MySQL_HostGroups_Manager.h`, where the same member was added. Keeping the dead copy in sync is misleading and the change here is dead code — remove it (or drop the whole `#if 0` block).</comment>
<file context>
@@ -725,6 +725,12 @@ class MySQL_HostGroups_Manager {
+ * @details Used during 'commit' to determine if config has changed for 'MYSQL_AWS_AURORA_HOSTGROUPS',
+ * and 'hostgroup_server_mapping' should be rebuild.
+ */
+ uint64_t hgsm_mysql_aws_aurora_hostgroups_checksum = 0;
</file context>
| aurora_hostname.c_str(), aurora_port | ||
| ); | ||
| free(domain_name); | ||
| return; |
There was a problem hiding this comment.
P3: When BGD is already in progress, this return skips the writer transition but the caller still records an Aurora failover. Return whether movement occurred and suppress the failover log when it did not.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/MySQL_HostGroups_Manager.cpp, line 6990:
<comment>When BGD is already in progress, this return skips the writer transition but the caller still records an Aurora failover. Return whether movement occurred and suppress the failover log when it did not.</comment>
<file context>
@@ -6966,6 +6980,16 @@ void MySQL_HostGroups_Manager::update_aws_aurora_set_writer(int _whid, int _rhid
+ aurora_hostname.c_str(), aurora_port
+ );
+ free(domain_name);
+ return;
+ }
+
</file context>
|
1 participant
Summary
ProxySQL currently builds
hostgroup_server_mappingonly frommysql_replication_hostgroupsandmysql_servers. This leaves a gap for AWS Aurora configurations that usemysql_aws_aurora_hostgroupsinstead ofmysql_replication_hostgroups: Aurora writer/reader servers are not present inhostgroup_server_mapping, even though they are valid backend servers managed by ProxySQL.AWS RDS Blue/Green Deployment handling relies on
hostgroup_server_mappingwhen creating and applying Blue/Green hostgroup entries, so Aurora-only configurations can fail to provide the mapping needed to perform writer transitions correctly (AWS RDS BGD: server :0 not found in hostgroup_server_mapping).Separately,
update_aws_aurora_set_writer()/update_aws_aurora_set_reader()have no guard against an in-progress AWS RDS Blue/Green switchover. This lets the Aurora monitor race with the B/G switchover state machine over the same server's hostgroup membership during a transition (e.g. reverting a shun the B/G logic just applied).What changed
hostgroup_server_mappingrebuild query (update_hostgroup_manager_mappings()) to also include activemysql_aws_aurora_hostgroupsentries (bothwriter_hostgroupandreader_hostgroupmembers), alongside the existingmysql_replication_hostgroups-based mapping.hgsm_mysql_aws_aurora_hostgroups_checksum) formysql_aws_aurora_hostgroupssohostgroup_server_mappingis rebuilt whenever the Aurora hostgroup configuration changes.update_aws_aurora_set_writer()andupdate_aws_aurora_set_reader()so Aurora writer/reader auto-movement is skipped while an AWS RDS Blue/Green switchover is already in progress for the same server (MySQL_Monitor::is_aws_rds_bgd_server_in_progress()).Files changed
include/Base_HostGroups_Manager.hinclude/MySQL_HostGroups_Manager.hlib/Base_HostGroups_Manager.cpplib/MySQL_HostGroups_Manager.cppTesting
This logic was originally developed and validated as a local patch on top of the v3.0.11 tag, and exercised against real AWS Aurora MySQL Blue/Green Deployment switchovers (multiple switchover attempts, confirming the previously-observed hostgroup duplication/oscillation issue no longer reproduced with these changes applied).
This PR rebases that same change onto current
v3.0HEAD. The rebase is a straightforward reapplication (no drift found in the touched functions between the v3.0.11 tag and currentv3.0), but it has not yet been independently recompiled/re-tested in this exact form — happy to follow up with a fresh build/switchover test if useful before merge.Notes
This does not change any admin table schema. It does not remove or bypass the existing
hostgroup_server_mappingvalidation in the B/G writer-configuration path — it only expands the mapping source so Aurora-hostgroup-only servers can be found through the same validation path.(Supersedes #6157, closed due to a commit-author identity mismatch.)
Summary by cubic
Fixes hostgroup discovery for Aurora-only configurations so AWS RDS Blue/Green deployments can locate writer/reader servers in
hostgroup_server_mapping. Previously the mapping was built only frommysql_replication_hostgroupsandmysql_servers, so Aurora setups usingmysql_aws_aurora_hostgroupsfailed with "server :0 not found in hostgroup_server_mapping". Also prevents the Aurora monitor from racing with an in-progress Blue/Green switchover over the same server's hostgroup membership.mysql_aws_aurora_hostgroupswriter and reader entries in thehostgroup_server_mappingrebuild query.mysql_aws_aurora_hostgroupsso the mapping rebuilds when Aurora config changes.No admin schema changes; the existing
hostgroup_server_mappingvalidation in the Blue/Green writer path is unchanged.Written for commit 0e4fe3e. Summary will update on new commits.
Summary by CodeRabbit
New Features
Bug Fixes