| From: | Nadav Shatz <nadav(at)tailorbrands(dot)com> |
|---|---|
| To: | pgpool-hackers(at)lists(dot)postgresql(dot)org |
| Subject: | [PATCH] Two small track_table_mutation follow-ups: doc figures and unused counters |
| Date: | 2026-09-22 06:30:27 |
| Message-ID: | CACeKOO1v6+KiK-qns_37zCz7YYxJ6Zi_xKaLAuQcjkYQcjQu2Q@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgpool-hackers |
Hi All,
While re-reading the committed track_table_mutation code I found three
small things that are wrong in master. Two patches attached, split by
concern.
0001 - Doc: fix memory figures and eviction description
The docs understate the feature's shared memory and describe its eviction
order incorrectly.
TrackTableMutationEntry is 56 bytes, not the documented 40. It holds two
struct timeval fields (16 bytes each on LP64), two ints, a uint32, an int
and a bool, which pad to 56. I confirmed this by compiling the struct on
both Linux (aarch64) and macOS; sizeof is 56 on both, with in_use at
offset 48.
So at the default track_table_mutation_table_size of 2048 the entries
alone are ~112 KB, plus ~4 KB for the default 1024-entry bucket array,
giving ~116 KB rather than the documented ~80 KB. The 4096-table example
needs ~228 KB, not ~160 KB. The bucket array was not accounted for at all
before, so the patch mentions it explicitly.
Separately, the docs say that when the map is full "oldest entries are
evicted". table_map_insert() actually scans from bucket zero and evicts
the head of the first non-empty bucket, so eviction order follows the
hash bucket layout rather than write time. The patch describes what the
code does, and adds the mitigating detail that expired entries are
reclaimed opportunistically, so a live entry is only evicted when more
than track_table_mutation_table_size tables are written within one TTL
window.
I only touched the English documentation. doc.ja carries the same
figures and the same eviction wording, but I would rather leave that to
you than guess at the phrasing.
0002 - Remove unused track_table_mutation statistics counters
TrackTableMutationState carries three counters - stats_queries_checked,
stats_forced_primary and stats_allowed_replica - that are initialized and
incremented but never read. Nothing in the tree consumes them; there is
no SHOW, reporting view or pcp command that exposes them, and the
pool_track_table_mutation_get_stats() accessor that would have is one of
the functions removed before commit.
They are not free. pool_track_table_mutation_table_is_stale() releases
the table map semaphore after the lookup and then re-acquires it only to
bump these counters, so every staleness check on every load-balanceable
SELECT makes a second trip through the semaphore to update state nobody
can observe. Removing them drops that critical section and lets the
table_oid/dboid sanity check return directly rather than jumping to a
shared epilogue.
The return value of pool_track_table_mutation_table_is_stale() is
unchanged for every input, so there is no functional change.
Both patches apply cleanly to current master, build without new warnings,
and pass the 78-column check from the committing checklist.
Thanks,
--
Nadav Shatz
Tailor Brands | CTO
| Attachment | Content-Type | Size |
|---|---|---|
| 0002-Remove-unused-track_table_mutation-statistics-counte.patch | application/octet-stream | 3.6 KB |
| 0001-Doc-fix-track_table_mutation-memory-figures-and-evic.patch | application/octet-stream | 3.9 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Tatsuo Ishii | 2026-09-29 11:00:01 | Re: Fixing pool_read2 |
| Previous Message | Koshino Taiki | 2026-09-10 07:56:35 | Fix response buffer pointer advancement in do_error_execute_command |