Re: Synchronizing slots from primary to standby

On Tue, Dec 19, 2023 at 11:37 AM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>
> On Tue, Dec 19, 2023 at 4:51 AM Peter Smith <smithpb2250(at)gmail(dot)com> wrote:
> >
> >
> > ======
> > doc/src/sgml/system-views.sgml
> >
> > 3.
> > + <para>
> > + The hot standby can have any of these sync_state values for the slots but
> > + on a hot standby, the slots with state 'r' and 'i' can neither be used
> > + for logical decoding nor dropped by the user.
> > + The sync_state has no meaning on the primary server; the primary
> > + sync_state value is default 'n' for all slots but may (if leftover
> > + from a promoted standby) also be 'r'.
> > + </para></entry>
> >
> > I still feel we are exposing too much useless information about the
> > primary server values.
> >
> > Isn't it sufficient to just say "The sync_state values have no meaning
> > on a primary server.", and not bother to mention what those
> > meaningless values might be -- e.g. if they are meaningless then who
> > cares what they are or how they got there?
> >
>
> I feel it would be good to mention somewhere that primary can have
> slots in 'r' state, if not here, some other place.
>
> >
> > 7.
> > +/*
> > + * Exit the slot sync worker with given exit-code.
> > + */
> > +static void
> > +slotsync_worker_exit(const char *msg, int code)
> > +{
> > + ereport(LOG, errmsg("%s", msg));
> > + proc_exit(code);
> > +}
> >
> > This could be written differently (don't pass the exit code, instead
> > pass a bool) like:
> >
> > static void
> > slotsync_worker_exit(const char *msg, bool restart_worker)
> >
> > By doing it this way, you can keep the special exit code values (0,1)
> > within this function where you can comment all about them instead of
> > having scattered comments about exit codes in the callers.
> >
> > SUGGESTION
> > ereport(LOG, errmsg("%s", msg));
> > /* <some big comment here about how the code causes the worker to
> > restart or not> */
> > proc_exit(restart_worker ? 1 : 0);
> >
>
> Hmm, I don't see the need for this function in the first place. We can
> use proc_exit in the two callers directly.
>
> --
> With Regards,
> Amit Kapila.

PFA v50 patch-set which addresses comments for v48-0002 and v49-0002
given in [1], [2] and [3].

TODO:
--Fix CFBot failure.
--Work on correctness of test to merge patch003 to patch002

[1]: https://www.postgresql.org/message-id/CAA4eK1Ko-EBBDkea2R8V8PeveGg10PBswCF7JQdnRu%2BMJP%2BYBQ%40mail.gmail.com
[2]: https://www.postgresql.org/message-id/CAHut%2BPsyZQZ1A4XcKw-D%3DvcTg16pN9Dw0PzE8W_X7Yz_bv00rQ%40mail.gmail.com
[3]: https://www.postgresql.org/message-id/CAHut%2BPv86wBZiyOLHxycd8Yj9%3Dk5kzVa1x7Gbp%2B%3Dc1VGT9TG2w%40mail.gmail.com

thanks
Shveta