From: | vignesh C <vignesh21(at)gmail(dot)com> |
---|---|
To: | Julien Rouhaud <rjuju123(at)gmail(dot)com> |
Cc: | "Hayato Kuroda (Fujitsu)" <kuroda(dot)hayato(at)fujitsu(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> |
Subject: | Re: pg_upgrade and logical replication |
Date: | 2023-07-01 05:09:00 |
Message-ID: | CALDaNm0mTJTqs-8hHrprtMEukNVrx7_7ya1zRVUYKP1m9JsYTg@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Mon, 24 Apr 2023 at 12:52, Julien Rouhaud <rjuju123(at)gmail(dot)com> wrote:
>
> Hi,
>
> On Tue, Apr 18, 2023 at 01:40:51AM +0000, Hayato Kuroda (Fujitsu) wrote:
> >
> > I found a cfbot failure on macOS [1]. According to the log,
> > "SELECT count(*) FROM t2" was executed before synchronization was done.
> >
> > ```
> > [09:24:21.018](0.132s) not ok 18 - Table t2 should now have 3 rows on the new subscriber
> > ```
> >
> > With the patch present, wait_for_catchup() is executed after REFRESH, but
> > it may not be sufficient because it does not check pg_subscription_rel.
> > wait_for_subscription_sync() seems better for the purpose.
>
> Fixed, thanks!
>
> v5 attached with all previously mentioned fixes.
Few comments:
1) Should we document this command:
+ case ALTER_SUBSCRIPTION_ADD_TABLE:
+ {
+ if (!IsBinaryUpgrade)
+ ereport(ERROR,
+
(errcode(ERRCODE_SYNTAX_ERROR)),
+ errmsg("ALTER
SUBSCRIPTION ... ADD TABLE is not supported"));
+
+ supported_opts = SUBOPT_RELID |
SUBOPT_STATE | SUBOPT_LSN;
+ parse_subscription_options(pstate,
stmt->options,
+
supported_opts, &opts);
+
+ /* relid and state should always be provided. */
+ Assert(IsSet(opts.specified_opts,
SUBOPT_RELID));
+ Assert(IsSet(opts.specified_opts,
SUBOPT_STATE));
+
+ AddSubscriptionRelState(subid,
opts.relid, opts.state,
+
opts.lsn);
+
Should we document something like:
This command is for use by in-place upgrade utilities. Its use for
other purposes is not recommended or supported. The behavior of the
option may change in future releases without notice.
2) Similarly in pg_dump too:
@@ -431,6 +431,7 @@ main(int argc, char **argv)
{"table-and-children", required_argument, NULL, 12},
{"exclude-table-and-children", required_argument, NULL, 13},
{"exclude-table-data-and-children", required_argument,
NULL, 14},
+ {"preserve-subscription-state", no_argument,
&dopt.preserve_subscriptions, 1},
Should we document something like:
This command is for use by in-place upgrade utilities. Its use for
other purposes is not recommended or supported. The behavior of the
option may change in future releases without notice.
3) This same error is possible for ready state table but with invalid
remote_lsn, should we include this too in the error message:
+ if (is_error)
+ pg_fatal("--preserve-subscription-state is incompatible with "
+ "subscription relations in non-ready state");
+
+ check_ok();
+}
Regards,
Vignesh
From | Date | Subject | |
---|---|---|---|
Next Message | Pavel Stehule | 2023-07-01 05:13:15 | Re: Schema variables - new implementation for Postgres 15 |
Previous Message | Thomas Munro | 2023-07-01 04:28:52 | Re: [PoC] Federated Authn/z with OAUTHBEARER |