Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactoring cluster module mysql_servers #4169

Closed
wants to merge 33 commits into from

Conversation

rahim-kanji
Copy link
Collaborator

@rahim-kanji rahim-kanji commented Apr 6, 2023

The previous implementation of the cluster module fetched data from the "mysql_servers" table (runtime_mysql_servers). It also fetched other dependent modules such as "mysql_replication_hostgroups," "mysql_group_replication_hostgroups," "mysql_galera_hostgroups," "mysql_aws_aurora_hostgroups," and "mysql_hostgroup_attributes." The accumulated checksum was computed by combining the checksum of the "mysql_servers" table with the checksums of the other dependent modules. This accumulated checksum was then compared with the peer checksum. If they matched, the configuration was loaded and saved to disk when the "cluster_mysql_servers_save_to_disk" option was set to true.

In the new implementation, several changes were introduced. A new admin variable called 'admin-cluster_mysql_servers_sync_algorithm' and a new table "mysql_servers_v2" (admin mysql_servers) were added. The fetching behavior was modified based on the value of 'admin-cluster_mysql_servers_sync_algorithm', which could be set to 1, 2, or 3. The main distinction between the previous and new implementations was the choice of tables to fetch and the calculation of checksums. In the previous version, the checksum for the "mysql_servers" table was computed and combined with the checksums of other dependent modules. However, in the new version, the checksum for "mysql_servers_v2" was calculated and combined with the checksums of the dependent modules, while the "mysql_servers" table now has an independent checksum.

Here's how it works:
When 'admin-cluster_mysql_servers_sync_algorithm' is set to 1, data is fetched from "mysql_servers_v2" and the dependent modules. The accumulated checksum is computed and compared with the peer checksum. If they match, the configuration is loaded and saved to disk when the "cluster_mysql_servers_save_to_disk" option is true. Additionally, the "mysql_servers" table is fetched, and its checksum is computed and compared with the peer checksum. If they match, the configuration is saved to disk.

If 'admin-cluster_mysql_servers_sync_algorithm' is set to 2, data is only fetched from the "mysql_servers_v2" table. The checksum is calculated and compared with the peer checksum, and if they match, the configuration is saved to disk.

In the case of 'admin-cluster_mysql_servers_sync_algorithm' set to 3, the decision of which table to fetch, "mysql_servers_v2" and/or "mysql_servers," depends on whether ProxySQL was executed with or without the '-M' argument. The presence of '-M' enforces the disablement of the monitor module. If '-M' is not present, 'admin-cluster_mysql_servers_sync_algorithm' 1 is selected, syncing both the "mysql_servers_v2" and "mysql_servers" tables. If '-M' is provided, 'admin-cluster_mysql_servers_sync_algorithm' 2 is chosen, syncing only the "mysql_servers_v2" table.

Note:

  • It's important to note that whenever the 'admin-cluster_mysql_servers_sync_algorithm' is changed, ProxySQL requires a restart (all nodes) or the addition/removal of a fake record in the "mysql_server" table on primary node, followed by loading it into the runtime 'LOAD MYSQL SERVERS TO RUNTIME'.
  • Any modifications made by the monitor module will not increase module version.
  • Global checksum is reset when the value of 'admin-cluster_mysql_servers_sync_algorithm' is modified.

* Added separate fetching of mysql_server_incoming and runtime_mysql_server records based on algorithm selection.
* Few memory leaks fix
* Code refactoring
@rahim-kanji rahim-kanji marked this pull request as draft April 6, 2023 07:01
* Changed commit signature.
* Few fixes
* Few issues fixed
* Fetching mysql_servers_v2 records from admin
* Added comments.
* Added detailed comments.
@rahim-kanji rahim-kanji force-pushed the v2.x_refactor_cluster_mysql_servers branch from 3f813d8 to 1af0586 Compare April 10, 2023 20:07
…ed ahead of the 'mysql_servers' code block. This ensures that the most recent value of 'admin-cluster_mysql_servers_sync_algorithm' is available in mysql_servers sync.

* Reset global checksum if 'admin-cluster_mysql_servers_sync_algorithm' is changed.
…sql_servers' and 'mysql_servers_v2' across all nodes. The test involves creating two replica nodes, one with a monitor enabled and the other with a monitor disabled. The test verifies that the correct records are synced among these nodes based on the value of 'admin-cluster_mysql_servers_sync_algorithm'.
@mirostauder
Copy link
Collaborator

retest this please

* Cleared duplicate code.
* Added comments
…se."

* In the event of any failure, it's crucial to call the cleanup function to prevent any orphaned replica instances from remaining active.
@rahim-kanji rahim-kanji force-pushed the v2.x_refactor_cluster_mysql_servers branch from f23abd1 to f350038 Compare April 20, 2023 10:48
…_mysql_servers

# Conflicts:
#	include/ProxySQL_Cluster.hpp
#	include/proxysql_admin.h
#	lib/MySQL_HostGroups_Manager.cpp
#	lib/ProxySQL_Admin.cpp
#	lib/ProxySQL_Cluster.cpp
@rahim-kanji
Copy link
Collaborator Author

retest this please

@mirostauder
Copy link
Collaborator

retest this please

…_mysql_servers

# Conflicts:
#	lib/MySQL_HostGroups_Manager.cpp
…ers' into v2.x_refactor_cluster_mysql_servers
…_mysql_servers_final

# Conflicts:
#	lib/MySQL_HostGroups_Manager.cpp
#	lib/ProxySQL_Cluster.cpp
@renecannao renecannao marked this pull request as ready for review June 13, 2023 08:38
@rahim-kanji rahim-kanji changed the title WIP: Refactoring cluster module mysql_servers Refactoring cluster module mysql_servers Jun 14, 2023
@mirostauder
Copy link
Collaborator

retest this please

TODO:
Update documentation: mysql_servers_v2
@rahim-kanji
Copy link
Collaborator Author

retest this please

If these resultsets are not updated, checksums will always mismatch
between the peer cluster nodes in case servers are deleted. The primary
will hold the correct checksum ('0x0'), but will report the old resultset
to the peer node, the second will compute a checksum different than the
expected '0x0'. Cycle won't stop until config is updated in the target
primary.
- Resultsets for 'runtime_mysql_servers' and 'mysql_servers_v2' are now
  directly generated by the SQL query.
- Checksum computations are simplified to 'SQLite3_result::raw_checksum'
  and equivalent function 'mysql_raw_checksum'.
@JavierJF
Copy link
Collaborator

Retest this please.

Config should only be used for checksum computation and cluster
resultset generation for 'LOAD MYSQL SERVERS TO RUNTIME'. Otherwise
user config ('main.mysql_servers') would be promoted at each 'commit'
call, and propagated through the cluster members.
@renecannao
Copy link
Contributor

retest this please

If 'MySQL_HostGroups_Manager::incoming_mysql_servers' happens to be
empty when a 'CLUSTER_QUERY_MYSQL_SERVERS_V2' request is received in
Admin, since there wasn't any user config promoted to runtime yet, the
response should be an empty resulset.
@renecannao
Copy link
Contributor

Closing become #4295 includes it , and it was merged

@renecannao renecannao closed this Jan 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants