Hi!
On Wed, Jan 31, 2024 at 11:57 AM Andrey Borodin <x4mmm@yandex-team.ru> wrote:
> > On 31 Jan 2024, at 14:27, Japin Li <japinli@hotmail.com> wrote:
> >
> > LGTM.
> >
> > If there is no other objections, I'll change it to ready for committer
> > next Monday.
>
> I think we have a quorum, so I decided to go ahead and flipped status to RfC. Thanks!
I checked this patch. Generally I look good. I've slightly revised that.
I think there is one unaddressed concern by Andres Freund [1] about
the overhead of this patch by adding extra branches and function calls
in the case transaction_timeout is disabled. I tried to measure the
overhead of this patch using a pgbench script containing 20 semicolons
(20 empty statements in 20 empty transactions). I didn't manage to
find measurable overhead or change of performance profile (I used
XCode Instruments on my x86 MacBook). One thing, which I still found
possible to do is to avoid unconditional calls to
get_timeout_active(TRANSACTION_TIMEOUT). Instead I put responsibility
for disabling timeout after GUC disables the transaction_timeout
assign hook.
I removed the TODO comment from _doSetFixedOutputState(). I think
backup restore is the operation where slow commands and slow
transactions are expected, and it's natural to disable
transaction_timeout among other timeouts there. And the existing
comment clarifies that.
Also I made some grammar fixes to docs and comments.
I'm going to push this if there are no objections.
Links.
1. https://www.postgresql.org/message-id/20221206011050.s6hapukjqha35hud%40alap3.anarazel.de
------
Regards,
Alexander Korotkov