15 Commits (dbb40a30baec7d23568ebe2d2b228737d31dfcba)

Author	SHA1	Message	Date
Rene Cannao	fe21aafbe8	fix(test/tap): eliminate compiler warnings in TAP tests ... - Replace diag("") with diag("%s", "") to avoid zero-length format warnings - Increase buffer and use unsigned format in unique_ip() to fix sprintf overflow - Use printf-style format strings instead of .c_str() for ok() calls - Fix %zu for size_t loop variable in fast_forward_switch_replication_deprecate_eof - Use explicit function pointer type instead of decltype(&pclose) - Replace string_format with plain assignment when no format args needed	1 month ago
Rene Cannao	3647759ea9	fix: correct SHUNNED status handling in pgsql cluster sync test ... SHUNNED is a transient runtime state, never persisted in config. Both runtime and main tables store SHUNNED servers as ONLINE after sync. Fix the test to expect ONLINE instead of SHUNNED in the replica's pgsql_servers main table. Also remove stray string_format() call that caused a compile error.	2 months ago
Rene Cannao	b60635f1c5	fix: address critical bugs and add missing test coverage for PGSQL cluster sync ... Critical bug fixes: - Add NULL check after execute_statement in flush_GENERIC_variables__checksum__database_to_runtime to prevent segfault when SQLite returns NULL resultset - Add NULL checks after mysql_store_result in all pgsql pull functions to prevent dereference of nullptr from OOM or protocol errors - Add proxy_error logging for double-NULL fallthrough in Admin_Handler pgsql query rules when both get_current_query_rules_inner() and get_current_query_rules() return NULL - Check execute_statement error output for pgsql_servers_v2 fallback query to detect schema corruption or disk failures instead of silently using empty results - Add NULL guards for MYSQL_ROW fields in update_pgsql_users to prevent atoll(NULL) undefined behavior on corrupted peer responses Test coverage additions: - Add check_pgsql_variables_sync: end-to-end test for pgsql variable propagation - Add check_diffs_before_sync_disabled: verify diffs_before_sync=0 prevents sync - Add SHUNNED and OFFLINE_SOFT server test cases to verify status handling - SHUNNED servers verified as mapped to ONLINE in runtime by cluster sync	2 months ago
René Cannaò	93eabfc7bc	fix: address code review issues in PostgreSQL cluster sync ... - Fix fetch_and_store incrementing success counter on failure - Fix sqlite3_stmt leak in pull_pgsql_variables_from_peer (use RAII) - Convert get_peer_to_sync_pgsql_query_rules and _servers_v2 to unified framework delegates (fixes atomic data race on .load()) - Merge dual scan loops in set_checksums to respect enabled_check - Remove spurious "deprecated" label from new checksum_pgsql_variables - Replace sprintf with snprintf in pgsql diffs_before_sync getters - Fix indentation on cluster_pgsql_variables blocks - Fix TAP plan mismatch: replace MYSQL_QUERY with inline mysql_query - Fix trivially-true table accessibility assertions - Add mysql_close on failed connect in test - Move test_cluster_sync_pgsql-t to legacy-g5 group	2 months ago
Rene Cannao	7e94ff904e	test: remove unnecessary fresh_connection workaround from pgsql cluster sync test ... The test was opening a new admin connection for every replica query, working around a supposed visibility issue where persistent sessions could not see synced pgsql data. Live testing confirms both persistent and fresh connections see identical data on the replica for all three modules (servers, users, query rules). The workaround is unnecessary.	2 months ago
Rene Cannao	859c89b30e	cluster: harden PostgreSQL cluster sync parity paths after real two-node validation ... This commit turns the current working branch follow-up into one coherent changeset after reviewing the PostgreSQL cluster sync implementation against real multi-node behavior instead of relying only on checksum presence and basic TAP accessibility checks. The main implementation change is that PostgreSQL cluster sync now updates the replica admin-memory tables in the places where the previous branch state was still weaker than the established MySQL behavior. In the PostgreSQL servers path, the replica now rewrites pgsql_servers, pgsql_replication_hostgroups, and pgsql_hostgroup_attributes before promoting the fetched state into runtime. That closes the most important parity gap that showed up during replica-side validation, because the cluster sync operation no longer leaves runtime state correct while replica admin-memory tables lag behind. In the PostgreSQL query rules path, the fetched query-rules and fast-routing resultsets are now hashed using the same converted representation that runtime loading consumes, then passed directly into load_pgsql_query_rules_to_runtime(), and finally written back from runtime into the replica admin-memory tables. In the PostgreSQL users path, the branch now relies on PostgreSQL-specific runtime checksum logic and persists accepted rows into the replica pgsql_users admin-memory table. This commit also fixes a real correctness bug that was discovered only after running a true two-node manual validation flow. The refactored module table in ProxySQL_Cluster::set_checksums() passed the full module names "pgsql_variables" and "ldap_variables" into pull_global_variables_from_peer(), but that shared helper only accepts the short dispatch keys "pgsql" and "ldap". In practice, changing a PostgreSQL variable on the primary caused the replica to abort on an assertion as soon as the cluster monitor attempted the variable pull. This commit restores the correct short dispatch keys so the PostgreSQL and LDAP variable sync paths actually execute instead of crashing. On the TAP side, the PostgreSQL cluster sync test is strengthened so it checks what the implementation is supposed to guarantee rather than what the earlier test happened to probe. The server sync test now mutates the real pgsql_servers admin table, then validates replica runtime_pgsql_servers, replica pgsql_servers, and disk.pgsql_servers when save_to_disk is enabled. The users sync test now validates runtime, main, and optional disk state on the replica. The query-rules sync test now validates both query rules and fast-routing rows across runtime, main, and optional disk persistence. The polling helpers were also hardened to support fresh replica admin sessions, because during local validation a long-lived admin session did not reliably observe synced rows even though fresh sessions and the manual harness did. The test now asserts the externally observable replica state seen by new admin clients. The documentation is updated to reflect the branch as it actually exists after this review pass. The PostgreSQL cluster sync status document now records the remaining non-CI TODO, the module-by-module behavior, the real two-node test plan, the outcomes for save_to_disk=false and save_to_disk=true, the variable sync dispatcher crash that was found and fixed locally, and the current MySQL parity assessment. This makes the branch easier to continue without having to reconstruct the reasoning from terminal logs or partial memory. Validation performed for this commit: - git diff --check - make build_src -j2 - PostgreSQL cluster-sync TAP two-node validation with save_to_disk=false - PostgreSQL cluster-sync TAP two-node validation with save_to_disk=true - manual two-node validation of pgsql_servers, pgsql_replication_hostgroups, pgsql_hostgroup_attributes, and a real PostgreSQL variable with save_to_disk=false - manual two-node validation of the same paths with save_to_disk=true Residual caveat intentionally documented but not treated as a blocker in this commit: - a long-lived replica admin session did not reliably observe synced PostgreSQL rows during polling, while fresh admin sessions and the manual validation harness did observe the expected state - the underlying cluster-sync apply path now validates correctly in real two-node runs, but the admin-session visibility quirk should still be understood better before calling the branch completely finished	2 months ago
Rene Cannao	bf9fc81f48	test: strengthen pgsql cluster sync TAP follow-up	2 months ago
Rene Cannao	5c7e616f9b	PR5297: resolve remaining actionable review findings for PGSQL checksum sync and TAP assertions ... This commit addresses the review findings that are still reproducible on top of fix/postgresql-cluster-sync_2 after the v3.0 merge, while leaving already-resolved or no-longer-applicable reports untouched. Changes included: - Admin_FlushVariables: restore active PGSQL checksum generation path in flush_pgsql_variables___database_to_runtime(). - Remove the stale commented-out checksum block. - After releasing GloPTH write lock, compute runtime PGSQL vars snapshot and call flush_GENERIC_variables__checksum__database_to_runtime("pgsql", checksum, epoch) under checksum mutex, matching the mysql/admin pattern. - ProxySQL_Admin::set_variable(checksum_pgsql_variables): when disabling, reset all PGSQL *_diffs_before_sync thresholds (variables/query_rules/servers/users) in both admin variables and GloProxyCluster state, and update warning text accordingly. - TAP test hardening in test_cluster_sync_pgsql-t: - Ensure each checksum-loop failure path emits an explicit failing ok(false, ...) before continue, preventing TAP plan/accounting mismatches. - Simplify pgsql_servers_v2 test tuple shape by removing the unused int field and using the comment field consistently. Validation performed: - make -C lib -j4 (pass) - make -C test/tap/tests test_cluster_sync_pgsql-t -j4 (pass) Note: - The previously reported double wrunlock() in flush_pgsql_variables__from_memory_to_disk() is not present in current HEAD; function has a single wrlock/wrunlock pair.	3 months ago
Rene Cannao	4cdf61cbae	cluster: complete PostgreSQL servers sync flow and fix TAP libtap stale-archive linker failures ... This commit finalizes the remaining PostgreSQL cluster-sync implementation work and includes a targeted TAP build fix that addresses the unresolved perform_simple_get() linker error seen while building debug TAP tests. PostgreSQL cluster sync changes: - Add conversion support to transform fetched pgsql resultsets into SQLite3_result objects suitable for admin/runtime loaders. - Rework runtime pgsql server pull path to: - use peer-selected checksums, - fetch runtime rows from the selected peer, - commit runtime state through PgHGM in only-runtime mode, - persist runtime state and keep locking semantics around pgsql server updates. - Rework pgsql_servers_v2 pull path to fetch and validate all related datasets together: - pgsql_servers_v2, - pgsql_replication_hostgroups, - pgsql_hostgroup_attributes, - runtime_pgsql_servers (when applicable). - Ensure checksum validation includes all fetched tables and rejects partial/inconsistent peer payloads. - Load the actual incoming converted datasets into runtime instead of placeholder/empty local resultsets. Test updates: - Extend test_cluster_sync_pgsql-t with additional verification for synchronized pgsql data and optional replica/runtime sync checks. - Adjust TAP planning and resultset handling so all executed queries are validated and cleaned up consistently. TAP build/linker fix: - Update test/tap/tap/Makefile archive targets (libtap_mariadb.a/libtap_mysql57.a/libtap_mysql8.a) to remove existing archives before recreating them. - Add Makefile as a prerequisite for those archive targets so this behavior is applied on rebuild after pulling. - This prevents stale archive members (for example old noise_utils*.o objects) from being retained and reintroduced into libtap.so, which was causing undefined references to an obsolete 4-argument perform_simple_get() symbol. Validation performed in this branch after these changes: - make -C lib -j4 - make -C src -j4 - make -C test/tap/tests test_cluster_sync_pgsql-t -j4 - make -C test/tap/tests admin-listen_on_unix-t -j4	3 months ago
Rene Cannao	f9e3e85e68	fix: address all AI review concerns for PostgreSQL cluster sync ... - Fix critical Admin_FlushVariables bug where PostgreSQL SELECT appended to MySQL checksum query - Add security check for integer overflow in pgsql_variable_validate_integer - Fix test suite tuple indexing and NULL safety issues - Add missing admin variables for PostgreSQL replication hostgroups and attributes - Implement checksum_pgsql_variables get/set variable behavior - Fix misleading log messages for PostgreSQL disable operations - Fix cluster_pgsql_variables_diffs_before_sync gating logic - Add missing sync_delayed metrics for pgsql modules - Fix CLUSTER_QUERY_RUNTIME_PGSQL_SERVERS macro numeric vs VARCHAR comparison Resolves issues identified by coderabbitai and gemini-code-assist reviews in PR 5297.	4 months ago
Rene Cannao	cf8692ace0	fix: address AI review feedback for PostgreSQL cluster sync ... - Fix CLUSTER_QUERY_RUNTIME_PGSQL_SERVERS macro to use proper VARCHAR string literals - Fix inconsistent gating/reset behavior for pgsql diffs when mysql checksum flags disabled - Fix copy/paste log text errors in ProxySQL_Admin.cpp - Improve test suite robustness with better NULL checks - Verify all PostgreSQL variables are properly included in admin_variables_names array Addresses feedback from coderabbitai and gemini-code-assist reviews in PR 5234.	4 months ago
Rene Cannao	c5916100f1	fix: complete all AI review fixes for PostgreSQL cluster synchronization ... This commit addresses the remaining issues identified by AI reviews in PR 5234: - Fixed CLUSTER_QUERY_RUNTIME_PGSQL_SERVERS macro to resolve numeric vs VARCHAR comparison issue - Verified that PostgreSQL variables are already included in admin_variables_names array - Fixed inconsistent gating/reset behavior for pgsql diffs when mysql checksum flags are disabled (added pgsql diffs reset for checksum_mysql_query_rules, checksum_mysql_servers, checksum_mysql_users) - Initialized atomic counters in ProxySQL_Cluster constructor to value 3 - Added missing get_peer_to_sync_pgsql_variables wrapper function (implementation + header declaration) - Fixed test issues in test_cluster_sync_pgsql-t.cpp (NULL checks, tuple indexing, test plan) - Updated PostgreSQL modules documentation to include pgsql_variables - Standardized SQL string quoting to single quotes consistently All compilation issues have been resolved and the codebase now has consistent PostgreSQL cluster synchronization behavior across all modules. Signed-off-by: Rene <rene@localhost>	4 months ago
Rene Cannao	1beb5b9e5b	fix: address review comments for PostgreSQL cluster sync PR ... - Fix SQL string literals in CLUSTER_QUERY_PGSQL_SERVERS_V2 to use single quotes - Initialize all std::atomic<int> counters in ProxySQL_Cluster constructor to 3 - Add missing cluster_pgsql_variables_diffs_before_sync and cluster_pgsql_variables_save_to_disk admin variables - Add get_peer_to_sync_pgsql_variables wrapper function for consistency - Fix copy/paste log text error in set_variable function for mysql_query_rules - Make PgSQL diffs consistent with MySQL diffs when checksum flags are disabled - Remove TAP test calls from helper function, move assertions to main - Fix tuple size mismatch in string_format calls (remove extraneous 12th element) - Update test plan from 6 to 10 to match actual test count Resolves all issues identified by gemini-code-assist and coderabbitai review comments.	4 months ago
Rene Cannao	187b71ee6d	fix: Correct TAP test syntax according to TAP_TESTS_GUIDE.md ... - Replace invalid Chinese '计划(plan, 6)' with proper 'plan(6)' - Replace invalid 'pass()' and 'fail()' with proper 'ok(condition, description)' - Follow TAP test patterns from documentation - Use proper mysql_errno() checking for error handling Fixes TAP test compilation errors and follows proper TAP protocol. Related: #5147	6 months ago
Rene Cannao	d96be35b59	fix: Add pgsql_servers_v2 to runtime_checksums_values table and create basic TAP test ... - Add missing pgsql_servers_v2 checksum to runtime_checksums_values population - Create basic TAP test template for PostgreSQL cluster sync validation - Test verifies PostgreSQL checksums appear in runtime_checksums_values - Test validates access to PostgreSQL tables and cluster variables This completes the PostgreSQL cluster sync implementation by ensuring all PostgreSQL modules have proper checksum tracking and basic test coverage. Related: #5147	6 months ago