On Mon, May 9, 2022 at 7:01 PM Euler Taveira <eu...@eulerto.com> 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.
