From 29ee30daf9cf449fd96b5d1c03ebe1e17f24567c Mon Sep 17 00:00:00 2001 From: Rene Cannao Date: Fri, 1 May 2026 04:15:24 +0000 Subject: [PATCH] docs(plugin-chassis): address PR-#5690 review feedback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CodeRabbit + Gemini review pass surfaced four small doc-precision items, all legitimate. None were code bugs. # CodeRabbit (markdownlint nit) - doc/PLUGIN_API.md:455: fenced code block at "DISK ↔ MEMORY ↔ RUNTIME" lacked a language specifier. Added `text` to satisfy MD040 / fenced-code-language. # Gemini (3x same root: clarify the matching algorithm) The chassis dispatch matcher is case-insensitive whole-identifier substring match, but three docs described it loosely. Plugin authors deciding what table name to register need to know that runtime_X does not match runtime_X_extra or stats_runtime_X. - doc/PLUGIN_API.md:289 ("any admin SELECT against it") -> spell out the whole-identifier rule with the longer-prefix / longer-suffix rejection examples. - doc/plugin-chassis/ABI.md:156 ("appears in the SQL") -> "is referenced as a whole identifier in the SQL query (case-insensitive; identifier-aware, so runtime_X_extra or stats_runtime_X do not match runtime_X)". - doc/plugin-chassis/FILE_CHANGES.md:102 ("substring match") -> "case-insensitive whole-identifier substring match", matching the precise wording already used in section B for the sql_references_table_ci helper. Cross-section consistency. No source code touched. No new content sections, only existing prose tightened. --- doc/PLUGIN_API.md | 12 ++++++++---- doc/plugin-chassis/ABI.md | 2 +- doc/plugin-chassis/FILE_CHANGES.md | 2 +- 3 files changed, 10 insertions(+), 6 deletions(-) diff --git a/doc/PLUGIN_API.md b/doc/PLUGIN_API.md index 27c6fe06a..89198ab6a 100644 --- a/doc/PLUGIN_API.md +++ b/doc/PLUGIN_API.md @@ -286,9 +286,13 @@ bool register_runtime_view(const ProxySQL_PluginRuntimeView &view); Declare an admin-side **view** of plugin-module state. The named `table_name` lives in `admin_db` (typically `runtime_`) and holds no persistent rows — the chassis invokes `refresh(admindb, -opaque)` before any admin SELECT against it. The refresh callback is -expected to do (typically) `BEGIN; DELETE FROM ; INSERT/REPLACE -INTO
...; COMMIT;` from the module's own in-memory state. +opaque)` before any admin `SELECT` that references the table as a +whole identifier. The match is case-insensitive but identifier-aware: +a query against `runtime_
_extra` (longer suffix) or +`stats_runtime_
` (longer prefix) does NOT trigger the refresh +for `runtime_
`. The refresh callback is expected to do +(typically) `BEGIN; DELETE FROM
; INSERT/REPLACE INTO +
...; COMMIT;` from the module's own in-memory state. The chassis deep-copies `table_name` so the plugin need not keep the pointed-to string alive after registration. The callback pointer must @@ -447,7 +451,7 @@ callbacks, not by linking against ProxySQL's SQLite wrapper. ProxySQL's three-tier configuration model is, in storage terms: -``` +```text DISK (config_db) ↔ MEMORY (admin_db editable tables) ↔ RUNTIME (in-module state) ``` diff --git a/doc/plugin-chassis/ABI.md b/doc/plugin-chassis/ABI.md index c1c6b201d..32e46e0ff 100644 --- a/doc/plugin-chassis/ABI.md +++ b/doc/plugin-chassis/ABI.md @@ -153,7 +153,7 @@ Therefore: - `LOAD TO RUNTIME` reads the editable admin table and hands the rows to the module via a typed install API that swaps state under the module's own lock. It MUST NOT touch `runtime_`. - `SAVE [FROM RUNTIME] TO MEMORY` dumps the module's in-memory state and `REPLACE INTO`s the editable admin table. It MUST NOT read `runtime_`. -- `runtime_` is repopulated by the registered refresh callback before any admin SELECT touches it. Admin's pre-SELECT hook walks every registered view and invokes the callback for any whose table name appears in the SQL. +- `runtime_` is repopulated by the registered refresh callback before any admin SELECT touches it. Admin's pre-SELECT hook walks every registered view and invokes the callback for any view whose table name is referenced as a whole identifier in the SQL query (case-insensitive; identifier-aware, so `runtime__extra` or `stats_runtime_` do not match `runtime_`). Disk-tier copies (`LOAD FROM DISK`, `SAVE TO DISK`) are the exception: those DO copy between configdb and admindb persistent tables, and they remain plain `BEGIN/DELETE/INSERT/COMMIT` with checked rollback. For those, the **empty-source-must-still-clear-destination** rule still applies — a `DELETE FROM mysqlx_users; SAVE MYSQLX USERS TO DISK;` must leave the disk table empty, not preserve the previous rows. PR #5643 fixed an early mysqlx implementation that omitted the unconditional DELETE on the disk path. diff --git a/doc/plugin-chassis/FILE_CHANGES.md b/doc/plugin-chassis/FILE_CHANGES.md index 6df6ed6d5..d66d4cd8b 100644 --- a/doc/plugin-chassis/FILE_CHANGES.md +++ b/doc/plugin-chassis/FILE_CHANGES.md @@ -99,7 +99,7 @@ Sections **A–G** cover the **chassis core**. Sections **H–O** cover the **my - Three free functions `proxysql_plugin_get_admindb/configdb/statsdb()` return `GloAdmin->{admindb,configdb,statsdb}` if `GloAdmin` is non-null. Gated by `PROXYSQL40` so v3.x exports no plugin-aware symbols at all. - `ProxySQL_Admin::dispatch_plugin_admin_command(sess, sql)` builds a `ProxySQL_PluginCommandContext{admindb,configdb,statsdb}`, calls `proxysql_dispatch_configured_plugin_admin_command`, and translates `result` into `send_ok_msg_to_client` / `send_error_msg_to_client`. - Explicit template instantiations for `MySQL_Session` and `PgSQL_Session`. - - **Pre-SELECT runtime-view dispatch in `GenericRefreshStatistics`** (around line 1654): `proxysql_refresh_configured_plugin_runtime_views(query_no_space, admindb)` is called for **every admin-port query**, gated only on `if (admin)` and placed **outside** the existing `if (refresh==true)` block. The chassis itself decides whether any plugin's refresh callback fires, by per-view substring match against the query — a query that touches no registered view is a cheap no-op (one shared lock + N substring scans). The `refresh` flag is left untouched; it gates a separate set of core-only refreshes (stats_mysql_processlist, runtime_mysql_users, etc.). + - **Pre-SELECT runtime-view dispatch in `GenericRefreshStatistics`** (around line 1654): `proxysql_refresh_configured_plugin_runtime_views(query_no_space, admindb)` is called for **every admin-port query**, gated only on `if (admin)` and placed **outside** the existing `if (refresh==true)` block. The chassis itself decides whether any plugin's refresh callback fires, by case-insensitive whole-identifier substring match against the query (matching the wording used in section B for the `sql_references_table_ci` matcher) — a query that touches no registered view is a cheap no-op (one shared lock + N substring scans). The `refresh` flag is left untouched; it gates a separate set of core-only refreshes (stats_mysql_processlist, runtime_mysql_users, etc.). ### `include/proxysql_admin.h` — MODIFIED, +8 lines