On Mon, Feb 20, 2023 at 03:07:37PM +0800, Julien Rouhaud wrote:
> On Mon, Feb 20, 2023 at 11:07:42AM +0530, Amit Kapila wrote:
> >
> > I think the current mechanism tries to provide more flexibility to the
> > users. OTOH, in some of the cases where users don't want to change
> > anything in the logical replication (both upstream and downstream
> > function as it is) after the upgrade then they need to do more work. I
> > think ideally there should be some option in pg_dump that allows us to
> > dump the contents of pg_subscription_rel as well, so that is easier
> > for users to continue replication after the upgrade. We can then use
> > it for binary-upgrade mode as well.
>
> Is there really a use case for dumping the content of pg_subscription_rel
> outside of pg_upgrade? I'm not particularly worried about the publisher going
> away or changing while pg_upgrade is running , but for a normal pg_dump /
> pg_restore I don't really see how anyone would actually want to resume logical
> replication from a pg_dump, especially since it's almost guaranteed that the
> node will already have consumed data from the publication that won't be in the
> dump in the first place.
>
> Are you ok with the suggested syntax above (probably with extra parens to avoid
> adding new keywords), or do you have some better suggestion? I'm a bit worried
> about adding some O(n) commands, as it can add some noticeable slow-down for
> pg_upgrade-ing logical replica, but I don't really see how to avoid that. Note
> that if we make this option available to end-users, we will have to use the
> relation name rather than its oid, which will make this option even more
> expensive when restoring due to the extra lookups.
>
> For the pg_upgrade use-case, do you see any reason to not restore the
> pg_subscription_rel by default? Maybe having an option to not restore it would
> make sense if it indeed add noticeable overhead when publications have a lot of
> tables?
Since I didn't hear any objection I worked on a POC patch with this approach.
For now when pg_dump is invoked with --binary, it will always emit extra
commands to restore the relation list. This command is only allowed when the
server is started in binary upgrade mode.
The new command is of the form
ALTER SUBSCRIPTION name ADD TABLE (relid = X, state = 'Y', lsn = 'Z/Z')
with the lsn part being optional. I'm not sure if there should be some new
regression test for that, as it would be a bit costly. Note that pg_upgrade of
a logical replica isn't covered by any regression test that I could find.
I did test it manually though, and it fixes my original problem, allowing me to
safely resume logical replication by just re-enabling it. I didn't do any
benchmarking to see how much overhead it adds.
