Re: [PoC] pg_upgrade: allow to upgrade publisher node

On Mon, Sep 25, 2023 at 4:31 PM Hayato Kuroda (Fujitsu)
<kuroda.hayato@fujitsu.com> wrote:
>
> > 4.
> > +        /*
> > +         * There is a possibility that following records may be generated
> > +         * during the upgrade.
> > +         */
> > +        is_valid = is_xlog_record_type(rmid, info, RM_XLOG_ID,
> > XLOG_CHECKPOINT_SHUTDOWN) ||
> > +            is_xlog_record_type(rmid, info, RM_XLOG_ID,
> > XLOG_CHECKPOINT_ONLINE) ||
> > +            is_xlog_record_type(rmid, info, RM_XLOG_ID, XLOG_SWITCH) ||
> > +            is_xlog_record_type(rmid, info, RM_XLOG_ID,
> > XLOG_FPI_FOR_HINT) ||
> > +            is_xlog_record_type(rmid, info, RM_XLOG_ID,
> > XLOG_PARAMETER_CHANGE) ||
> > +            is_xlog_record_type(rmid, info, RM_STANDBY_ID,
> > XLOG_RUNNING_XACTS) ||
> > +            is_xlog_record_type(rmid, info, RM_HEAP2_ID,
> > XLOG_HEAP2_PRUNE);
> >
> > What if we missed to capture the WAL records that may be generated
> > during upgrade?
>
> If such records are generated before calling binary_upgrade_validate_wal_logical_end(),
> the upgrading would fail. Otherwise it would be succeeded. Anyway, we don't care
> such records because those aren't required to be replicated. The main thing we
> want to detect is that we don't miss any record generated before server shutdown.

I read this https://www.postgresql.org/message-id/20230725170319.h423jbthfohwgnf7@awork3.anarazel.de
and understand that the current patch implements the approach
suggested there - "scan the end of the WAL for records that should
have been streamed out". I think the WAL records that should have been
streamed out are all WAL record types in XXXX_decode functions except
the ones that have a no-op or an op unrelated to logical decoding. For
instance,
- for xlog_decode, if the records of type {XLOG_CHECKPOINT_ONLINE,
XLOG_PARAMETER_CHANGE, XLOG_NOOP, XLOG_NEXTOID, XLOG_SWITCH,
XLOG_BACKUP_END, XLOG_RESTORE_POINT, XLOG_FPW_CHANGE,
XLOG_FPI_FOR_HINT, XLOG_FPI, XLOG_OVERWRITE_CONTRECORD} are found
after confirmed_flush LSN, it is fine.
- for xact_decode, if the records of type {XLOG_XACT_ASSIGNMENT} are
found after confirmed_flush LSN, it is fine.
- for standby_decode, if the records of type {XLOG_STANDBY_LOCK,
XLOG_INVALIDATIONS} are found after confirmed_flush LSN, it is fine.
- for standby_decode, if the records of type {XLOG_STANDBY_LOCK,
XLOG_INVALIDATIONS} are found after confirmed_flush LSN, it is fine.
- for heap2_decode, if the records of type {XLOG_HEAP2_REWRITE,
XLOG_HEAP2_FREEZE_PAGE, XLOG_HEAP2_PRUNE, XLOG_HEAP2_VACUUM,
XLOG_HEAP2_VISIBLE, XLOG_HEAP2_LOCK_UPDATED} are found after
confirmed_flush LSN, it is fine.
- for heap_decode, if the records of type {XLOG_HEAP_LOCK} are found
after confirmed_flush LSN, it is fine.

I think all of the above WAL records are okay to be present after
cofirmed_flush LSN. If any WAL records other than the above are found
after confirmed_flush LSN, those are the one that should have been
streamed out and the pg_upgrade must complain with "The slot "foo" has
not consumed the WAL yet" for all such slots, right? But, the function
binary_upgrade_validate_wal_logical_end checks for only a handful of
the above record types. I know that the list is arrived at based on
testing, but it may happen that any of the above WAL records may be
generated and present before/during/after pg_upgrade for which
pg_upgrade failure isn't wanted.

Perhaps, a function in logical/decode.c returning the WAL record as
valid if the record type is any of the above. A note in
replication/decode.h and/or access/rmgrlist.h asking rmgr adders to
categorize the WAL record type in the new function based on its
decoding operation might help with future new WAL record type
additions.

Thoughts?

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