11 Commits (31d7ae9ecbfcce4062dde20dfd67ff60dd0db29c)

Author	SHA1	Message	Date
Rene Cannao	ab9d5a1036	fix(plugin-chassis): address deep-review findings ... Follow-up to cd48c5613 (PROXYSQL40 gating commit). Multiple reviewers reported issues ranging from descriptor-layout UB to test false-positives; this commit resolves all of them. Showstoppers fixed: * plugins/mysqlx/Makefile: PROXYSQL40 (and all feature-tier flags) are now propagated from the top-level Makefile and converted to -DPROXYSQL40 in CXXFLAGS. Before this, PROXYSQL40=1 built a v4 core that dlopen'd a 6-field v3 mysqlx descriptor, and the loader then read past the end of the plugin's static struct. Top-level Makefile's four plugins/mysqlx recursive-make invocations now pass PROXYSQL40=$(PROXYSQL40); the test/tap/tests/unit mysqlx_plugin_build rule does the same. * ABI version gating (S2/S3): add PROXYSQL_PLUGIN_ABI_VERSION macro to ProxySQL_Plugin.h (1 without PROXYSQL40, 2 with). Plugins set descriptor.abi_version from this macro. Loader rejects abi_version < 1 or > PROXYSQL_PLUGIN_ABI_VERSION_MAX. The register_schemas field is only read when abi_version >= 2 -- a v1 plugin's 6-field descriptor is no longer dereferenced past its own layout even when loaded by a v4 core. ScopedRegistryTarget RAII guard replaces the inline g_registry_target = this / = nullptr pairs so exceptions from plugin callbacks can't leave the globals dirty. * Admin_Handler TOCTOU: proxysql_resolve_configured_plugin_admin_alias now returns std::string by value instead of const char*. The previous signature borrowed a c_str() from inside commands_ under the plugin manager lock; callers then released the lock before dispatch, and a concurrent reload between the two calls could dangle the pointer. Returning by value copies the canonical SQL out while the lock is still held. * test_phase_b_db_handles_are_null was a false-positive trap: it logged "null" whenever the getter pointer was null OR returned null, so a regression to get_admindb = nullptr would have silently passed. The fake plugin now emits three distinct markers (phase_b_getter_null vs phase_b_handles_null vs phase_b_handles_live); the test asserts the exact contract (non-null function pointer that returns nullptr). * Snapshot stubs contract: the header claimed Phase D services are "every field LIVE", but get_mysql_users_snapshot et al. point to a nullptr-returning stub in every phase. Header now explicitly documents that snapshot getters are not yet implemented and plugins MUST treat null return as "not available" across every phase. * stop() teardown symmetry: stop_all() previously skipped plugins that succeeded init() but failed start(). Resources allocated in init would leak. Now stop_all() runs stop() for every plugin where initialized=true, irrespective of started. proxysql_plugin_stop_cb's header docstring documents this: stop pairs with init, not start. * register_schemas partial-failure rollback: if a plugin's register_schemas registered some tables/commands and then returned false, the registry kept the partial registrations. A retry (same plugin, bug fixed) would then duplicate-fail. Both invoke_register_schemas_phase and init_all now snapshot the registration counts before invoking the plugin callback and truncate back on failure. Tightened / documented: * alias normalization delta (PROXYSQL40 collapses whitespace and trims trailing ';', !PROXYSQL40 is length-strict) is documented in canonicalize_plugin_command's docstring as an intentional UX improvement for the chassis tier, not a regression. * init-after-publish ordering invariant documented at both the publish site (proxysql_load_configured_plugins Phase-B branch) and at proxysql_init_configured_plugins. Phase D writes to commands_ / mysql_query_hook_ / pgsql_query_hook_ on the already-published manager; safe today because ProxySQL_Main_init_phase3___start_all runs strictly after Phase D, so no thread ever observes the manager during its Phase-D mutations. * Phase-D failure path documented: treated as fatal startup error (exit(EXIT_FAILURE) in main.cpp). Plugins that succeeded init() are still torn down via stop_all() during process teardown, now that stop pairs with init. Runtime reload is out of scope for this iteration. * Admin_Bootstrap.cpp materialize_plugin_tables(): removed duplicate ATTACH DATABASE calls for "disk" and "stats". init_sqlite3db() in the same startup flow attaches them first; a duplicate ATTACH errors out in SQLite. * test/tap/tests/unit/Makefile PROXYSQL31 autodetection: previously used the same probe symbol (MySQLFFTO) as PSQLFFTO, which collapsed PROXYSQL31 onto PROXYSQLFFTO. Now PROXYSQL31 is set when either PROXYSQLFFTO or PROXYSQLTSDB is detected (matching the top-level cascade's PROXYSQL31 => FFTO && TSDB). Tests added (all under PROXYSQL40 only): * plugin_lifecycle_unit-t: - test_phase_b_partial_failure_rolls_back: plugin registers a table then returns false; rollback verified by retrying the same plugin with the failure toggle cleared -- succeeds. - test_stop_runs_when_start_fails: exercises the init-success / start-failure path and asserts stop() marker in log. - test_bogus_abi_version_rejected: loads a descriptor with abi_version=99; loader rejects with "unsupported plugin ABI version" and init is not called. - plan(26), up from plan(14). * plugin_dispatch_unit-t: - test_cross_plugin_command_collision: loads both fake_plugin and fake_plugin2 as real .so's, both registering the same admin-command SQL; init fails with a message naming the collision. Complements the same-manager API-level collision test already present. - plan(52), up from plan(50). * plugin_query_hook_unit-t: - test_embedded_nul_in_query_text: dispatch with an embedded NUL in query_text asserts the full length_len byte range reaches the hook intact (not strlen-truncated). - test_reentrant_dispatch_across_protocols: mysql-hook callback re-enters dispatch_query_hook(pgsql). Asserts no deadlock and correct inner-hook invocation count. - plan(46), up from plan(41). Total plugin unit tests: 48 (config) + 52 (dispatch) + 26 (lifecycle) + 41 (prometheus) + 46 (query_hook) = 213 under PROXYSQL40; 47 + 32 = 79 under v3.1 non-chassis.	1 month ago
Rene Cannao	cd48c5613e	feat(plugin-chassis): gate chassis ABI additions behind PROXYSQL40 ... The four-phase plugin lifecycle (Step 2.2), pre-execution query-hook ABI (Step 2.1), shared Prometheus registry access (Step 2.3), and generic admin-command alias dispatch (Step 2.5) introduce new plugin ABI surface area — a new descriptor field (register_schemas), new services-struct entries (register_query_hook, get_prometheus_registry, register_command_alias), new public manager methods (invoke_register_schemas_phase, dispatch_query_hook, etc.), and a new startup ordering in main(). These changes should not appear in the v3.x stable/innovative tiers. Wrap every chassis-only declaration, definition, and dispatch path in `#ifdef PROXYSQL40` so the v3.0/v3.1 builds retain the pre-chassis behavior: a single init()-only descriptor layout, two-phase load+init lifecycle, and the hardcoded MYSQLX alias ladder in Admin_Handler.cpp. Changes: * Top-level Makefile: introduce PROXYSQL40 as a distinct tier above PROXYSQL31. Cascade rule: PROXYSQLGENAI=1 implies PROXYSQL40=1 implies PROXYSQL31=1. Bump major version number on PROXYSQL40=1 (previously tied to PROXYSQLGENAI). Export PROXYSQL40 for recursive submakes. * lib/Makefile, src/Makefile: add -DPROXYSQL40 via $(PSQL40) so the compile commands match the top-level tier selection. * include/ProxySQL_Plugin.h: gate the register_schemas descriptor field, query-hook types/enums/typedefs, the alias-registration callback, and the trailing services-struct fields (register_query_hook, get_prometheus_registry, register_command_alias). The pre-chassis descriptor remains six fields (name, abi_version, init, start, stop, status_json) so plugins built against v3.x headers still link and load. * include/ProxySQL_PluginManager.h: gate the chassis-exclusive public methods and private members (schemas_registered, aliases, services_phase_b_, query hook pointers). Provide two declarations of proxysql_load_configured_plugins — the four-phase variant (paired with proxysql_init_configured_plugins) under PROXYSQL40, the legacy single-call variant otherwise. * lib/ProxySQL_PluginManager.cpp: gate every method that belongs to the chassis (invoke_register_schemas_phase, register_command_alias, resolve_alias_to_canonical, register_query_hook, has_query_hook, dispatch_query_hook), plus the services_phase_b_ initialization (nullptr DB handle stubs), the proxysql_dispatch_configured_plugin_ query_hook entry point, and the alternate four-phase body of proxysql_load_configured_plugins / proxysql_init_configured_plugins. The legacy two-phase loader is restored under !PROXYSQL40. get_admindb_service / get_configdb_service / get_statsdb_service stay ungated because they back the unconditional services_ member. * lib/Admin_Handler.cpp: gate the `#include \"ProxySQL_PluginManager.h\"` and restore under !PROXYSQL40 the 16 hardcoded MYSQLX alias vectors plus the 16-deep if-ladder in admin_handler_command_*(). Under PROXYSQL40 the ladder is replaced by a single call to proxysql_resolve_configured_plugin_admin_alias(), which is plugin-agnostic. * src/main.cpp: gate InitConfiguredPlugins and branch the startup sequence in ProxySQL_Main_init_phase2___not_started: - PROXYSQL40: LoadConfiguredPlugins (Phase A+B) -> admin init (C) -> materialize_plugin_tables -> InitConfiguredPlugins (D) -> StartConfiguredPlugins (E). - !PROXYSQL40: admin init -> LoadConfiguredPlugins (combined load+init) -> materialize_plugin_tables -> StartConfiguredPlugins, matching the pre-chassis ordering. * plugins/mysqlx/src/mysqlx_plugin.cpp: split init under PROXYSQL40 into mysqlx_register_schemas (schema + commands) and a minimal mysqlx_init (context only). Under !PROXYSQL40 the original single mysqlx_init remains. Descriptor conditionally wires register_schemas. * plugins/mysqlx/src/mysqlx_admin_schema.cpp: under PROXYSQL40 use the reg() helper to register canonical+aliases via the new ABI; under !PROXYSQL40 fall back to 16 direct register_command() calls (aliases handled by Admin_Handler's hardcoded ladder). * test/tap/test_helpers/fake_plugin.cpp: gate fake_query_hook, fake_register_schemas, fake_descriptor_with_phase_b, the REGISTER_QUERY_HOOK block in fake_init, and the ENABLE_PHASE_B descriptor selection. The pre-chassis descriptor-returning stub is the sole return path under !PROXYSQL40. * test/tap/tests/unit/Makefile: autodetect PROXYSQL40 via invoke_register_schemas_phase (the chassis-exclusive symbol) in the library archive; add -DPROXYSQL40 to fake_plugin .so compilation so the descriptor layout matches the loader's expectations; append the three chassis-only tests (plugin_lifecycle, plugin_prometheus, plugin_query_hook) to UNIT_TESTS only when PROXYSQL40=1. * test/tap/tests/unit/plugin_config_unit-t.cpp, test/tap/tests/unit/plugin_dispatch_unit-t.cpp: compile-time-gate the chassis-only subtests, adjust plan() counts per mode (48/50 with PROXYSQL40, 47/32 without), and provide a proxysql_init_configured_plugins_compat() helper that is a no-op returning true under !PROXYSQL40 so shared test helpers work in both modes. Verification — all plugin unit tests pass in both modes: PROXYSQL40=1 build (chassis enabled): plugin_config_unit-t: 48/48 plugin_dispatch_unit-t: 50/50 plugin_lifecycle_unit-t: 14/14 plugin_query_hook_unit-t: 41/41 PROXYSQL40=0 build (v3.x behavior): plugin_config_unit-t: 47/47 plugin_dispatch_unit-t: 32/32 (plugin_lifecycle, plugin_query_hook not built)	1 month ago
Rene Cannao	f13463d334	feat(plugin-abi): Step 2.2 — four-phase plugin lifecycle ... Adds the register_schemas() callback to the plugin descriptor and wires the loader + main.cpp so that a plugin's schema is declared BEFORE the admin module bootstraps, and the plugin's init() runs AFTER the schema has been materialized. Phase sequence (previously two-phase: load+init): A. load() — dlopen each .so, read the descriptor. B. register_schemas() — NEW, optional; plugin calls services->register_table with its admin/config/stats table defs. Services hand has register_table live but all DB-handle getters stubbed to nullptr. C. ProxySQL_Main_init_Admin_module + GloAdmin->materialize_plugin_tables — admin bootstrap drains the pending-tables list via merge_plugin_tables (first-boot == reload). D. init() — plugin's startup logic with full services (live DB handles pointing at a schema that already contains its own tables). E. start() — plugin launches threads / accept loops. ABI (include/ProxySQL_Plugin.h): * New callback typedef proxysql_plugin_register_schemas_cb. * New descriptor field register_schemas — appended at the end of ProxySQL_PluginDescriptor so existing 6-field initializers leave it null (aggregate init value-initializes the trailing slot). Plugins that left register_schemas null keep the pre-existing two-phase behavior: Phase B is skipped and their init() is responsible for both schema registration and startup work. * Phase-by-phase services availability matrix documented next to ProxySQL_PluginServices. Manager (lib/ProxySQL_PluginManager.cpp): * invoke_register_schemas_phase(err) — public method that runs Phase B for every plugin whose descriptor sets the callback. Uses a "services_phase_b_" variant where DB-handle getters are nullptr stubs; register_query_hook returns false with a warning (hooks cannot register this early). * proxysql_load_configured_plugins() — now stops at Phase B. On success, installs the built manager as the active one so that ProxySQL_Admin::materialize_plugin_tables (which reads from the active manager's pending-tables list) sees the registered tables. * proxysql_init_configured_plugins() — new entry point, calls init_all() for Phase D. Separated from load so admin bootstrap can run between the two. Main (src/main.cpp): * Reorder in ProxySQL_Main_init_phase2___not_started: LoadConfiguredPlugins(); // Phase A + B ProxySQL_Main_init_Admin_module(...); // Phase C GloAdmin->materialize_plugin_tables(); // Phase C (materialize) InitConfiguredPlugins(); // Phase D — NEW StartConfiguredPlugins(); // Phase E * Error messages updated to name the phase that failed. mysqlx plugin: * mysqlx_init split into: - mysqlx_register_schemas() — calls mysqlx_register_admin_schema (pure services->register_table use, no DB access); - mysqlx_init() — wires the plugin context (services, config store, started flag) and returns true. * Descriptor wires the new callback, exercising the full four- phase path end-to-end. Tests: * plugin_lifecycle_unit-t (new) — four cases: both callbacks fire in order; register_schemas-null skips Phase B; Phase-B DB handles return nullptr; Phase-B failure aborts init. * plugin_config_unit-t, plugin_dispatch_unit-t, plugin_query_hook_unit-t — existing tests updated to also call proxysql_init_configured_plugins after proxysql_load_configured_plugins where init side-effects are asserted. test_init_registration_failure_aborts_load reworked: the fake plugin's invalid-table registration happens in init (not register_schemas), so load now succeeds and the failure surfaces at init time. * test_mysqlx_admin_tables / test_mysqlx_listener_smoke — add an explicit invoke_register_schemas_phase() call before init_all() so the mysqlx plugin's schema gets registered under its new split. No behavior change for plugins that didn't opt into Phase B: they still see the same services_ layout and the same {load, init, start, stop} ordering. Opt-in plugins gain DB access from init() against a schema that already contains their tables, eliminating the chicken- and-egg that required ProtocolX's fd5f02947 workaround.	1 month ago
Rene Cannao	9ed621edac	test: substantially expand plugin unit test coverage ... Plugin assertions: 63 → 230 (+167) across 4 binaries. Per-binary deltas (assertions / plan): plugin_registry_unit-t : 25 → 68 (+43) plugin_manager_unit-t : 26 → 83 (+57) plugin_config_unit-t : 12 → 47 (+35) plugin_dispatch_unit-t : - → 32 (new file) New coverage areas: Registry edge cases: - case-insensitive duplicate-table detection (lower vs upper vs mixed) - case-insensitive duplicate-command detection across canonical forms - tabs / newlines / multi-space canonicalisation - leading + trailing spaces, multiple trailing semicolons (incl. interleaved) - internal semicolons preserved (not collapsed) - ";;;" / whitespace-only / "\\t\\n \\r ;" canonicalise to empty → reject - register_table input validation (null name, empty name, null/empty def, out-of-range db_kind) - same-name tables across admin/config/stats kinds all accepted - tables() returns empty for invalid db_kind - insertion-order preservation for both tables and commands - 64-table stress: all stored pointers remain valid after deque growth - 2KB command roundtrip - dispatch result fields (error_code / rows_affected / message) propagated Manager state machine: - load on a missing path: false, non-empty err mentions dlopen, size stays 0 - empty manager: init/start/stop all succeed - idempotent init/start/stop (re-calls are no-ops) - destructor stops started plugins (verified via log) - destructor SKIPS plugins that were never started - destructor doesn't re-run stop if explicit stop_all already happened - register_command failure inside init_all aborts and names the operation - register_table-with-invalid-kind inside init_all aborts and names op Multi-plugin (new fake_plugin2.so): - init/start happen in registration order; stop runs in reverse order (verified via shared log) - second plugin's init failure prevents both starts (no destructor stops) - second plugin's start failure: first plugin (which started) IS stopped by the destructor, second is NOT (never successfully started) Config helpers: - multiple plugins parsed - empty plugins=() clears stale modules - non-string entries (int, bool) silently skipped between strings - missing path bubbles up with the path in err - active manager visibility before/after load and stop - reload replaces previous manager (full lifecycle for both old and new) - reload to empty list: previous plugin stopped, manager nulled - partial-load failure leaves no active manager behind New plugin_dispatch_unit-t: - global dispatcher returns false when no manager active - dispatcher canonicalises input the same way registry does - dispatcher tolerates context with all-null DB pointers - dispatcher returns false after stop, even for a previously-routed command - 8 threads x 250 dispatches concurrent: 2000/2000 succeed, 0 fail - stop helper / start helper safe on null inputs Infrastructure changes: - test/tap/test_helpers/fake_plugin.cpp: parameterised via FAKE_PLUGIN_NAME and FAKE_PLUGIN_ENV_PREFIX so the same source can be compiled into a second .so (libproxysql_fake_plugin2.so) used by multi-plugin tests. Log entries now prefixed with "<name>:" so a shared log file can distinguish events from each plugin. - test/tap/tests/unit/Makefile: builds the second .so, wires it into plugin_manager_unit-t and plugin_config_unit-t, and adds the new plugin_dispatch_unit-t target. All 57 unit-test binaries pass.	1 month ago
Rene Cannao	55556979e0	feat(plugin-abi): Step 2.1 — pre-execution query-hook ABI + dispatch ... First half of Step 2 from the GenAI plugin carve-out design (see docs/superpowers/specs/2026-04-16-genai-plugin-carveout-design.md). Adds the query-hook ABI surface and the manager-level dispatch, but NOT yet the hot-path call sites in MySQL_Session / PgSQL_Session -- those are intentionally split into the next commit so the hot-path diff stays small and reviewable on its own. ABI extension (include/ProxySQL_Plugin.h): - ProxySQL_PluginProtocol enum {mysql, pgsql} - ProxySQL_PluginQueryHookPayload {user, client_ip, schema, query_text, query_len} - ProxySQL_PluginQueryHookAction enum {allow, deny} - ProxySQL_PluginQueryHookResult {action, message} - proxysql_plugin_query_hook_cb typedef - proxysql_plugin_register_query_hook_cb typedef - new field on ProxySQL_PluginServices: register_query_hook (additive at the end of the struct -- older plugins that were built against the previous layout don't read past it; new plugins must check non-null before calling, same convention used elsewhere) Manager (lib/ProxySQL_PluginManager.cpp + .h): - One hook per protocol per manager (nullptr means "no hook"). - register_query_hook(proto, cb): rejects null cb; rejects duplicate registration for a protocol that already has a hook; surfaces registration errors through the same note_registration_failure / init-aborts path that register_table and register_command use. - dispatch_query_hook(proto, payload, result): synchronous, returns true if a hook fired and stored its result, false otherwise. - has_query_hook(proto): cheap predicate for the hot-path predicate. Global helpers (for the hot path, next commit): - proxysql_dispatch_configured_plugin_query_hook(proto, payload, result): takes the active-manager mutex, dispatches if there is one. - proxysql_has_configured_plugin_query_hook(proto): lock-free atomic read of the manager pointer + a pointer-sized field read. Designed to be the gate the hot path checks first; the dispatch call can be elided entirely on the no-plugin path. Documented as spurious-true-tolerant -- the dispatch helper re-checks under the lock. Test plugin extension (test/tap/test_helpers/fake_plugin.cpp): - New env vars PROXYSQL_FAKE_PLUGIN[2]_REGISTER_QUERY_HOOK, REGISTER_QUERY_HOOK_PROTO ("mysql"|"pgsql"), HOOK_DENY. - fake_query_hook echoes the SQL through the result message so tests can verify the payload reached the callback intact. New unit test: plugin_query_hook_unit-t (41 assertions) - empty manager: neither protocol has a hook, dispatch returns false, result struct untouched - register MySQL allow hook → dispatch returns ALLOW with empty msg - register MySQL deny hook → dispatch returns DENY with the message - null callback rejected; duplicate registration rejected; original hook still in effect after rejected duplicate - protocols are independent (mysql allow + pgsql deny coexist) - payload threaded through (echo hook reads user/ip/schema/sql) - global dispatcher: false when no active manager, untouched result - global dispatcher with active manager: routes to the fake plugin's hook, propagates ALLOW vs DENY (env-flipped between calls), and goes back to false after stop_configured_plugins All 59 unit-test binaries pass.	1 month ago
Rene Cannao	3d107c3bed	chore: commit pre-existing plugin manager improvements ... Improvements left uncommitted at the end of Step 0: - atomic g_active_plugin_manager pointer - reject plugin descriptors with null/empty name - assert single-threaded init phase - debug-log dispatched plugin commands - propagate mysqlx_register_admin_schema failure into init - additional plugin manager unit tests (init/start/stop fail, double load) - updated ABI doc/comments for table copy semantics, command context lifetime, services availability, and status_json storage	1 month ago
Rene Cannao	9b87260b70	fix: harden plugin init failures and command dispatch	2 months ago
Rene Cannao	fd5f029474	fix: expose plugin db handles during startup	2 months ago
Rene Cannao	80fa6bee24	fix: cover configured plugin lifecycle wiring	2 months ago
Rene Cannao	da7e182719	fix: align plugin loader ABI and tests	2 months ago
Rene Cannao	7e1a12b8f7	feat: add generic plugin ABI and loader	2 months ago