Re: Proposal: Recent mutated table tracking in memory

> Hi Tatsuo,
>
> hank you for the detailed review. Attached patch addresses all items.

I guess the attached patch is on top of
v1-0001-Feature-load-balancing-control-by-table-tracking.patch. To
apply v2-0001-address-review.patch, we need to apply
v1-0001-Feature-load-balancing-control-by-table-tracking.patch first.
Unfortunately due to recent commit, it does not apply anymore. Can you
please provide v1 + v2 that are rebased against latest master branch?
Also 042 regression test is already used by recent commit. Can you
renumber 042.track_table_mutation and
043.track_table_mutation_watchdog to 043.track_table_mutation and
044.track_table_mutation_watchdog accordingly?

Looking forward to seeing new patch.

Regards,
--
Tatsuo Ishii
SRA OSS K.K.
English: http://www.sraoss.co.jp/index_en/
Japanese:http://www.sraoss.co.jp

> memqcache bug fix
> -----------------
>
> Good catch. The root cause: pool_set_writing_transaction() was
> explicitly skipping dml_adaptive_global, so
> pool_is_writing_transaction() always returned false in this mode.
> The query cache fetch guard at pool_proto_modules.c:270
> (!pool_is_writing_transaction()) then served stale cached results
> after DML in the same transaction.
>
> Fix: pool_set_writing_transaction() now sets the flag for
> dml_adaptive_global (only 'off' and 'dml_adaptive' skip it). This
> ensures the query cache is properly bypassed after writes within
> the same transaction.
>
> Removed dead query parse cache code (~700 lines)
> -------------------------------------------------
>
> You're right -- pool_track_table_mutation_get_cached_parse,
> pool_track_table_mutation_cache_parse, and
> pool_track_table_mutation_normalize_and_hash were never called.
> These were leftover from an earlier design where we planned to
> cache SQL parse results in shared memory. The feature ended up
> using pgpool's existing parser directly, and this code was never
> wired up.
>
> Removed: QueryParseCache and QueryParseEntry structs, all related
> static functions, the TRACK_TABLE_MUTATION_QUERY_SEM semaphore,
> and the track_table_mutation_query_buckets /
> track_table_mutation_query_parse_cache_size configuration
> parameters. This also reduces shared memory usage from ~6.4 MB
> to ~80 KB with default settings.
>
> check_object_relationship_list scope
> -------------------------------------
>
> You're correct -- dml_adaptive_global does not use
> dml_adaptive_object_relationship_list. Changed
> check_object_relationship_list() to check for DLBOW_DML_ADAPTIVE
> only, not DLBOW_IS_DML_ADAPTIVE (which includes global).
>
> Documentation fixes
> -------------------
>
> - Removed "(Lagless Replica Reads)" from section title and
> "lagless" language from description.
>
> - Described fallback behavior when neither
> replication_delay_source_cmd nor delay_threshold_by_time is
> configured (TTL stays at 100ms default minimum).
>
> - "query cache" references removed (the query parse cache is gone).
>
> - Added 128-table-per-SELECT limit to Limitations section
> (uses POOL_MAX_SELECT_OIDS).
>
> Code style fixes
> ----------------
>
> - DLBOW_IS_DML_ADAPTIVE() calls no longer split across lines.
>
> - Split the long errmsg line in
> is_select_object_in_temp_write_list.
>
> - Removed redundant is_adaptive variable in
> is_select_object_in_temp_write_list (the check at function
> entry already guarantees it).
>
> Thanks!
>
> On Wed, Apr 15, 2026 at 1:43 AM Tatsuo Ishii <ishii(at)postgresql(dot)org> wrote:
>
>> Hi Nadav,
>>
>> > Hi Tatsuo,
>> >
>> > Looks good to me thanks!
>> >
>> > Please go ahead with your review. waiting to hear back from you.
>>
>> Here are the code review results.
>>
>> diff --git a/doc/src/sgml/loadbalance.sgml b/doc/src/sgml/loadbalance.sgml
>> index 9e1e7b39b..7384ce81a 100644
>> --- a/doc/src/sgml/loadbalance.sgml
>> +++ b/doc/src/sgml/loadbalance.sgml
>> :
>> + <sect2 id="runtime-config-table-mutation-map">
>> + <title>Table Mutation Map Configuration (Lagless Replica Reads)</title>
>>
>> "(Lagless Replica Reads)" sounds like an advertisement to me. It
>> should be removed.
>>
>> + <para>
>> + These parameters configure the track table mutation feature, which is
>> activated by setting
>> + <xref linkend="guc-disable-load-balance-on-write"> to
>> <literal>dml_adaptive_global</literal>.
>> + The feature tracks recently written tables to prevent stale reads from
>> replica nodes during
>> + replication lag, implementing the "lagless" architecture pattern for
>> distributed systems
>> + with read replicas.
>>
>> I think the feature does not guarantee "lagless" anytime, in all cases.
>>
>> + <para>
>> + This feature requires time-based replication delay monitoring. This
>> can be provided by either
>> + <xref linkend="guc-replication-delay-source-cmd"> (external command
>> mode) or by setting
>> + <xref linkend="guc-delay-threshold-by-time"> (which uses
>> <literal>pg_stat_replication.replay_lag</literal>
>> + from PostgreSQL 10+). At least one of these must be configured for the
>> TTL calculation to work.
>>
>> If one of these is not set, what happens? Error? Need to describe it.
>>
>> + </para>
>> +
>> + <warning>
>> + <para>
>> + Enabling <literal>dml_adaptive_global</literal> increases shared
>> memory consumption. With default settings,
>> + the feature requires approximately 6.4 MB of shared memory (0.1 MB
>> for table tracking + 6.3 MB for query cache).
>>
>> "query cache" should be "query parse cache".
>>
>> + Memory usage scales with configuration parameters:
>> + </para>
>> + <itemizedlist>
>> + <listitem>
>> + <para>
>> + Table tracking: <literal>track_table_mutation_table_size * 40
>> bytes</literal> (default: 2048 * 40 = ~80 KB)
>> + </para>
>> + </listitem>
>> + <listitem>
>> + <para>
>> + Query cache: <literal>track_table_mutation_query_parse_cache_size *
>> 640 bytes</literal> (default: 10000 * 640 = ~6.3 MB)
>>
>> "query cache" should be "query parse cache".
>>
>> + <title>Limitations</title>
>>
>> I think number of tables tacked in a SELECT is limited to 8. It should
>> be mentioned.
>>
>> diff --git a/src/context/pool_query_context.c
>> b/src/context/pool_query_context.c
>> index a056ac596..0190d3673 100644
>> --- a/src/context/pool_query_context.c
>> +++ b/src/context/pool_query_context.c
>> @@ -1828,15 +1829,23 @@ is_in_list(char *name, List *list)
>> static bool
>> is_select_object_in_temp_write_list(Node *node, void *context)
>> {
>> - if (node == NULL || pool_config->disable_load_balance_on_write !=
>> DLBOW_DML_ADAPTIVE)
>> + if (node == NULL ||
>> + !DLBOW_IS_DML_ADAPTIVE(
>> +
>> pool_config->disable_load_balance_on_write))
>>
>> You don't need to split the line.
>>
>> + is_adaptive = DLBOW_IS_DML_ADAPTIVE(
>> +
>> pool_config->disable_load_balance_on_write);
>>
>> You don't need to split the line.
>>
>> - if (pool_config->disable_load_balance_on_write ==
>> DLBOW_DML_ADAPTIVE && session_context->is_in_transaction)
>> + if (is_adaptive &&
>> + session_context->is_in_transaction)
>> {
>> ereport(DEBUG1,
>>
>> (errmsg("is_select_object_in_temp_write_list: \"%s\", found relation
>> \"%s\"", (char *) context, rgv->relname)));
>> This line is too long. Please split.
>>
>> @@ -1880,7 +1889,13 @@ static char
>> *get_associated_object_from_dml_adaptive_relations
>> void
>> check_object_relationship_list(char *name, bool is_func_name)
>> {
>> - if (pool_config->disable_load_balance_on_write ==
>> DLBOW_DML_ADAPTIVE &&
>> pool_config->parsed_dml_adaptive_object_relationship_list)
>> + bool is_adaptive;
>> +
>> + is_adaptive = DLBOW_IS_DML_ADAPTIVE(
>> +
>> pool_config->disable_load_balance_on_write);
>>
>> I wrote in the commit message:
>>
>> modifications are only detected in the same transaction). Note,
>> however, you cannot use dml_adaptive_object_relationship_list to track
>> dependency among table and other objects.
>>
>> In my understanding the feature does not use
>> dml_adaptive_object_relationship_list. If this is correct, why
>> check_object_relationship_list() is called here in case
>> dml_adaptive_global? If the feature uses
>> dml_adaptive_object_relationship_list, test cases should be included.
>>
>> diff --git a/src/utils/pool_track_table_mutation.c
>> b/src/utils/pool_track_table_mutation.c
>> new file mode 100644
>> index 000000000..9be46b28f
>> --- /dev/null
>> +++ b/src/utils/pool_track_table_mutation.c
>>
>> It seems following functions are not used anywhere. I wonder if this
>> feature actually use "query parse cache".
>>
>> pool_track_table_mutation_get_cached_parse
>> pool_track_table_mutation_cache_parse
>> pool_track_table_mutation_normalize_and_hash
>>
>> Besides the code review, I mutated one of regression tests to check
>> whether the feature co exists with in the existing memory query cache
>> feature. After attached patch applied, I ran 006.memqcache and got the
>> following result.
>>
>> cd src/test/regression
>> ./regress.sh 006
>> creating pgpool-II temporary installation ...
>> moving pgpool_setup to temporary installation path ...
>> moving watchdog_setup to temporary installation path ...
>> using pgpool-II at
>> /home/t-ishii/work/Pgpool-II/current/pgpool2/src/test/regression/temp/installed
>> *************************
>> REGRESSION MODE : install
>> Pgpool-II version : pgpool-II version 4.8devel (mitsukakeboshi)
>> Pgpool-II install path :
>> /home/t-ishii/work/Pgpool-II/current/pgpool2/src/test/regression/temp/installed
>> PostgreSQL bin : /usr/local/pgsql/bin
>> PostgreSQL Major version : 18
>> pgbench : /usr/local/pgsql/bin/pgbench
>> PostgreSQL jdbc :
>> /usr/local/pgsql/share/postgresql-9.2-1003.jdbc4.jar
>> *************************
>> testing 006.memqcache...failed.
>> out of 1 ok:0 failed:1 timeout:0
>>
>> log/006.memqcache shows:
>>
>> ../expected.txt result.txt differ: char 1, line 1
>>
>> So I checked the test script and found the error was generated by a
>> Java program test.
>>
>> java jdbctest > result.txt 2>&1
>> cmp ../expected.txt result.txt
>> if [ $? != 0 ];then
>> ./shutdownall
>> exit 1
>> fi
>>
>> In jdbctest.java:
>>
>> /*
>> * Cache test in an explicit transaction
>> */
>> conn.setAutoCommit(false);
>> // execute DML. This should prevent SELECTs from using
>> query cache in the transaction.
>> sql = "UPDATE t1 SET i = 2;";
>> pst = conn.createStatement();
>> pst.executeUpdate(sql);
>> pst.close();
>> // should not use the cache and should return "2", rather
>> than "1"
>> prest = conn.prepareStatement("SELECT * FROM t1");
>> rs = prest.executeQuery();
>>
>> The expected file (expected.txt) has "2" but the result file
>> (testdir/result.txt) was "1". This is the reason why the test
>> failed. I wonder if there's something wrong with the feature when the
>> query cache is enabled. Can you look into this?
>>
>> 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