docs(chassis): mark query-hook ABI scaffold-state explicit + add TODO markers

The chassis ABI 2 query-hook surface (ProxySQL_PluginQueryHookPayload /
Result / Action plus register_query_hook / dispatch_configured_plugin_
query_hook) is wired through ProxySQL_PluginManager and exercised end-
to-end by unit tests. But the production data plane never calls
proxysql_dispatch_configured_plugin_query_hook — neither MySQL_Session
nor PgSQL_Session has the integration point.

Net effect today: a plugin can register a query hook successfully and
unit tests can drive dispatch through it, but a real client query
arriving over MySQL or PgSQL will never consult the hook. The DENY
contract from the public ABI ("DENY prevents a query from dispatching")
is currently a promise the production path doesn't keep.

# Why scaffold-state instead of immediate full integration

Wiring the dispatch into MySQL_Session::handler___status_WAITING_
CLIENT_DATA___STATE_SLEEP___MYSQL_COM_QUERY_qpo (and the COM_STMT_
PREPARE / COM_STMT_EXECUTE entries that share the qpo machinery) is
mechanical but touches a 95k-LOC hot-path file. A bug in the
integration would manifest as a query-blocking regression that's hard
to debug and easy to ship.

Same for PgSQL_Session. The integration deserves its own focused PR
with TAP coverage that exercises both ALLOW and DENY paths against a
fake plugin — not a side-quest in this PR which is already large.

# What this commit does

1. Updates the ABI doc block in include/ProxySQL_Plugin.h to call out
   the scaffold state explicitly. A plugin author reading the header
   today would reasonably assume the hook fires in production; that
   assumption is wrong, and the doc block now says so. References the
   precise grep target ("TODO(plugin-query-hook)") that points at the
   missing injection sites.

2. Adds two TODO(plugin-query-hook) markers to MySQL_Session.cpp:
   - line ~3387 (COM_STMT_PREPARE → COM_QUERY shared codepath)
   - line ~5403 (the main COM_QUERY execution path)
   Each immediately follows the GloMyQPro->process_query call, where
   CurrentQuery is populated and the dispatch payload would be ready.

3. Adds one TODO(plugin-query-hook) marker to PgSQL_Session.cpp at
   line ~2387, following the GloPgQPro->process_query call.

# What this commit deliberately does NOT do

The actual dispatch wiring. The next person picking this up has:
- A precise injection site (3 in MySQL, 1+ in PgSQL).
- Pre-built ABI surface (proxysql_has_configured_plugin_query_hook
  for the lock-free fast-path probe; proxysql_dispatch_configured_
  plugin_query_hook for the actual hook invocation under shared lock).
- Existing test_helpers/fake_plugin.cpp with a HOOK_DENY env toggle
  that exercises the dispatch path under unit test today — extensible
  to TAP integration tests once the production wiring lands.

Caught by an external review pass, finding #5. Filing as a deferred
fix with explicit scaffold acknowledgement rather than rushing the
integration.
pull/5702/head
Rene Cannao 3 weeks ago
parent e535a66ee1
commit 6d8dff2939

@ -102,6 +102,19 @@ using proxysql_plugin_log_message_cb =
#ifdef PROXYSQL40
// Pre-execution query hook (Step 2 ABI extension).
//
// STATUS (chassis ABI 2 — initial baseline): the registration and
// dispatch path are wired through ProxySQL_PluginManager and the
// chassis fast-path probe (proxysql_has_configured_plugin_query_hook),
// and unit tests exercise the dispatch surface end-to-end. However,
// the production data plane (MySQL_Session::handler___status_WAITING_
// CLIENT_DATA___STATE_SLEEP___MYSQL_COM_QUERY_qpo and the matching
// PgSQL_Session COM_QUERY entry) does NOT yet call the dispatch
// helper. Plugins that register a query hook today will see the
// callback invoked from unit tests but not from real client traffic.
// Wiring the production fast-path is a deliberate follow-up — search
// for "TODO(plugin-query-hook)" in lib/MySQL_Session.cpp and
// lib/PgSQL_Session.cpp for the precise injection points.
//
// Wire protocol the hook is being invoked for. A plugin can register
// independently for each protocol; one hook per protocol per plugin.
enum class ProxySQL_PluginProtocol : uint8_t {

@ -3383,6 +3383,16 @@ void MySQL_Session::handler___status_WAITING_CLIENT_DATA___STATE_SLEEP___MYSQL_C
(begint.tv_sec*1000000000+begint.tv_nsec);
}
assert(qpo); // GloMyQPro->process_mysql_query() should always return a qpo
// TODO(plugin-query-hook): inject pre-execution hook dispatch
// here, after CurrentQuery is populated and before the query
// is committed for backend execution. The chassis ABI surface
// (proxysql_has_configured_plugin_query_hook +
// proxysql_dispatch_configured_plugin_query_hook in
// include/ProxySQL_PluginManager.h) is in place; this is the
// missing data-plane integration. See ProxySQL_Plugin.h's
// "STATUS (chassis ABI 2 — initial baseline)" comment for
// the full scope. Same injection pattern needed at line ~5398
// (this same source file) and the PgSQL COM_QUERY entry.
// setting 'prepared' to prevent fetching results from the cache if the digest matches
rc_break=handler___status_WAITING_CLIENT_DATA___STATE_SLEEP___MYSQL_COM_QUERY_qpo(&pkt, &lock_hostgroup, ps_type_prepare_stmt);
if (rc_break==true) {
@ -5402,6 +5412,11 @@ __get_pkts_from_client:
(endt.tv_sec*1000000000+endt.tv_nsec) -
(begint.tv_sec*1000000000+begint.tv_nsec);
}
// TODO(plugin-query-hook): inject pre-execution
// hook dispatch here for the COM_QUERY codepath,
// after CurrentQuery is populated. See
// ProxySQL_Plugin.h "STATUS (chassis ABI 2 —
// initial baseline)".
assert(qpo); // GloMyQPro->process_mysql_query() should always return a qpo
{

@ -2385,6 +2385,15 @@ __implicit_sync:
(endt.tv_sec * 1000000000 + endt.tv_nsec) -
(begint.tv_sec * 1000000000 + begint.tv_nsec);
}
// TODO(plugin-query-hook): inject pre-execution
// hook dispatch here for the PgSQL COM_QUERY
// codepath, after CurrentQuery is populated.
// Use proxysql_dispatch_configured_plugin_
// query_hook with ProxySQL_PluginProtocol::pgsql.
// See ProxySQL_Plugin.h "STATUS (chassis ABI 2 —
// initial baseline)" — the dispatch ABI is
// already in place; this is the data-plane
// integration that needs to land.
assert(qpo); // GloPgQPro->process_mysql_query() should always return a qpo
// ===================================================
if (qpo->max_lag_ms >= 0) {

Loading…
Cancel
Save