Hi Tatsuo,
Thank you for the thorough review and the thoughtful pushback on the cross-session security concern. You're right that "dml_adaptive just hits himself in the foot, your patch allows him to hit someone else's foot" — that asymmetry needs a technical answer, not just a threat-model argument.
I've addressed this in the updated patch with two mechanisms:
1. Maximum staleness cap (`track_table_mutation_max_staleness`)
New configuration parameter (default: 60 seconds, range: 0–3600000 ms). This bounds how long any single table entry can continuously force primary routing, measured from the first write that created the entry. Even under sustained writes, the entry expires after this period. If the table is written to again after expiry, a fresh entry is created.
This directly addresses the concern: the worst-case cross-session impact is bounded and configurable. An operator can look at this parameter and know: "no matter what, a table's staleness effect on other sessions cannot exceed X seconds continuously."
For legitimately busy tables, the brief gap between forced expiry and the next write re-marking the table is negligible — typically milliseconds, since writes are frequent. The correctness guarantee is preserved.
2. Database-scoped isolation (documented)
The tracking is already scoped by database OID — writes in one database never affect routing decisions for sessions in a different database. I've documented this explicitly as a security boundary in the docs. In multi-tenant deployments with separate databases, tenants are isolated from each other's write activity.
Combined with the existing safeguards (committed writes only, bounded table map size, opt-in mode), the cross-session impact is now bounded in both duration and scope.
I've also addressed the other review comments — they should be applied in the patch as well. tests and code structure.
Thanks!
> Added some handling for possible causes - works now.
Here are comments for the patch.
- Some code lines are too long. We recommend to limit each source code
line up to 78 chars. You can use following script to detect too long
lines (you can ignore reports other than *.[c]) See
https://wiki.postgresql.org/wiki/Committing_checklist
git diff origin/master | grep -E '^(\+|diff)' | sed 's/^+//' | expand -t4 | awk "length > 78 || /^diff/"
--- /dev/null
+++ b/src/test/regression/tests/043.track_table_mutation_watchdog/.gitignore
Please avoid to install .gitignore. .gitignore file are maintained by
pgpool core developers.
+++ b/src/test/regression/tests/043.track_table_mutation_watchdog/leader.conf
To test watchdog, you should use the standard watchdog_setup too.
+++ b/src/utils/pool_track_table_mutation.c
+static inline void
+query_cache_lock(void)
"query_cache_*" is confusing since we already have query cache
feature. Please use different name.
+static int
+track_table_mutation_get_database_oid_internal(void)
+{
:
:
+ /* Ensure we have a valid query context */
+ if (session_context->query_context == NULL)
+ return oid;
Why does this need? The query context is not used in this function.
+/* ----------------
+ * Public API implementation
+ * ----------------
+ */
Please add a comments on what these function do.
+Size
+pool_track_table_mutation_shmem_size(void)
+void
+pool_track_table_mutation_init(void)
+void
+pool_track_table_mutation_child_init(void)
+bool
+pool_track_table_mutation_in_cold_start(void)
+void
+pool_track_table_mutation_trigger_global_cold_start(void)
+bool
+pool_track_table_mutation_table_is_stale(int table_oid, int dboid)
__sync_fetch_and_add are old functions. I recommend to replace with
ordinary statements using semaphore to protect the critical region.
+ __sync_fetch_and_add(&track_table_mutation_shmem->state.stats_queries_checked, 1);
Please add a comments on what these function do.
+pool_track_table_mutation_mark_tables_written(const int *table_oids, int num_tables, int dboid)
+void
+pool_track_table_mutation_update_ttl(uint64 delay_us)
+bool
+pool_track_table_mutation_get_cached_parse(uint64 hash, bool *is_write,
+void
+pool_track_table_mutation_cache_parse(uint64 hash, bool is_write,
+ const char table_names[][TRACK_TABLE_MUTATION_TABLE_NAME_LEN],
+ int num_tables)
+uint64
+pool_track_table_mutation_normalize_and_hash(const char *query)
Best regards,
--
Tatsuo Ishii
SRA OSS K.K.
English: http://www.sraoss.co.jp/index_en/
Japanese:http://www.sraoss.co.jp
--
Nadav Shatz
Tailor Brands | CTO