Re: pg_recvlogical prints bogus error when interrupted

On Fri, Oct 28, 2022 at 4:41 AM Andres Freund <andres@anarazel.de> wrote:
>
> On 2022-10-24 08:15:11 +0530, Bharath Rupireddy wrote:
>
>
> > +             /* When we get SIGINT/SIGTERM, we exit */
> > +             if (ready_to_exit)
> > +             {
> > +                     /*
> > +                      * Try informing the server about our exit, but don't wait around
> > +                      * or retry on failure.
> > +                      */
> > +                     (void) PQputCopyEnd(conn, NULL);
> > +                     (void) PQflush(conn);
> > +                     time_to_abort = ready_to_exit;
>
> This doesn't strike me as great - because the ready_to_exit isn't checked in
> the loop around StreamLogicalLog(), we'll reconnect if something else causes
> StreamLogicalLog() to return.

Fixed.

> Why do we need both time_to_abort and ready_to_exit?

Intention to have ready_to_exit is to be able to distinguish between
SIGINT/SIGTERM and aborting when endpos is reached so that necessary
code is skipped/executed and proper logs are printed.

> Perhaps worth noting that
> time_to_abort is still an sig_atomic_t, but isn't modified in a signal
> handler, which seems a bit unnecessarily confusing.

time_to_abort is just a static variable, no?

+static bool    time_to_abort = false;
+static volatile sig_atomic_t ready_to_exit = false;

Please see the attached v3 patch.

--
Bharath Rupireddy
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com