Re: Proposal: Recent mutated table tracking in memory

Hi Nadav,

> Hi Tatsuo,
>
> Thanks for checking the V3, sorry for missing the test issue.
>
> I reproduced the timeout locally. Found and fixed the root cause.
>
> Root cause
> ----------
>
> In CommandComplete.c, the autocommit write-tracking code was
> gated only on session_context->is_in_transaction, not on the
> cluster mode.

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?

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