public inbox for [email protected]  
help / color / mirror / Atom feed
From: Amit Kapila <[email protected]>
To: Euler Taveira <[email protected]>
Cc: [email protected] <[email protected]>
Cc: Masahiko Sawada <[email protected]>
Cc: [email protected] <[email protected]>
Cc: [email protected] <[email protected]>
Cc: Peter Smith <[email protected]>
Cc: Fabrice Chapuis <[email protected]>
Cc: Simon Riggs <[email protected]>
Cc: Petr Jelinek <[email protected]>
Cc: [email protected] <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Cc: Ajin Cherian <[email protected]>
Subject: Re: Logical replication timeout problem
Date: Tue, 10 May 2022 08:52:38 +0530
Message-ID: <CAA4eK1LM5e_gKvmAwYEgpQY=8siboEL53HgDTwtsR612nMA2VA@mail.gmail.com> (raw)
In-Reply-To: <[email protected]>
References: <OS3PR01MB6275982DA7BBA25DE88F6E839EF59@OS3PR01MB6275.jpnprd01.prod.outlook.com>
	<CAD21AoDWs2Ru9Zc_jzjrA4k4JUtUDN2_9GktbuAzwHwdJY2myg@mail.gmail.com>
	<OS0PR01MB5716F30A9F73742BC8EA95A594FD9@OS0PR01MB5716.jpnprd01.prod.outlook.com>
	<CAD21AoD7P9UU4nMgrotD_7LDbzpTpKugpwbjq5VUAbriqzVOSg@mail.gmail.com>
	<CAA4eK1JBT_ZKY5xSG_cwD7EOb26GrK2_CS1YPBgxE7rc69Y3jg@mail.gmail.com>
	<CAD21AoB4km=SgT0DAJOoKYicVNqvWZMtuQ7D9d0RFq20NK_EEw@mail.gmail.com>
	<CAA4eK1JuhAuG-SzxCXi5YMhpKvGT_-boJT61e7Z14qfp6AwG6g@mail.gmail.com>
	<CAD21AoAB_K2ZA_=Ja4COLM5-dJb2O_7nr1q1TmapSWSxdnh5KA@mail.gmail.com>
	<OS3PR01MB62754AA446A2FB5B72816C529EC59@OS3PR01MB6275.jpnprd01.prod.outlook.com>
	<CAA4eK1LoVa8rJ0nD3kyDMEc4ZmGWjjwDk+twSiz81ffMua5iXQ@mail.gmail.com>
	<[email protected]>

On Mon, May 9, 2022 at 7:01 PM Euler Taveira <[email protected]> wrote:
>
> On Mon, May 9, 2022, at 3:47 AM, Amit Kapila wrote:
>
> Thanks. The patch LGTM. I'll push and back-patch this after the
> current minor release is done unless there are more comments related
> to this work.
>
> Looks sane to me. (I only tested the HEAD version)
>
> +   bool        end_xact = ctx->end_xact;
>
> Do you really need a new variable here? It has the same name and the new one
> isn't changed during the execution.
>

I think both ways should be okay. I thought the proposed way is okay
because it is used in more than one place and is probably slightly
easier to follow by having a separate variable.

> Does this issue deserve a test? A small wal_receiver_timeout. Although, I'm not
> sure how stable the test will be.
>

Yes, the main part is how to write a stable test because estimating
how many changes are enough for the configured wal_receiver_timeout to
pass on all the buildfarm machines is tricky. Also, I am not sure how
important is to test this behavior because based on this theory we
should have tests for all kinds of timeouts.

-- 
With Regards,
Amit Kapila.





view thread (61+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
  Subject: Re: Logical replication timeout problem
  In-Reply-To: <CAA4eK1LM5e_gKvmAwYEgpQY=8siboEL53HgDTwtsR612nMA2VA@mail.gmail.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox