On Thu, Feb 25, 2021 at 1:52 PM Amit Kapila wrote:
>
> On Wed, Feb 24, 2021 at 5:55 PM Amit Kapila wrote:
> >
> > On Wed, Feb 24, 2021 at 12:47 PM Masahiko Sawada
> > wrote:
> > >
> > > On Fri, Feb 12, 2021 at 2:49 PM Amit Kapila
> > > wrote:
> > > >
> > > >
> > > > Thanks, I have pushed the
On Wed, Feb 24, 2021 at 5:55 PM Amit Kapila wrote:
>
> On Wed, Feb 24, 2021 at 12:47 PM Masahiko Sawada
> wrote:
> >
> > On Fri, Feb 12, 2021 at 2:49 PM Amit Kapila wrote:
> > >
> > >
> > > Thanks, I have pushed the fix and the latest run of 'thorntail' has
> > > passed.
> >
> > I got the
On Wed, Feb 24, 2021 at 12:47 PM Masahiko Sawada wrote:
>
> On Fri, Feb 12, 2021 at 2:49 PM Amit Kapila wrote:
> >
> >
> > Thanks, I have pushed the fix and the latest run of 'thorntail' has passed.
>
> I got the following WARNING message from a logical replication apply worker:
>
> WARNING:
On Fri, Feb 12, 2021 at 2:49 PM Amit Kapila wrote:
>
> On Fri, Feb 12, 2021 at 10:08 AM Ajin Cherian wrote:
> >
> > On Fri, Feb 12, 2021 at 2:46 PM Amit Kapila wrote:
> >
> > >
> > > Thanks, I have pushed the patch but getting one failure:
> > >
On Fri, Feb 12, 2021 at 10:08 AM Ajin Cherian wrote:
>
> On Fri, Feb 12, 2021 at 2:46 PM Amit Kapila wrote:
>
> >
> > Thanks, I have pushed the patch but getting one failure:
> > https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=thorntail=2021-02-12%2002%3A28%3A12
> >
> > The reason seems
On Fri, Feb 12, 2021 at 2:46 PM Amit Kapila wrote:
>
> Thanks, I have pushed the patch but getting one failure:
> https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=thorntail=2021-02-12%2002%3A28%3A12
>
> The reason seems to be that we are trying to connect and
> max_wal_senders is set to
On Fri, Feb 12, 2021 at 7:18 AM Ajin Cherian wrote:
>
> On Thu, Feb 11, 2021 at 10:38 PM Amit Kapila wrote:
>
> > Okay, attached an updated patch with only that change.
>
> I ran Erik's test suite [1] on this patch overnight and found no
> errors. No more comments from me. The patch looks good.
On Thu, Feb 11, 2021 at 10:38 PM Amit Kapila wrote:
> Okay, attached an updated patch with only that change.
I ran Erik's test suite [1] on this patch overnight and found no
errors. No more comments from me. The patch looks good.
regards,
Ajin Cherian
Fujitsu Australia
[1]-
On Thu, Feb 11, 2021 at 3:32 PM Petr Jelinek
wrote:
>
> On 11 Feb 2021, at 10:56, Amit Kapila wrote:
> >
> >> Well, I was thinking it could be NOTICE or LOG to be honest, WARNING seems
> >> unnecessarily scary for those usecases to me.
> >>
> >
> > I am fine with LOG and will make that change.
On 11 Feb 2021, at 10:56, Amit Kapila wrote:
>
> On Thu, Feb 11, 2021 at 3:20 PM Petr Jelinek
> wrote:
>>
>> On 11 Feb 2021, at 10:42, Amit Kapila wrote:
>>>
>>> On Thu, Feb 11, 2021 at 1:51 PM Petr Jelinek
>>> wrote:
On 10 Feb 2021, at 06:32, Amit Kapila wrote:
>
> On
On Thu, Feb 11, 2021 at 3:20 PM Petr Jelinek
wrote:
>
> On 11 Feb 2021, at 10:42, Amit Kapila wrote:
> >
> > On Thu, Feb 11, 2021 at 1:51 PM Petr Jelinek
> > wrote:
> >>
> >> On 10 Feb 2021, at 06:32, Amit Kapila wrote:
> >>>
> >>> On Wed, Feb 10, 2021 at 7:41 AM Peter Smith wrote:
>
>
On 11 Feb 2021, at 10:42, Amit Kapila wrote:
>
> On Thu, Feb 11, 2021 at 1:51 PM Petr Jelinek
> wrote:
>>
>> On 10 Feb 2021, at 06:32, Amit Kapila wrote:
>>>
>>> On Wed, Feb 10, 2021 at 7:41 AM Peter Smith wrote:
On Tue, Feb 9, 2021 at 10:38 AM Peter Smith wrote:
>
On Thu, Feb 11, 2021 at 1:51 PM Petr Jelinek
wrote:
>
> On 10 Feb 2021, at 06:32, Amit Kapila wrote:
> >
> > On Wed, Feb 10, 2021 at 7:41 AM Peter Smith wrote:
> >>
> >> On Tue, Feb 9, 2021 at 10:38 AM Peter Smith wrote:
> >>>
> >>
> >> PSA v2 of this WalRcvExceResult patch (it is same as v1
On 10 Feb 2021, at 06:32, Amit Kapila wrote:
>
> On Wed, Feb 10, 2021 at 7:41 AM Peter Smith wrote:
>>
>> On Tue, Feb 9, 2021 at 10:38 AM Peter Smith wrote:
>>>
>>
>> PSA v2 of this WalRcvExceResult patch (it is same as v1 but includes
>> some PG doc updates).
>> This applies OK on top of
On Wed, Feb 10, 2021 at 7:41 AM Peter Smith wrote:
>
> On Tue, Feb 9, 2021 at 10:38 AM Peter Smith wrote:
> >
>
> PSA v2 of this WalRcvExceResult patch (it is same as v1 but includes
> some PG doc updates).
> This applies OK on top of v30 of the main patch.
>
Thanks, I have integrated these
On Tue, Feb 9, 2021 at 10:38 AM Peter Smith wrote:
> PSA an alternative patch. This one adds a new member to
> WalRcvExecResult and so is able to detect the "slot does not exist"
> error. This patch also applies on top of V28, if you want it.
Did some testing with this patch on top of v29. I
On Tue, Feb 9, 2021 at 10:38 AM Peter Smith wrote:
>
> On Mon, Feb 8, 2021 at 11:42 AM Peter Smith wrote:
> >
> > On Sun, Feb 7, 2021 at 2:38 PM Peter Smith wrote:
> > >
> > > On Sat, Feb 6, 2021 at 2:10 AM Petr Jelinek
> > > wrote:
> > > >
> > > > Hi,
> > > >
> > > > Some minor comments about
On Tue, Feb 9, 2021 at 8:32 PM Amit Kapila wrote:
>
> On Tue, Feb 9, 2021 at 12:02 PM Peter Smith wrote:
> >
> > Here are my feedback comments for the V29 patch.
> >
>
> Thanks.
>
> >
> > 3.
> > Previously the tablesync origin name format was encapsulated in a
> > common function. IMO it was
On Tue, Feb 9, 2021 at 12:02 PM Peter Smith wrote:
>
> Here are my feedback comments for the V29 patch.
>
>
>
> FILE: logical-replication.sgml
>
> +slots have generated names:
> pg_%u_sync_%u_%llu
> +(parameters: Subscription oid,
> +Table relid, system
> identifiersysid)
> +
On Tue, Feb 9, 2021 at 1:37 PM Peter Smith wrote:
>
> Looking at the V29 style tablesync slot names now they appear like this:
>
> WARNING: could not drop tablesync replication slot
> "pg_16397_sync_16389_6927117142022745645"
> That is in the order subid + relid + sysid
>
> Now that I see it in
On Tue, Feb 9, 2021 at 12:02 PM Peter Smith wrote:
>
> Here are my feedback comments for the V29 patch.
>
Thanks.
>
> 3.
> Previously the tablesync origin name format was encapsulated in a
> common function. IMO it was cleaner/safer how it was before, instead
> of the same "pg_%u_%u" cut/paste
When looking at the DropSubscription code I noticed that there is a
small difference between the HEAD code and the V29 code when slot_name
= NONE.
HEAD does
--
if (!slotname)
{
table_close(rel, NoLock);
return;
}
--
V29 does
--
if (!slotname)
Looking at the V29 style tablesync slot names now they appear like this:
WARNING: could not drop tablesync replication slot
"pg_16397_sync_16389_6927117142022745645"
That is in the order subid + relid + sysid
Now that I see it in a message it seems a bit strange with the sysid
just tacked onto
More V29 Feedback
FILE: alter_subscription.sgml
8.
+
+ Commands ALTER SUBSCRIPTION ... REFRESH .. and
+ ALTER SUBSCRIPTION ... SET PUBLICATION .. with refresh
+ option as true cannot be executed inside a transaction block.
+
My guess is those two lots of double dots ("..") were
Here are my feedback comments for the V29 patch.
FILE: logical-replication.sgml
+slots have generated names:
pg_%u_sync_%u_%llu
+(parameters: Subscription oid,
+Table relid, system
identifiersysid)
+
1.
There is a missing space before the sysid parameter.
=
FILE:
On Mon, Feb 8, 2021 8:04 PM Amit Kapila wrote:
> On Mon, Feb 8, 2021 at 12:22 PM osumi.takami...@fujitsu.com
> wrote:
> > On Monday, February 8, 2021 1:44 PM osumi.takami...@fujitsu.com
> >
> > > On Mon, Feb 8, 2021 11:36 AM Peter Smith
> > > wrote:
> > > > 2. For the 004 test case I know the
On Mon, Feb 8, 2021 at 11:42 AM Peter Smith wrote:
>
> On Sun, Feb 7, 2021 at 2:38 PM Peter Smith wrote:
> >
> > On Sat, Feb 6, 2021 at 2:10 AM Petr Jelinek
> > wrote:
> > >
> > > Hi,
> > >
> > > Some minor comments about code:
> > >
> > > > + else if (res->status == WALRCV_ERROR &&
On Mon, Feb 8, 2021 at 12:22 PM osumi.takami...@fujitsu.com
wrote:
>
> On Monday, February 8, 2021 1:44 PM osumi.takami...@fujitsu.com
>
> > On Mon, Feb 8, 2021 11:36 AM Peter Smith
> > wrote:
> > > 2. For the 004 test case I know the test is needing some PK constraint
> > > violation # Check
On Fri, Feb 5, 2021 at 8:40 PM Petr Jelinek
wrote:
>
> Hi,
>
> We had a bit high-level discussion about this patches with Amit
> off-list, so I decided to also take a look at the actual code.
>
Thanks for the discussion and a follow-up review.
> My main concern originally was the potential for
On Monday, February 8, 2021 1:44 PM osumi.takami...@fujitsu.com
> On Mon, Feb 8, 2021 12:40 PM Amit Kapila
> wrote:
> > On Mon, Feb 8, 2021 at 8:06 AM Peter Smith
> > wrote:
> > >
> > > On Sat, Feb 6, 2021 at 6:30 PM osumi.takami...@fujitsu.com
> > > wrote:
> > > >
> > > > > I have another
Hello
On Mon, Feb 8, 2021 12:40 PM Amit Kapila wrote:
> On Mon, Feb 8, 2021 at 8:06 AM Peter Smith
> wrote:
> >
> > On Sat, Feb 6, 2021 at 6:30 PM osumi.takami...@fujitsu.com
> > wrote:
> > >
> > > > I have another idea for a test case: What if we write a test such
> > > > that it fails PK
On Mon, Feb 8, 2021 at 8:06 AM Peter Smith wrote:
>
> On Sat, Feb 6, 2021 at 6:30 PM osumi.takami...@fujitsu.com
> wrote:
> >
> > > I have another idea for a test case: What if we write a test such that it
> > > fails PK
> > > violation on copy and then drop the subscription. Then check there
On Sat, Feb 6, 2021 at 6:30 PM osumi.takami...@fujitsu.com
wrote:
>
> Hi
>
>
> On Friday, February 5, 2021 5:51 PM Amit Kapila
> wrote:
> > On Fri, Feb 5, 2021 at 12:36 PM osumi.takami...@fujitsu.com
> > wrote:
> > >
> > > We need to add some tests to prove the new checks of
On Sun, Feb 7, 2021 at 2:38 PM Peter Smith wrote:
>
> On Sat, Feb 6, 2021 at 2:10 AM Petr Jelinek
> wrote:
> >
> > Hi,
> >
> > Some minor comments about code:
> >
> > > + else if (res->status == WALRCV_ERROR && missing_ok)
> > > + {
> > > + /* WARNING.
On Sat, Feb 6, 2021 at 2:10 AM Petr Jelinek
wrote:
>
> Hi,
>
> Some minor comments about code:
>
> > + else if (res->status == WALRCV_ERROR && missing_ok)
> > + {
> > + /* WARNING. Error, but missing_ok = true. */
> > +
On 06/02/2021 06:07, Amit Kapila wrote:
On Sat, Feb 6, 2021 at 6:22 AM Peter Smith wrote:
On Sat, Feb 6, 2021 at 2:10 AM Petr Jelinek
wrote:
+ReplicationSlotNameForTablesync(Oid suboid, Oid relid, char
syncslotname[NAMEDATALEN])
+{
+ if (syncslotname)
+
Hi
On Friday, February 5, 2021 5:51 PM Amit Kapila wrote:
> On Fri, Feb 5, 2021 at 12:36 PM osumi.takami...@fujitsu.com
> wrote:
> >
> > 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.
On Sat, Feb 6, 2021 at 6:22 AM Peter Smith wrote:
>
> On Sat, Feb 6, 2021 at 2:10 AM Petr Jelinek
> wrote:
> >
> > > +ReplicationSlotNameForTablesync(Oid suboid, Oid relid, char
> > > syncslotname[NAMEDATALEN])
> > > +{
> > > + if (syncslotname)
> > > + sprintf(syncslotname,
On Sat, Feb 6, 2021 at 2:10 AM Petr Jelinek
wrote:
>
> > +ReplicationSlotNameForTablesync(Oid suboid, Oid relid, char
> > syncslotname[NAMEDATALEN])
> > +{
> > + if (syncslotname)
> > + sprintf(syncslotname, "pg_%u_sync_%u", suboid, relid);
> > + else
> > +
Hi,
We had a bit high-level discussion about this patches with Amit
off-list, so I decided to also take a look at the actual code.
My main concern originally was the potential for left-over slots on
publisher, but I think the state now is relatively okay, with couple of
corner cases that
I did some basic cross-version testing, publisher on PG13 and
subscriber on PG14 and publisher on PG14 and subscriber on PG13.
Did some basic operations, CREATE, ALTER and STOP subscriptions and it
seemed to work fine, no errors.
regards,
Ajin Cherian
Fujitsu Australia.
On Fri, Feb 5, 2021 at 12:36 PM osumi.takami...@fujitsu.com
wrote:
>
> 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
Hello
On Friday, February 5, 2021 2:23 PM Amit Kapila
> On Fri, Feb 5, 2021 at 7:09 AM Peter Smith wrote:
> >
> > On Thu, Feb 4, 2021 at 8:33 PM Amit Kapila
> wrote:
> > >
> > ...
> >
> > > Thanks. I have fixed one of the issues reported by me earlier [1]
> > > wherein the tablesync worker
On Fri, Feb 5, 2021 at 7:09 AM Peter Smith wrote:
>
> On Thu, Feb 4, 2021 at 8:33 PM Amit Kapila 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
On Thu, Feb 4, 2021 at 8:33 PM Amit Kapila 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
On Thu, Feb 4, 2021 at 9:55 AM Ajin Cherian wrote:
>
> On Wed, Feb 3, 2021 at 11:38 PM Amit Kapila wrote:
>
> > Thanks for the report. The problem here was that the error occurred
> > when we were trying to copy the large data. Now, before fetching the
> > entire data we issued a rollback that
On Wed, Feb 3, 2021 at 11:38 PM Amit Kapila wrote:
> Thanks for the report. The problem here was that the error occurred
> when we were trying to copy the large data. Now, before fetching the
> entire data we issued a rollback that led to this problem. I think the
> alternative here could be to
On Wed, Feb 3, 2021 at 1:28 PM Ajin Cherian wrote:
>
> On Tue, Feb 2, 2021 at 9:03 PM Ajin Cherian wrote:
>
> > I am sorry, my above steps were not correct. I think the reason for
> > the failure I was seeing were some other steps I did prior to this. I
> > will recreate this and update you with
On Wed, Feb 3, 2021 at 2:51 PM Peter Smith wrote:
>
> On Wed, Feb 3, 2021 at 1:34 PM Amit Kapila wrote:
> >
> > On Wed, Feb 3, 2021 at 6:38 AM Peter Smith wrote:
> > >
> > > On Wed, Feb 3, 2021 at 12:26 AM Amit Kapila
> > > wrote:
> > > >
> > > > On Tue, Feb 2, 2021 at 3:31 PM Peter Smith
>
On Tue, Feb 2, 2021 at 9:03 PM Ajin Cherian wrote:
> I am sorry, my above steps were not correct. I think the reason for
> the failure I was seeing were some other steps I did prior to this. I
> will recreate this and update you with the appropriate steps.
The correct steps are as follows:
On Wed, Feb 3, 2021 at 1:34 PM Amit Kapila wrote:
>
> On Wed, Feb 3, 2021 at 6:38 AM Peter Smith wrote:
> >
> > On Wed, Feb 3, 2021 at 12:26 AM Amit Kapila wrote:
> > >
> > > On Tue, Feb 2, 2021 at 3:31 PM Peter Smith wrote:
> > > >
> > > > After seeing Ajin's test [ac0202] which did a DROP
On Wed, Feb 3, 2021 at 6:38 AM Peter Smith wrote:
>
> On Wed, Feb 3, 2021 at 12:26 AM Amit Kapila wrote:
> >
> > On Tue, Feb 2, 2021 at 3:31 PM Peter Smith wrote:
> > >
> > > After seeing Ajin's test [ac0202] which did a DROP TABLE, I have also
> > > tried a simple test where I do a DROP TABLE
On Wed, Feb 3, 2021 at 12:24 AM Amit Kapila wrote:
> The problem here is that we are allowing to drop the table when table
> synchronization is still in progress and then we don't have any way to
> know the corresponding slot or origin. I think we can try to drop the
> slot and origin as well
On Wed, Feb 3, 2021 at 12:26 AM Amit Kapila wrote:
>
> On Tue, Feb 2, 2021 at 3:31 PM Peter Smith wrote:
> >
> > After seeing Ajin's test [ac0202] which did a DROP TABLE, I have also
> > tried a simple test where I do a DROP TABLE with very bad timing for
> > the tablesync worker. It seems that
On Tue, Feb 2, 2021 at 3:31 PM Peter Smith wrote:
>
> After seeing Ajin's test [ac0202] which did a DROP TABLE, I have also
> tried a simple test where I do a DROP TABLE with very bad timing for
> the tablesync worker. It seems that doing this can cause the sync
> worker's
On Tue, Feb 2, 2021 at 11:35 AM Ajin Cherian wrote:
>
> Another failure I see in my testing
>
The problem here is that we are allowing to drop the table when table
synchronization is still in progress and then we don't have any way to
know the corresponding slot or origin. I think we can try to
On Tue, Feb 2, 2021 at 7:40 PM Amit Kapila wrote:
>
> On Tue, Feb 2, 2021 at 10:34 AM Ajin Cherian wrote:
> >
> > On Mon, Feb 1, 2021 at 11:26 PM Amit Kapila wrote:
> > > I have updated the patch to display WARNING for each of the tablesync
> > > slots during DropSubscription. As discussed, I
After seeing Ajin's test [ac0202] which did a DROP TABLE, I have also
tried a simple test where I do a DROP TABLE with very bad timing for
the tablesync worker. It seems that doing this can cause the sync
worker's MyLogicalRepWorker->relid to become invalid.
In my test this caused a stack trace
On Tue, Feb 2, 2021 at 10:34 AM Ajin Cherian wrote:
>
> On Mon, Feb 1, 2021 at 11:26 PM Amit Kapila wrote:
> > I have updated the patch to display WARNING for each of the tablesync
> > slots during DropSubscription. As discussed, I have moved the drop
> > slot related code towards the end in
On Mon, Feb 1, 2021 at 11:26 PM Amit Kapila wrote:
>
> I have updated the patch to display WARNING for each of the tablesync
> slots during DropSubscription. As discussed, I have moved the drop
> slot related code towards the end in AlterSubscription_refresh. Apart
> from this, I have fixed one
Another failure I see in my testing
On publisher create a big enough table:
publisher:
postgres=# CREATE TABLE tab_rep (a int primary key);CREATE TABLE
postgres=# INSERT INTO tab_rep SELECT generate_series(1,100);
INSERT 0 100
postgres=# CREATE PUBLICATION tap_pub FOR ALL TABLES;
CREATE
On Mon, Feb 1, 2021 at 11:26 PM Amit Kapila wrote:
> I have updated the patch to display WARNING for each of the tablesync
> slots during DropSubscription. As discussed, I have moved the drop
> slot related code towards the end in AlterSubscription_refresh. Apart
> from this, I have fixed one
On Tue, Feb 2, 2021 at 8:29 AM Peter Smith wrote:
>
> On Mon, Feb 1, 2021 at 11:26 PM Amit Kapila wrote:
>
> > I have updated the patch to display WARNING for each of the tablesync
> > slots during DropSubscription. As discussed, I have moved the drop
> > slot related code towards the end in
On Mon, Feb 1, 2021 at 11:26 PM Amit Kapila wrote:
> I have updated the patch to display WARNING for each of the tablesync
> slots during DropSubscription. As discussed, I have moved the drop
> slot related code towards the end in AlterSubscription_refresh. Apart
> from this, I have fixed one
On Mon, Feb 1, 2021 at 11:23 AM Peter Smith wrote:
>
> On Mon, Feb 1, 2021 at 3:44 PM Amit Kapila wrote:
> >
> > On Mon, Feb 1, 2021 at 9:38 AM Peter Smith wrote:
> >
> > No, there is a functionality change as well. The way we have code in
> > v22 can easily lead to a problem when we have
On Mon, Feb 1, 2021 at 1:08 PM Peter Smith wrote:
>
> On Mon, Feb 1, 2021 at 5:19 PM Amit Kapila wrote:
>
> > > > AFAIK there is always a potential race with DropSubscription dropping
> > > > > slots. The DropSubscription might be running at exactly the same time
> > > > > the apply worker has
On Mon, Feb 1, 2021 at 5:19 PM Amit Kapila wrote:
> > > AFAIK there is always a potential race with DropSubscription dropping
> > > > slots. The DropSubscription might be running at exactly the same time
> > > > the apply worker has just dropped the very same tablesync slot.
> > > >
> > >
> > >
On Mon, Feb 1, 2021 at 11:23 AM Peter Smith wrote:
>
> On Mon, Feb 1, 2021 at 3:44 PM Amit Kapila wrote:
> >
> > On Mon, Feb 1, 2021 at 9:38 AM Peter Smith wrote:
> > >
> > > > I think this is true only when the user specifically requested it by
> > > > the use of "ALTER SUBSCRIPTION ... SET
On Mon, Feb 1, 2021 at 3:44 PM Amit Kapila wrote:
>
> On Mon, Feb 1, 2021 at 9:38 AM Peter Smith wrote:
> >
> > On Mon, Feb 1, 2021 at 1:54 PM Amit Kapila wrote:
> > >
> > > On Mon, Feb 1, 2021 at 6:48 AM Peter Smith wrote:
> > > >
> > > > On Sun, Jan 31, 2021 at 12:19 AM Amit Kapila
> > > >
On Mon, Feb 1, 2021 at 10:14 AM Amit Kapila wrote:
>
> On Mon, Feb 1, 2021 at 9:38 AM Peter Smith wrote:
> >
> > On Mon, Feb 1, 2021 at 1:54 PM Amit Kapila wrote:
> > >
> > > On Mon, Feb 1, 2021 at 6:48 AM Peter Smith wrote:
> > > >
> > > > On Sun, Jan 31, 2021 at 12:19 AM Amit Kapila
> > >
On Mon, Feb 1, 2021 at 9:38 AM Peter Smith wrote:
>
> On Mon, Feb 1, 2021 at 1:54 PM Amit Kapila wrote:
> >
> > On Mon, Feb 1, 2021 at 6:48 AM Peter Smith wrote:
> > >
> > > On Sun, Jan 31, 2021 at 12:19 AM Amit Kapila
> > > wrote:
> > > >
> > > > I have made the below changes in the patch.
On Mon, Feb 1, 2021 at 1:54 PM Amit Kapila wrote:
>
> On Mon, Feb 1, 2021 at 6:48 AM Peter Smith wrote:
> >
> > On Sun, Jan 31, 2021 at 12:19 AM Amit Kapila
> > wrote:
> > >
> > > I have made the below changes in the patch. Let me know what you think
> > > about these?
> > > 1. It was a bit
On Mon, Feb 1, 2021 at 6:48 AM Peter Smith wrote:
>
> On Sun, Jan 31, 2021 at 12:19 AM Amit Kapila wrote:
> >
> > I have made the below changes in the patch. Let me know what you think
> > about these?
> > 1. It was a bit difficult to understand the code in DropSubscription
> > so I have
On Sun, Jan 31, 2021 at 12:19 AM Amit Kapila wrote:
> 2. In AlterSubscription_refresh(), we can't allow workers to be
> stopped at commit time as we have already dropped the slots because
> the worker can access the dropped slot. We need to stop the workers
> before dropping slots. This makes
On Sun, Jan 31, 2021 at 12:19 AM Amit Kapila wrote:
>
> I have made the below changes in the patch. Let me know what you think
> about these?
> 1. It was a bit difficult to understand the code in DropSubscription
> so I have rearranged the code to match the way we are doing in HEAD
> where we
On Fri, Jan 29, 2021 at 4:07 PM Peter Smith wrote:
>
>
> Differences from v21:
> + Patch is rebased to latest OSS HEAD @ 29/Jan.
> + Includes new code as suggested [ak0128] to ensure no dangling slots
> at Drop/AlterSubscription.
> + Removes the slot/origin cleanup down by process interrupt logic
On Thu, Jan 28, 2021 at 9:37 PM Amit Kapila wrote:
>
> On Thu, Jan 28, 2021 at 12:32 PM Peter Smith wrote:
> >
> > On Wed, Jan 27, 2021 at 2:53 PM Amit Kapila wrote:
> > >
> > > On Sat, Jan 23, 2021 at 5:56 PM Amit Kapila
> > > wrote:
> > > >
> > > > On Sat, Jan 23, 2021 at 4:55 AM Peter
Hi Amit.
PSA the v22 patch for the Tablesync Solution1.
Differences from v21:
+ Patch is rebased to latest OSS HEAD @ 29/Jan.
+ Includes new code as suggested [ak0128] to ensure no dangling slots
at Drop/AlterSubscription.
+ Removes the slot/origin cleanup down by process interrupt logic
On Thu, Jan 28, 2021 at 12:32 PM Peter Smith wrote:
>
> On Wed, Jan 27, 2021 at 2:53 PM Amit Kapila wrote:
> >
> > On Sat, Jan 23, 2021 at 5:56 PM Amit Kapila wrote:
> > >
> > > On Sat, Jan 23, 2021 at 4:55 AM Peter Smith wrote:
> > > >
> > > > PSA the v18 patch for the Tablesync Solution1.
>
On Wed, Jan 27, 2021 at 2:53 PM Amit Kapila wrote:
>
> On Sat, Jan 23, 2021 at 5:56 PM Amit Kapila wrote:
> >
> > On Sat, Jan 23, 2021 at 4:55 AM Peter Smith wrote:
> > >
> > > PSA the v18 patch for the Tablesync Solution1.
> >
> > 7. Have you tested with the new patch the scenario where we
Hi Amit.
PSA the v21 patch for the Tablesync Solution1.
Main differences from v20:
+ Rebased to latest OSS HEAD @ 27/Jan
+ v21 is a merging of patches [v17] and [v20], which was made
necessary when it was found [ak0127] that the v20 usage of TEMPORARY
tablesync slots did not work correctly. v21
On Sat, Jan 23, 2021 at 5:56 PM Amit Kapila wrote:
>
> On Sat, Jan 23, 2021 at 4:55 AM Peter Smith wrote:
> >
> > PSA the v18 patch for the Tablesync Solution1.
>
> 7. Have you tested with the new patch the scenario where we crash
> after FINISHEDCOPY and before SYNCDONE, is it able to pick up
On Mon, Jan 25, 2021 at 4:48 PM Amit Kapila wrote:
>
> On Mon, Jan 25, 2021 at 8:03 AM Peter Smith wrote:
> >
> > Hi Amit.
> >
> > PSA the v19 patch for the Tablesync Solution1.
> >
>
> I see one race condition in this patch where we try to drop the origin
> via apply process and
On Mon, Jan 25, 2021 at 4:48 PM Amit Kapila wrote:
>
> On Mon, Jan 25, 2021 at 8:03 AM Peter Smith wrote:
> >
> > Hi Amit.
> >
> > PSA the v19 patch for the Tablesync Solution1.
> >
>
> I see one race condition in this patch where we try to drop the origin
> via apply process and
On Mon, Jan 25, 2021 at 2:54 PM Amit Kapila wrote:
>
> On Mon, Jan 25, 2021 at 8:23 AM Peter Smith wrote:
> >
> > On Sat, Jan 23, 2021 at 11:26 PM Amit Kapila
> > wrote:
> > >
> > > 2.
> > > @@ -98,11 +102,16 @@
> > > #include "miscadmin.h"
> > > #include "parser/parse_relation.h"
> > >
On Mon, Jan 25, 2021 at 1:58 PM Amit Kapila wrote:
>
> On Sun, Jan 24, 2021 at 12:24 PM Peter Smith wrote:
> >
> > On Sat, Jan 23, 2021 at 11:26 PM Amit Kapila
> > wrote:
> > >
> > >
> > > Few comments:
> > > =
> > > 1.
> > > - * So the state progression is always: INIT ->
On Thu, Jan 21, 2021 at 9:17 PM Amit Kapila wrote:
> 7.
> +# check for occurrence of the expected error
> +poll_output_until("replication slot \"$slotname\" already exists")
> +or die "no error stop for the pre-existing origin";
>
> In this test, isn't it better to check for datasync state
Hi Amit.
PSA the v20 patch for the Tablesync Solution1.
Main differences from v19:
+ Updated TAP test [ak0123-7]
+ Fixed comment [ak0125-1]
+ Removed redundant header [ak0125-2]
+ Protection against race condition [ak0125-race]
[ak0123-7]
On Mon, Jan 25, 2021 at 8:03 AM Peter Smith wrote:
>
> Hi Amit.
>
> PSA the v19 patch for the Tablesync Solution1.
>
I see one race condition in this patch where we try to drop the origin
via apply process and DropSubscription. I think it can lead to the
error "cache lookup failed for
On Mon, Jan 25, 2021 at 8:23 AM Peter Smith wrote:
>
> On Sat, Jan 23, 2021 at 11:26 PM Amit Kapila wrote:
> >
> > 2.
> > @@ -98,11 +102,16 @@
> > #include "miscadmin.h"
> > #include "parser/parse_relation.h"
> > #include "pgstat.h"
> > +#include "postmaster/interrupt.h"
> > #include
On Sat, Jan 23, 2021 at 11:08 AM Ajin Cherian wrote:
>
> On Sat, Jan 23, 2021 at 3:16 PM Amit Kapila wrote:
>
> >
> > I think so. But do you have any reason to believe that it won't be
> > required anymore?
>
> A temporary slot will not clash with a permanent slot of the same name,
>
I have
On Sun, Jan 24, 2021 at 12:24 PM Peter Smith wrote:
>
> On Sat, Jan 23, 2021 at 11:26 PM Amit Kapila wrote:
> >
> >
> > Few comments:
> > =
> > 1.
> > - * So the state progression is always: INIT -> DATASYNC -> SYNCWAIT ->
> > - * CATCHUP -> SYNCDONE -> READY.
> > + * So the
On Sat, Jan 23, 2021 at 11:26 PM Amit Kapila wrote:
>
> 2.
> @@ -98,11 +102,16 @@
> #include "miscadmin.h"
> #include "parser/parse_relation.h"
> #include "pgstat.h"
> +#include "postmaster/interrupt.h"
> #include "replication/logicallauncher.h"
> #include "replication/logicalrelation.h"
>
On Mon, Jan 25, 2021 at 6:15 AM Peter Smith wrote:
>
> On Sun, Jan 24, 2021 at 5:54 PM Peter Smith wrote:
> > > 4.
> > > - /*
> > > - * To build a slot name for the sync work, we are limited to NAMEDATALEN
> > > -
> > > - * 1 characters. We cut the original slot name to NAMEDATALEN - 28 chars
Hi Amit.
PSA the v19 patch for the Tablesync Solution1.
Main differences from v18:
+ Patch has been rebased off HEAD @ 24/Jan
+ Addressing some review comments [ak0123]
[ak0123]
https://www.postgresql.org/message-id/CAA4eK1JhpuwujrV6ABMmZ3jXfW37ssZnJ3fikrY7rRdvoEmu_g%40mail.gmail.com
On Sun, Jan 24, 2021 at 5:54 PM Peter Smith wrote:
> > 4.
> > - /*
> > - * To build a slot name for the sync work, we are limited to NAMEDATALEN -
> > - * 1 characters. We cut the original slot name to NAMEDATALEN - 28 chars
> > - * and append _%u_sync_%u (1 + 10 + 6 + 10 + '\0'). (It's
On Sat, Jan 23, 2021 at 11:26 PM Amit Kapila wrote:
>
> On Sat, Jan 23, 2021 at 4:55 AM Peter Smith wrote:
> >
> > PSA the v18 patch for the Tablesync Solution1.
> >
>
> Few comments:
> =
> 1.
> - * So the state progression is always: INIT -> DATASYNC -> SYNCWAIT ->
> - * CATCHUP
FYI - I have done some long-running testing using the current patch [v18].
1. The src/test/subscription TAP tests:
- Subscription TAP tests were executed in a loop X 150 iterations.
- Duration 5 hrs.
- All iterations report "Result: PASS"
2. The postgres "make check" tests:
- make check was
On Sat, Jan 23, 2021 at 4:55 AM Peter Smith wrote:
>
> PSA the v18 patch for the Tablesync Solution1.
>
Few comments:
=
1.
- * So the state progression is always: INIT -> DATASYNC -> SYNCWAIT ->
- * CATCHUP -> SYNCDONE -> READY.
+ * So the state progression is always: INIT ->
On Sat, Jan 23, 2021 at 3:16 PM Amit Kapila wrote:
>
> I think so. But do you have any reason to believe that it won't be
> required anymore?
A temporary slot will not clash with a permanent slot of the same name,
regards,
Ajin Cherian
Fujitsu
1 - 100 of 187 matches
Mail list logo
