On Sun, Dec 18, 2022 at 12:53:31PM -0800, Andrey Borodin wrote:
> I've rewritten this part to correctly report all timeouts that did
> happen. However there's now a tricky comma-formatting code which was
> tested only manually.

I suspect this will make translation difficult.

>> > > > +     ahprintf(AH, "SET transaction_timeout = 0;\n");
>> > >
>> > > Hm - why is that the right thing to do?
>> > Because transaction_timeout has effects of statement_timeout.
>>
>> I guess it's just following precedent - but it seems a bit presumptuous
>> to just disable safety settings a DBA might have set up. That makes some
>> sense for e.g. idle_in_transaction_session_timeout, because I think
>> e.g. parallel backup can lead to a connection being idle for a bit.
> 
> I do not know. My reasoning - everywhere we turn off
> statement_timeout, we should turn off transaction_timeout too.
> But I have no strong opinion here. I left this code as is in the patch
> so far. For the same reason I did not change anything in
> pg_backup_archiver.c.

>From 8383486's commit message:

        We disable statement_timeout and lock_timeout during dump and restore,
        to prevent any global settings that might exist from breaking routine
        backups.

I imagine changing this could disrupt existing servers that depend on these
overrides during backups, although I think Andres has a good point about
disabling safety settings.  This might be a good topic for another thread.

-- 
Nathan Bossart
Amazon Web Services: https://aws.amazon.com


Reply via email to