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

Hayato Kuroda (Fujitsu) <kuroda.hayato@fujitsu.com>

From: "Hayato Kuroda (Fujitsu)" <kuroda.hayato@fujitsu.com>
To: 'Bharath Rupireddy' <bharath.rupireddyforpostgres@gmail.com>, "Zhijie Hou (Fujitsu)" <houzj.fnst@fujitsu.com>
Cc: Amit Kapila <amit.kapila16@gmail.com>, Dilip Kumar <dilipbalaut@gmail.com>, Peter Smith <smithpb2250@gmail.com>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>, Bruce Momjian <bruce@momjian.us>, Julien Rouhaud <rjuju123@gmail.com>, vignesh C <vignesh21@gmail.com>, Masahiko Sawada <sawada.mshk@gmail.com>
Date: 2023-09-29T11:57:51Z
Lists: pgsql-hackers
Dear Bharath,

Thanks for giving your idea!

> > I think this approach can work, but I am not sure if it's better than other
> > approaches. Mainly because it has almost the same maintaince burden as the
> > current approach, i.e. we need to verify and update the check function each
> > time we add a new WAL record type.
> 
> I think that's not a big problem if we have comments in
> replication/decode.h, access/rmgrlist.h, docs to categorize the new
> WAL records as decodable. Currently, the WAL record types adders will
> have to do certain things based on notes in comments or docs anyways.
> 
> Another idea to enforce categorizing decodability of WAL records is to
> have a new RMGR API rm_is_record_decodable or such, the RMGR
> implementers will then add respective functions returning true/false
> if a given WAL record is decodable or not:
>     void        (*rm_decode) (struct LogicalDecodingContext *ctx,
>                               struct XLogRecordBuffer *buf);
>     bool        (*rm_is_record_decodable) (uint8 type);
> } RmgrData;
> 
> PG_RMGR(RM_XLOG_ID, "XLOG", xlog_redo, xlog_desc, xlog_identify, NULL,
> NULL, NULL, xlog_is_record_decodable), then the
> xlog_is_record_decodable can look something like [1].
> 
> This approach can also enforce/help custom RMGR implementers to define
> the decodability of the WAL records.

Yeah, the approach enforces developers to check the decodability.
But the benefit seems smaller than required efforts for it because the function
would be used only by pg_upgrade. Could you tell me if you have another use case
in mind? We may able to adopt if we have...
Also, this approach cannot be backported.

Anyway, let's see how senior members say.

Best Regards,
Hayato Kuroda
FUJITSU LIMITED

Commits

  1. Fix issues in binary_upgrade_logical_slot_has_caught_up().

  2. Fix a random failure in 003_logical_slots.pl.

  3. Fix a test in 003_logical_slots.

  4. Fix uninitialized slot array access during the upgrade.

  5. Fix the test 003_logical_slots.

  6. Commit b195e6d482 forgot to update meson.build.

  7. Use shorter file names in the upgrade logical slots test.

  8. Migrate logical slots to the new node during an upgrade.

  9. Flush logical slots to disk during a shutdown checkpoint if required.

  10. Prevent possibility of panics during shutdown checkpoint.