Re: Proposal: Recent mutated table tracking in memory

From: Nadav Shatz <nadav(at)tailorbrands(dot)com>
To: Tatsuo Ishii <ishii(at)postgresql(dot)org>
Cc: pgpool-hackers(at)lists(dot)postgresql(dot)org
Subject: Re: Proposal: Recent mutated table tracking in memory
Date: 2026-02-26 15:26:33
Message-ID: CACeKOO1SCQLwx4=4gp6+swH5NW_GLJCd0uOSL0cU2U7VFR_o8g@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgpool-hackers

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!

On Thu, Feb 26, 2026 at 9:47 AM Tatsuo Ishii <ishii(at)postgresql(dot)org> wrote:

> > 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

Attachment Content-Type Size
table_track.patch application/octet-stream 102.1 KB

In response to

Responses

Browse pgpool-hackers by date

  From Date Subject
Next Message Tatsuo Ishii 2026-03-02 01:00:28 Close listening socokets before forking
Previous Message Tatsuo Ishii 2026-02-26 07:47:42 Re: Proposal: Recent mutated table tracking in memory