On Monday, March 4, 2024 11:44 PM Bertrand Drouvot <bertranddrouvot.pg@gmail.com> wrote:
>
> Hi,
>
> On Mon, Mar 04, 2024 at 01:28:04PM +0000, Zhijie Hou (Fujitsu) wrote:
> > Attach the V105 patch set
>
> Thanks!
>
> Sorry I missed those during the previous review:
No problem, thanks for the comments!
>
> 1 ===
>
> Commit message: "these functions will block until"
>
> s/block/wait/ ?
>
> 2 ===
>
> + when used with logical failover slots, will block until all
>
> s/block/wait/ ?
>
> It seems those are the 2 remaining "block" that could deserve the proposed
> above change.
I prefer using 'block' here. And it seems others also suggest
to change the 'wait'[1].
>
> 3 ===
>
> + invalidated = slot->data.invalidated != RS_INVAL_NONE;
> + inactive = slot->active_pid == 0;
>
> invalidated = (slot->data.invalidated != RS_INVAL_NONE); inactive =
> (slot->active_pid == 0);
>
> instead?
>
> I think it's easier to read and it looks like this is the way it's written in other
> places (at least the few I checked).
I think the current code is consistent with other similar code in slot.c.
(grep "data.invalidated != RS_INVAL_NONE").
[1] https://www.postgresql.org/message-id/CAHut%2BPsATK8z1TEcfFE8zWoS1hagqsvaWYCgom_zYtScfwO7uQ%40mail.gmail.com
Best Regards,
Hou zj