From: | Peter Smith <smithpb2250(at)gmail(dot)com> |
---|---|
To: | Tomas Vondra <tomas(dot)vondra(at)enterprisedb(dot)com> |
Cc: | PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
Subject: | Re: walsender.c fileheader comment |
Date: | 2024-07-19 05:02:43 |
Message-ID: | CAHut+PsvhETkXW78vsT=xCx6SZt8jEgh-3OdKsPuenLGrUnd0A@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Hi, Thankyou for taking the time to look at this and reply.
>
> I did look at this, and while the explanation in the current comment may
> seem a bit confusing, I'm not sure the suggested changes improve the
> situation very much.
>
> This suggests the two comments somehow disagree, but it does not say in
> what exactly, so perhaps I just missed it :-(
>
> ISTM there's a bit of confusion what is meant by "stopping" state - you
> seem to be interpreting it as a general concept, where the walsender is
> requested to stop (through the signal), and starts doing stuff to exit.
> But the comments actually talk about WalSnd->state, where "stopping"
> means it needs to be set to WALSNDSTATE_STOPPING.
Yes, I interpreted the "stopping" state meaning as when the boolean
flag 'got_STOPPING' is assigned true.
>
> And we only ever switch to that state in two places - in WalSndPhysical
> and exec_replication_command. And that does not happen in regular
> logical replication (which is what "logical replication is in progress"
> refers to) - if you have a walsender just replicating DML, it will never
> see the WALSNDSTATE_STOPPING state. It will simply do the cleanup while
> still in WALSNDSTATE_STREAMING state, and then just exit.
>
> So from this point of view, the suggestion is actually wrong.
OK.
>
> To conclude, I think this probably makes the comments more confusing. If
> we want to make it clearer, I'd probably start by clarifying what the
> "stopping" state means. Also, it's a bit surprising we may not actually
> go through the "stopping" state during shutdown.
>
I agree. My interpretation of the (ambiguous) "stopping" state led me
to believe the comment was quite wrong. So, this thread was only
intended as a trivial comment fix in passing but clearly there is more
to this than I anticipated. I would be happy if someone with more
knowledge about the WALSNDSTATE_STOPPING versus got_STOPPING could
disambiguate the file header comment, but that's not me, so I have
withdrawn this from the Commitfest.
======
Kind Regards,
Peter Smith.
Fujitsu Australia
From | Date | Subject | |
---|---|---|---|
Next Message | Amit Kapila | 2024-07-19 05:15:16 | Re: Slow catchup of 2PC (twophase) transactions on replica in LR |
Previous Message | Etsuro Fujita | 2024-07-19 05:01:54 | Re: d844cd75a and postgres_fdw |