Re: Single transaction in the tablesync worker?

On Thu, Jan 7, 2021 at 3:20 PM Amit Kapila <amit.kapila16@gmail.com> wrote:

> > BTW, I have analyzed whether we need any modifications to
> > pg_dump/restore for this patch as this changes the state of one of the
> > fields in the system table and concluded that we don't need any
> > change. For subscriptions, we don't dump any of the information from
> > pg_subscription_rel, rather we just dump subscriptions with the
> > connect option as false which means users need to enable the
> > subscription and refresh publication after restore. I have checked
> > this in the code and tested it as well. The related information is
> > present in pg_dump doc page [1], see from "When dumping logical
> > replication subscriptions ....".
> >
>
> I have further analyzed that we don't need to do anything w.r.t
> pg_upgrade as well because it uses pg_dump/pg_dumpall to dump the
> schema info of the old cluster and then restore it to the new cluster.
> And, we know that pg_dump ignores the info in pg_subscription_rel, so
> we don't need to change anything as our changes are specific to the
> state of one of the columns in pg_subscription_rel. I have not tested
> this but we should test it by having some relations in not_ready state
> and then allow the old cluster (<=PG13) to be upgraded to new (pg14)
> both with and without this patch and see if there is any change in
> behavior.

I have tested this scenario, stopped a server running PG_13 when
subscription table sync was in progress.
One of the tables in pg_subscription_rel was still in 'd' state (DATASYNC)

postgres=# select * from pg_subscription_rel;
 srsubid | srrelid | srsubstate |  srsublsn
---------+---------+------------+------------
   16424 |   16384 | d          |
   16424 |   16390 | r          | 0/247A63D8
   16424 |   16395 | r          | 0/247A6410
   16424 |   16387 | r          | 0/247A6448
(4 rows)

then initiated the pg_upgrade to PG_14 with the patch and without the patch:
I see that the subscription exists but is not enabled:

postgres=# select * from pg_subscription;
  oid  | subdbid | subname | subowner | subenabled | subbinary |
substream |               subconninfo                | subslotname |
subsynccommit | subpublications
-------+---------+---------+----------+------------+-----------+-----------+------------------------------------------+-------------+---------------+-----------------
 16407 |   16401 | tap_sub |       10 | f          | f         | f
    | host=localhost port=6972 dbname=postgres | tap_sub     | off
      | {tap_pub}
(1 row)

and looking at the pg_subscription_rel:

postgres=# select * from pg_subscription_rel;
 srsubid | srrelid | srsubstate | srsublsn
---------+---------+------------+----------
(0 rows)

As can be seen, none of the data in the pg_subscription_rel has been
copied over. Same behaviour is seen with the patch and without the
patch.

regards,
Ajin Cherian
Fujitsu Australia