public inbox for [email protected]
help / color / mirror / Atom feedFrom: Tatsuo Ishii <[email protected]>
To: [email protected]
Cc: [email protected]
Subject: Re: Proposal: Recent mutated table tracking in memory
Date: Tue, 23 Jun 2026 20:38:48 +0900 (JST)
Message-ID: <[email protected]> (raw)
In-Reply-To: <CACeKOO31reCtDYU1twicmqgdEStCJXWJ6P4LMFMYytaYaGew-A@mail.gmail.com>
References: <CACeKOO02V7_SBpCUJWG7yS-ABNEXzAAU-vb+JrY=rD8yBiiy0g@mail.gmail.com>
<[email protected]>
<CACeKOO31reCtDYU1twicmqgdEStCJXWJ6P4LMFMYytaYaGew-A@mail.gmail.com>
Hi Nadav,
Thank you for updating the patch. I will look into this.
Regards,
--
Tatsuo Ishii
SRA OSS K.K.
English: http://www.sraoss.co.jp/index_en/
Japanese:http://www.sraoss.co.jp
> Hi Tatsuo,
>
> Good catch -- that was a bug, and chasing it down turned up a second
> one in the same area.
>
> The original problem you described: the flush ran in dml_adaptive()
> while pgpool was *routing* the COMMIT, before the COMMIT was sent to
> the backend. So a COMMIT that later failed (your deferred-constraint
> example) had already marked the tables, even though the writes rolled
> back.
>
> My first instinct was to just move the flush to handle_query_context(),
> where reaching CommandComplete for the COMMIT means it succeeded. But
> that path resolves table/database OIDs via pool_table_name_to_oid() /
> the relcache, which on a cache miss issues a do_query() -- and at that
> point the COMMIT response is still being read from the backend, so
> injecting a query desyncs the protocol and hangs the session. Testing
> caught this immediately.
>
> It turned out the existing autocommit marking branch had the very same
> hazard: pool_extract_table_oids() in handle_query_context() also does a
> relcache do_query() on a miss, so e.g. "DROP TABLE IF EXISTS <nonexistent>"
> (an uncacheable name -> guaranteed miss) reliably hung the session too.
>
> So I fixed both uniformly: resolve the written tables' OIDs (and the
> database OID) at DML *routing* time, when the connection is idle and
> do_query is safe -- the same context where the feature already resolves
> OIDs for the SELECT staleness check. The OIDs are stashed in the
> session context and only *marked* in shared memory at CommandComplete,
> which now does no backend I/O at all. Autocommit statements mark on
> their own CommandComplete; explicit transactions accumulate and mark on
> COMMIT; ROLLBACK and failed/rolled-back COMMITs never reach the mark
> step, so their writes are correctly not recorded.
>
> A side benefit: using pool_extract_table_oids() for both paths means we
> now mark only the actual write *targets*, instead of every relation in
> the statement (the in-transaction path previously walked all RangeVars,
> so an "INSERT ... SELECT FROM t" would also mark the read source t).
>
> Since your really_writing_transaction fix is now in master (1333d2ae8),
> I dropped the copy I was carrying and rebased the feature directly onto
> current master. One patch this time:
>
> v6-0001-Feature-load-balancing-control-by-table-tracking.patch
>
> Verified on a PG16 streaming-replication cluster (dml_adaptive_global,
> standby weighted for load balancing):
>
> - your deferred-constraint COMMIT fails -> table not marked -> reads
> keep load-balancing to the standby;
> - a successful COMMIT, and a successful autocommit write -> table
> marked -> reads forced to the primary;
> - COMMIT returns promptly (no hang), and "DROP TABLE IF EXISTS
> <nonexistent>" no longer hangs.
>
> Thanks!
>
> On Fri, Jun 19, 2026 at 5:10 AM Tatsuo Ishii <[email protected]> wrote:
>
>> > understood. we'll wait patiently or try master.
>> >
>> > thanks!
>>
>> While reviewing
>> v5-0002-Feature-load-balancing-control-by-table-tracking.patch, I
>> noticed following code:
>>
>> static void
>> dml_adaptive(Node *node, char *query)
>> {
>> :
>> :
>> + /*
>> + * For dml_adaptive_global: on COMMIT,
>> flush the accumulated
>> + * table writes to shared memory. On
>> ROLLBACK, skip -- the
>> + * writes never committed so no stale-read
>> risk exists. This
>> + * prevents polluting the table map with
>> rolled-back
>> + * transactions.
>> + */
>> + int dlbow =
>> + pool_config->disable_load_balance_on_write;
>> + List *wlist =
>> +
>> session_context->transaction_temp_write_list;
>> +
>> + if (dlbow == DLBOW_DML_ADAPTIVE_GLOBAL &&
>> + is_commit_query(node) &&
>> + wlist != NIL)
>> + {
>> + ListCell *cell;
>> + int dboid;
>> +
>> + dboid =
>> +
>> pool_track_table_mutation_get_database_oid();
>> + if (dboid > 0)
>> + {
>> + foreach(cell, wlist)
>> + {
>> + char *tname;
>> + int
>> toid;
>> +
>> + tname = (char *)
>> lfirst(cell);
>> + toid =
>> +
>> pool_table_name_to_oid(tname);
>> +
>> + if (toid > 0)
>> +
>> pool_track_table_mutation_mark_table_written(
>> +
>>
>> toid, dboid);
>> + }
>>
>> The code path is executed before the commit command is sent to primary
>> PostgreSQL. I wonder what will happen if the commit command failed?
>> Example:
>>
>> CREATE TEMP TABLE t1(i int PRIMARY KEY DEFERRABLE INITIALLY DEFERRED);
>> CREATE TABLE
>> INSERT INTO t1 VALUES(1);
>> INSERT 0 1
>> BEGIN;
>> BEGIN
>> INSERT INTO t1 VALUES(1);
>> INSERT 0 1
>> INSERT INTO t1 VALUES(2);
>> INSERT 0 1
>> COMMIT;
>> psql:deferable.sql:6: ERROR: duplicate key value violates unique
>> constraint "t1_pkey"
>> DETAIL: Key (i)=(1) already exists.
>>
>> As you can see, the COMMIT command failed and the effect of INSERT
>> commands vanished too. However the code invalidates the data on the
>> shared memory by pool_track_table_mutation_mark_table_written() when
>> it should not be called?
>>
>> 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
view thread (61+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected]
Subject: Re: Proposal: Recent mutated table tracking in memory
In-Reply-To: <[email protected]>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox