From: | "osumi(dot)takamichi(at)fujitsu(dot)com" <osumi(dot)takamichi(at)fujitsu(dot)com> |
---|---|
To: | 'Amit Kapila' <amit(dot)kapila16(at)gmail(dot)com>, Peter Smith <smithpb2250(at)gmail(dot)com> |
Cc: | Ajin Cherian <itsajin(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
Subject: | RE: Single transaction in the tablesync worker? |
Date: | 2021-02-05 07:06:15 |
Message-ID: | OSBPR01MB4888B09FB5D4D2DED6CBDB4EEDB29@OSBPR01MB4888.jpnprd01.prod.outlook.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Hello
On Friday, February 5, 2021 2:23 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
> On Fri, Feb 5, 2021 at 7:09 AM Peter Smith <smithpb2250(at)gmail(dot)com> wrote:
> >
> > On Thu, Feb 4, 2021 at 8:33 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
> wrote:
> > >
> > ...
> >
> > > Thanks. I have fixed one of the issues reported by me earlier [1]
> > > wherein the tablesync worker can repeatedly fail if after dropping
> > > the slot there is an error while updating the SYNCDONE state in the
> > > database. I have moved the drop of the slot just before commit of
> > > the transaction where we are marking the state as SYNCDONE.
> > > Additionally, I have removed unnecessary includes in tablesync.c,
> > > updated the docs for Alter Subscription, and updated the comments at
> > > various places in the patch. I have also updated the commit message this
> time.
> > >
> >
> > Below are my feedback comments for V17 (nothing functional)
> >
> > ~~
> >
> > 1.
> > V27 Commit message:
> > For the initial table data synchronization in logical replication, we
> > use a single transaction to copy the entire table and then
> > synchronizes the position in the stream with the main apply worker.
> >
> > Typo:
> > "synchronizes" -> "synchronize"
> >
>
> Fixed and added a note about Alter Sub .. Refresh .. command can't be
> executed in the transaction block.
Thank you for the updates.
We need to add some tests to prove the new checks of AlterSubscription() work.
I chose TAP tests as we need to set connect = true for the subscription.
When it can contribute to the development, please utilize this.
I used v28 to check my patch and works as we expect.
Best Regards,
Takamichi Osumi
Attachment | Content-Type | Size |
---|---|---|
AlterSubscription_with_refresh_tests.patch | application/octet-stream | 2.5 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Ronan Dunklau | 2021-02-05 07:22:17 | Re: Preserve attstattarget on REINDEX CONCURRENTLY |
Previous Message | Amit Kapila | 2021-02-05 07:01:53 | Re: pg_replication_origin_drop API potential race condition |