Date: Sat, 23 May 2026 20:18:16 +0900 (JST)
Message-Id: <20260523.201816.2108820434774242495.ishii@postgresql.org>
To: nadav@tailorbrands.com
Cc: pgpool-hackers@lists.postgresql.org
Subject: Re: Proposal: Recent mutated table tracking in memory
From: Tatsuo Ishii <ishii@postgresql.org>
In-Reply-To: <20260521.185044.504532841722447976.ishii@postgresql.org>
References: <20260520.132810.1024114140609315185.ishii@postgresql.org>
	<CACeKOO2eUrfo_UDMFSEd=2y8zj8y93m38EzRCpg1HuizYBf3wA@mail.gmail.com>
	<20260521.185044.504532841722447976.ishii@postgresql.org>
Mime-Version: 1.0
Content-Type: Multipart/Mixed;
 boundary="--Next_Part(Sat_May_23_20_18_16_2026_327)--"
Content-Transfer-Encoding: 7bit
Archived-At: 
 <https://www.postgresql.org/message-id/20260523.201816.2108820434774242495.ishii%40postgresql.org>
Precedence: bulk

----Next_Part(Sat_May_23_20_18_16_2026_327)--
Content-Type: Text/Plain; charset=iso-2022-jp
Content-Transfer-Encoding: 7bit

> I think you are talking about the logic to judge whether we are in an
> explicite transaction or not here. Current dml_adaptive checks
> supplied query is a transaction starting command like BEGIN. IMO this
> is fundamentaly wrong because the command may fail by various reasons.
> The correct way is checking transaction state by using TSTATE
> macro. Note that the macro can only be used at leat one ready for
> query response returned from backend (simple query protocol case), or
> command complete response is returned from backend (extended query
> protocol case).
> 
>> In native replication and snapshot isolation modes,
>> dml_adaptive() is never called (it lives inside
>> where_to_send_main_replica), so is_in_transaction is never set
>> to true even inside an explicit BEGIN/COMMIT block.  That meant
>> every DML in those modes was treated as autocommit by the
>> write-tracking code, triggering
>> pool_track_table_mutation_get_database_oid() ― which does a
>> relcache do_query ― while a transaction was actually in flight
>> on the backend connection.  The do_query conflicts with the
>> in-flight transaction and hangs the session.
> 
> Assuming "a transaction was actually in flight" means a transaction
> was open (explicit transaction), not really. do_query can be called
> inside or outside of an explicit transaction.
> 
> Anyway, I found dml_adaptive is completely broken (it brings wrong
> results if query cache enabled). Unless there are users for the
> feature, maybe we should remove dml_adaptive entirely?

It appears that other options of disable_load_balance_on_write are all
broken too, except "transaction". I don't want to discard all of them,
so I come up with attached patch.

The query cache relies on is_writing_transaction of session context to
judge whether cache can be safely used. However,
disable_load_balance_on_write overrides it to true when it should not,
and vice versa for its own purpose. To fix this, a new session context
variable "really_writing_transaction" is introduced. It is almost same
as existing writing_transaction, but it faithfully tracks whether a
writing query appears in an explicit transaction. The query cache uses
it instead of writing_transaction variable.

Currently, master branch is broken because of commit 2ae004a48.  If
you want to try the patch, I recommend to checkout 48e1d6d3c, then
apply the patch.

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

----Next_Part(Sat_May_23_20_18_16_2026_327)--
Content-Type: Text/X-Patch; charset=us-ascii
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
 filename="v1-0001-Fix-disable_load_balance_on_write-and-query-cache.patch"