On Tue, Mar 2, 2021 at 12:43 PM vignesh C wrote:
>
> On Tue, Mar 2, 2021 at 9:33 AM Amit Kapila wrote:
> >
> > On Tue, Mar 2, 2021 at 8:20 AM vignesh C wrote:
> > >
> > >
> > > I have a minor comment regarding the below:
> > > +
> > > +
> > > + two_phase bool
> > > +
> > >
On Tue, Mar 2, 2021 at 9:33 AM Amit Kapila wrote:
>
> On Tue, Mar 2, 2021 at 8:20 AM vignesh C wrote:
> >
> >
> > I have a minor comment regarding the below:
> > +
> > +
> > + two_phase bool
> > +
> > +
> > + True if two-phase commits are enabled on this slot.
>
On Tue, Mar 2, 2021 at 10:38 AM Ajin Cherian wrote:
>
> On Tue, Mar 2, 2021 at 3:03 PM Amit Kapila wrote:
>
> One minor comment:
> +
> +
> + True if the slot is enabled for decoding prepared transactions.
> Always
> + false for physical slots.
> +
> +
>
> Ther
On Tue, Mar 2, 2021 at 3:03 PM Amit Kapila wrote:
One minor comment:
+
+
+ True if the slot is enabled for decoding prepared transactions. Always
+ false for physical slots.
+
+
There is an extra space before Always. But when rendered in html this
is not seen,
On Tue, Mar 2, 2021 at 8:20 AM vignesh C wrote:
>
>
> I have a minor comment regarding the below:
> +
> +
> + two_phase bool
> +
> +
> + True if two-phase commits are enabled on this slot.
> +
> +
>
> Can we change something like:
> True if the slot is
On Tue, Mar 2, 2021 at 6:37 AM Ajin Cherian wrote:
>
> On Mon, Mar 1, 2021 at 8:08 PM Amit Kapila wrote:
>
> > Few minor comments on 0002 patch
> > =
> > 1.
> > ctx->streaming &= enable_streaming;
> > - ctx->twophase &= enable_twophase;
> > +
> > }
> >
> > Spurious
On Mon, Mar 1, 2021 at 8:08 PM Amit Kapila wrote:
> Few minor comments on 0002 patch
> =
> 1.
> ctx->streaming &= enable_streaming;
> - ctx->twophase &= enable_twophase;
> +
> }
>
> Spurious line addition.
Deleted.
>
> 2.
> - proallargtypes =>
> '{name,name,text,
On Mon, Mar 1, 2021 at 7:23 AM Ajin Cherian wrote:
>
Few minor comments on 0002 patch
=
1.
ctx->streaming &= enable_streaming;
- ctx->twophase &= enable_twophase;
+
}
Spurious line addition.
2.
- proallargtypes =>
'{name,name,text,oid,bool,bool,int4,xid,xid,pg_lsn
On Mon, Mar 1, 2021 at 7:23 AM Ajin Cherian wrote:
>
Pushed, the first patch in the series.
--
With Regards,
Amit Kapila.
On Mon, Mar 1, 2021 at 7:23 AM Ajin Cherian wrote:
>
> On Sat, Feb 27, 2021 at 11:06 PM Amit Kapila wrote:
>
> > Few comments on 0002 patch:
> > =
> > 1.
> > +
> > + /*
> > + * Disable two-phase here, it will be set in the core if it was
> > + * enabled whole creating the
t; when we support it in CreateSubscription). Right now, there is nothing
> to test all the code you have added in repl_gram.y.
>
Removed that.
> 4. I think we can expose this new option via pg_replication_slots.
>
Done. Added,
regards,
Ajin Cherian
Fujitsu Australia
v7-0001-Avoid-re
t; > 3. Ran pgindent, minor changes in comments, and modified the commit message.
> >
> > Let me know what you think about these changes.
> >
>
> In the attached, I have just bumped SNAPBUILD_VERSION as we are
> adding a new member in the SnapBuild structure.
>
On Sat, Feb 27, 2021 at 8:29 AM Amit Kapila wrote:
>
> On Fri, Feb 26, 2021 at 7:26 PM vignesh C wrote:
> >
> > On Fri, Feb 26, 2021 at 4:13 PM Ajin Cherian wrote:
> > >
> > > On Fri, Feb 26, 2021 at 7:47 PM Ajin Cherian wrote:
> > >
> > > > I've updated snapshot_was_exported_at_ member to pg_
port it in CreateSubscription). Right now, there is nothing
to test all the code you have added in repl_gram.y.
4. I think we can expose this new option via pg_replication_slots.
--
With Regards,
Amit Kapila.
v6-0001-Avoid-repeated-decoding-of-prepared-transactions-.patch
Description: Binary data
v6-0002-Add-option-to-enable-two-phase-commits-in-pg_crea.patch
Description: Binary data
52ba6cc8602f66acac8209317da0a Mon Sep 17 00:00:00 2001
From: Ajin Cherian
Date: Fri, 26 Feb 2021 02:58:49 -0500
Subject: [PATCH v5 1/2] Avoid repeated decoding of prepared transactions after
the restart.
In commit a271a1b50e, we allowed decoding at prepare time and the prepare
was decoded again i
On Sat, 27 Feb, 2021, 1:59 pm Amit Kapila, wrote:
>
> I have recommended above to change this name to initial_consistency_at
> because there are times when we don't export snapshot and we still set
> this like when creating slots with CRS_NOEXPORT_SNAPSHOT or when
> creating via SQL APIs. I am n
On Thu, Feb 25, 2021 at 5:04 PM vignesh C wrote:
>
> On Wed, Feb 24, 2021 at 5:06 PM Ajin Cherian wrote:
> >
> > On Wed, Feb 24, 2021 at 4:48 PM Ajin Cherian wrote:
> >
> > > I plan to split this into two patches next. But do review and let me
> > > know if you have any comments.
> >
> > Attachi
On Fri, Feb 26, 2021 at 7:26 PM vignesh C wrote:
>
> On Fri, Feb 26, 2021 at 4:13 PM Ajin Cherian wrote:
> >
> > On Fri, Feb 26, 2021 at 7:47 PM Ajin Cherian wrote:
> >
> > > I've updated snapshot_was_exported_at_ member to pg_replication_slots as
> > > well.
> > > Do have a look and let me kn
On Fri, Feb 26, 2021 at 4:13 PM Ajin Cherian wrote:
>
> On Fri, Feb 26, 2021 at 7:47 PM Ajin Cherian wrote:
>
> > I've updated snapshot_was_exported_at_ member to pg_replication_slots as
> > well.
> > Do have a look and let me know if there are any comments.
>
> Update with both patches.
Thank
On Fri, Feb 26, 2021 at 7:47 PM Ajin Cherian wrote:
> I've updated snapshot_was_exported_at_ member to pg_replication_slots as
> well.
> Do have a look and let me know if there are any comments.
Update with both patches.
regards,
Ajin Cherian
Fujitsu Australia
v4-0002-Add-option-to-enable-t
On Thu, Feb 25, 2021 at 10:36 PM Amit Kapila wrote:
> Few comments on the first patch:
> 1. We can't remove ReorderBufferSkipPrepare because we rely on that in
> SnapBuildDistributeNewCatalogSnapshot.
> 2. I have changed the name of the variable from
> snapshot_was_exported_at_lsn to snapshot_was
On Wed, Feb 24, 2021 at 5:06 PM Ajin Cherian wrote:
>
> On Wed, Feb 24, 2021 at 4:48 PM Ajin Cherian wrote:
>
> > I plan to split this into two patches next. But do review and let me
> > know if you have any comments.
>
> Attaching an updated patch-set with the changes for
> snapshot_was_exported
On Wed, Feb 24, 2021 at 5:06 PM Ajin Cherian wrote:
>
> On Wed, Feb 24, 2021 at 4:48 PM Ajin Cherian wrote:
>
> > I plan to split this into two patches next. But do review and let me
> > know if you have any comments.
>
> Attaching an updated patch-set with the changes for
> snapshot_was_exported
On Wed, Feb 24, 2021 at 4:48 PM Ajin Cherian wrote:
> I plan to split this into two patches next. But do review and let me
> know if you have any comments.
Attaching an updated patch-set with the changes for
snapshot_was_exported_at_lsn separated out from the changes for the
APIs pg_create_logic
On Tue, Feb 23, 2021 at 8:54 PM Amit Kapila wrote:
> 1. With respect to SQL APIs, currently 'two-phase-commit' is a plugin
> option so it is possible that the first time when it gets changes
> (pg_logical_slot_get_changes) *without* 2PC enabled it will not get
> the prepared even though prepare i
On Mon, Feb 22, 2021 at 2:57 PM Andres Freund wrote:
>
> > Yeah, we need to probably store this new point as slot's persistent
> > information.
>
> Should be fine I think...
>
So, we are in agreement that the above solution will work and we won't
need to resend the prepare after the restart. I w
On Mon, Feb 22, 2021 at 8:27 PM Andres Freund wrote:
> > Yeah, we need to probably store this new point as slot's persistent
> > information.
>
> Should be fine I think...
This idea looks convincing. I'll write up a patch incorporating these changes.
regards,
Ajin Cherian
Fujitsu Australia
On Mon, Feb 22, 2021 at 2:55 PM Andres Freund wrote:
>
> Hi,
>
> On 2021-02-22 09:25:52 +0100, Markus Wanner wrote:
> > What issues do you see with the approach I proposed?
>
> Very significant increase in complexity for initializing a logical
> replica, because there's no easy way to just use the
Hi,
On 2021-02-22 14:29:09 +0530, Amit Kapila wrote:
> On Mon, Feb 22, 2021 at 9:39 AM Andres Freund wrote:
> > What I am proposing is to instead track the point at which the slot
> > gained consistency - a simple LSN. That way you can change the above
> > logic to instead be
> >
> > if (txn->fin
Hi,
On 2021-02-22 09:25:52 +0100, Markus Wanner wrote:
> What issues do you see with the approach I proposed?
Very significant increase in complexity for initializing a logical
replica, because there's no easy way to just use the initial slot.
- Andres
On Mon, Feb 22, 2021 at 9:39 AM Andres Freund wrote:
>
> Hi,
>
> On 2021-02-22 08:22:35 +0530, Amit Kapila wrote:
> > On Mon, Feb 22, 2021 at 3:56 AM Andres Freund wrote:
> > >
> > > On 2021-02-21 11:32:29 +0530, Amit Kapila wrote:
> > > > Here, I am assuming you are asking to disable 2PC both vi
On 22.02.21 05:22, Andres Freund wrote:
Hi,
On 2021-02-19 15:53:32 +0100, Markus Wanner wrote:
However, more generally speaking, I suspect you are overthinking this. All
of the complexity arises because of the assumption that an output plugin
receiving and confirming a PREPARE may not be able t
On 20.02.21 13:15, Amit Kapila wrote:
I think after the patch Ajin proposed decoders won't need any special
checks after receiving the prepared xacts. What additional simplicity
this approach will bring?
The API becomes clearer in that all PREPAREs are always decoded in WAL
stream order and ar
Hi,
On 2021-02-19 15:53:32 +0100, Markus Wanner wrote:
> However, more generally speaking, I suspect you are overthinking this. All
> of the complexity arises because of the assumption that an output plugin
> receiving and confirming a PREPARE may not be able to persist that first
> phase of trans
Hi,
On 2021-02-22 08:22:35 +0530, Amit Kapila wrote:
> On Mon, Feb 22, 2021 at 3:56 AM Andres Freund wrote:
> >
> > On 2021-02-21 11:32:29 +0530, Amit Kapila wrote:
> > > Here, I am assuming you are asking to disable 2PC both via
> > > apply-worker and tablesync worker till the initial sync (aka
On Mon, Feb 22, 2021 at 3:56 AM Andres Freund wrote:
>
> On 2021-02-21 11:32:29 +0530, Amit Kapila wrote:
> > Here, I am assuming you are asking to disable 2PC both via
> > apply-worker and tablesync worker till the initial sync (aka all
> > tables are in SUBREL_STATE_READY state) phase is complet
Hi,
On 2021-02-21 11:32:29 +0530, Amit Kapila wrote:
> Here, I am assuming you are asking to disable 2PC both via
> apply-worker and tablesync worker till the initial sync (aka all
> tables are in SUBREL_STATE_READY state) phase is complete. If we do
> that and what if commit prepared happened aft
On Sat, Feb 20, 2021 at 10:26 PM Andres Freund wrote:
>
> Hi,
>
> On Fri, Feb 19, 2021, at 19:38, Amit Kapila wrote:
> > On Fri, Feb 19, 2021 at 8:23 PM Markus Wanner
> > wrote:
> > >
> > > With that line of thinking, the point in time (or in WAL) of the COMMIT
> > > PREPARED does not matter at a
Hi,
On 2021-02-19 13:50:52 +1100, Ajin Cherian wrote:
> From 129947ab2d0ba223862ed1c87be0f96b51645ba0 Mon Sep 17 00:00:00 2001
> From: Ajin Cherian
> Date: Thu, 18 Feb 2021 20:18:16 -0500
> Subject: [PATCH] Don't allow repeated decoding of prepared transactions.
>
> Enfo
Hi,
On Fri, Feb 19, 2021, at 19:38, Amit Kapila wrote:
> On Fri, Feb 19, 2021 at 8:23 PM Markus Wanner
> wrote:
> >
> > With that line of thinking, the point in time (or in WAL) of the COMMIT
> > PREPARED does not matter at all to reason about the decoding of the
> > PREPARE operation. Instead,
On Sat, Feb 20, 2021 at 4:25 PM Markus Wanner
wrote:
>
> On 20.02.21 04:38, Amit Kapila wrote:
> > I see a problem with this assumption. During the initial
> > synchronization, this transaction won't be visible to snapshot and we
> > won't copy it. Then later if we won't decode and send it then th
On 20.02.21 04:38, Amit Kapila wrote:
I see a problem with this assumption. During the initial
synchronization, this transaction won't be visible to snapshot and we
won't copy it. Then later if we won't decode and send it then the
replica will be out of sync. Such a problem won't happen with Ajin
On Sat, Feb 20, 2021 at 9:46 AM Amit Kapila wrote:
>
> On Fri, Feb 19, 2021 at 8:21 AM Ajin Cherian wrote:
> >
> > Based on this suggestion, I have created a patch on HEAD which now
> > does not allow repeated decoding
> > of prepared transactions. F
On Fri, Feb 19, 2021 at 8:21 AM Ajin Cherian wrote:
>
> Based on this suggestion, I have created a patch on HEAD which now
> does not allow repeated decoding
> of prepared transactions. For this, the code now enforces
> full_snapshot if two-phase decoding is enabled.
> Do have a
On Fri, Feb 19, 2021 at 8:23 PM Markus Wanner
wrote:
>
> With that line of thinking, the point in time (or in WAL) of the COMMIT
> PREPARED does not matter at all to reason about the decoding of the
> PREPARE operation. Instead, there are only exactly two cases to consider:
>
> a) the PREPARE hap
Ajin, Amit,
thank you both a lot for thinking this through and even providing a patch.
The changes in expectation for twophase.out matches exactly with what I
prepared. And the switch with pg_logical_slot_get_changes indeed is
something I had not yet considered, either.
On 19.02.21 03:50, A
re two_phase decoding option is enabled then
> we won't have that problem. The drawback is that in some cases it can
> take a bit more time for initial snapshot building but maybe that is
> better than the current solution.
Based on this suggestion, I have created a patch on H
On Tue, Feb 16, 2021 at 9:43 AM Amit Kapila wrote:
>
> On Thu, Feb 11, 2021 at 4:06 PM Amit Kapila wrote:
> >
> > On Mon, Feb 8, 2021 at 2:01 PM Markus Wanner
> > wrote:
> > >
> > Now, coming back to the restart case where the prepared transaction
> > can be sent again by the publisher. I unders
On Thu, Feb 11, 2021 at 4:06 PM Amit Kapila wrote:
>
> On Mon, Feb 8, 2021 at 2:01 PM Markus Wanner
> wrote:
> >
> Now, coming back to the restart case where the prepared transaction
> can be sent again by the publisher. I understand yours and others
> point that we should not send prepared trans
On Sat, Feb 13, 2021 at 10:23 PM Andres Freund wrote:
>
> On 2021-02-13 17:37:29 +0100, Petr Jelinek wrote:
>
> > AFAIK this is exactly why origins are Wal logged along with
> > transaction, it allows us to guarantee never getting anything that has
> > beed durably written.
>
> I think you'd need
Hi,
On 2021-02-13 17:37:29 +0100, Petr Jelinek wrote:
> Agreed, if we relied purely on flush location of a slot, there would
> be no need for origins to track the lsn.
And we would be latency bound replicating transactions, which'd not be
fun for single-insert ones for example...
> AFAIK this i
>
> On 13 Feb 2021, at 17:32, Andres Freund wrote:
>
> Hi,
>
> On 2021-02-10 08:02:17 +0530, Amit Kapila wrote:
>> On Wed, Feb 10, 2021 at 12:08 AM Robert Haas wrote:
>>>
>> If by successfully confirmed, you mean that once the subscriber node
>> has received, it won't be sent again then as fa
Hi,
On 2021-02-10 08:02:17 +0530, Amit Kapila wrote:
> On Wed, Feb 10, 2021 at 12:08 AM Robert Haas wrote:
> >
> > On Tue, Feb 9, 2021 at 6:57 AM Amit Kapila wrote:
> > > I think similar happens without any of the work done in PG-14 as well
> > > if we restart the apply worker before the commit
On Fri, Feb 12, 2021 at 1:10 AM Robert Haas wrote:
>
> On Thu, Feb 11, 2021 at 5:37 AM Amit Kapila wrote:
> > to explain the exact case to you which is not very apparent. The basic
> > idea is that we ship/replay all transactions where commit happens
> > after the snapshot has a consistent state
On Thu, Feb 11, 2021 at 5:37 AM Amit Kapila wrote:
> to explain the exact case to you which is not very apparent. The basic
> idea is that we ship/replay all transactions where commit happens
> after the snapshot has a consistent state (SNAPBUILD_CONSISTENT), see
> atop snapbuild.c for details. No
Hello Amit,
thanks a lot for your extensive explanation and examples, I appreciate
this very much. I'll need to think this through and see how we can make
this work for us.
Best Regards
Markus
On Mon, Feb 8, 2021 at 2:01 PM Markus Wanner
wrote:
>
> I did not expect this, as any receiver that wants to have decoded 2PC is
> likely supporting some kind of two-phase commits itself. And would
> therefore prepare the transaction upon its first reception. Potentially
> receiving it a second
On Tue, Feb 9, 2021 at 9:32 PM Amit Kapila wrote:
> If by successfully confirmed, you mean that once the subscriber node
> has received, it won't be sent again then as far as I know that is not
> true. We rely on the flush location sent by the subscriber to advance
> the decoding locations. We upd
On Wed, Feb 10, 2021 at 1:40 PM Markus Wanner
wrote:
>
> On 10.02.21 07:32, Amit Kapila wrote:
> > On Wed, Feb 10, 2021 at 11:45 AM Ajin Cherian wrote:
> >> But the other side of the problem is that ,without this, if the
> >> prepared transaction is prior to a consistent snapshot when decoding
>
On 10.02.21 07:32, Amit Kapila wrote:
On Wed, Feb 10, 2021 at 11:45 AM Ajin Cherian wrote:
But the other side of the problem is that ,without this, if the
prepared transaction is prior to a consistent snapshot when decoding
starts/restarts, then only the "commit prepared" is sent to downstream
On Wed, Feb 10, 2021 at 11:45 AM Ajin Cherian wrote:
>
> On Wed, Feb 10, 2021 at 3:43 PM Ashutosh Bapat
> wrote:
>
>
> > I think we need to treat a prepared transaction slightly different from an
> > uncommitted transaction when sending downstream. We need to send a whole
> > uncommitted transa
On Wed, Feb 10, 2021 at 3:43 PM Ashutosh Bapat
wrote:
> I think we need to treat a prepared transaction slightly different from an
> uncommitted transaction when sending downstream. We need to send a whole
> uncommitted transaction downstream again because previously applied changes
> must ha
On Wed, Feb 10, 2021 at 8:02 AM Amit Kapila wrote:
> On Wed, Feb 10, 2021 at 12:08 AM Robert Haas
> wrote:
> >
> > On Tue, Feb 9, 2021 at 6:57 AM Amit Kapila
> wrote:
> > > I think similar happens without any of the work done in PG-14 as well
> > > if we restart the apply worker before the comm
On Wed, Feb 10, 2021 at 12:08 AM Robert Haas wrote:
>
> On Tue, Feb 9, 2021 at 6:57 AM Amit Kapila wrote:
> > I think similar happens without any of the work done in PG-14 as well
> > if we restart the apply worker before the commit completes on the
> > subscriber. After the restart, we will send
On Tue, Feb 9, 2021 at 6:57 AM Amit Kapila wrote:
> I think similar happens without any of the work done in PG-14 as well
> if we restart the apply worker before the commit completes on the
> subscriber. After the restart, we will send the start_decoding_at
> point based on some previous commit wh
On Tue, Feb 9, 2021 at 11:29 AM Ashutosh Bapat
wrote:
>
> On Tue, Feb 9, 2021 at 8:32 AM Amit Kapila wrote:
> >
> > On Mon, Feb 8, 2021 at 8:36 PM Markus Wanner
> > wrote:
> > >
> > > Hello Amit,
> > >
> > > thanks for your very quick response.
> > >
> > > On 08.02.21 11:13, Amit Kapila wrote:
>
On Tue, Feb 9, 2021 at 4:59 PM Ashutosh Bapat
wrote:
> Can you please provide steps which can lead to this situation? If
> there is an earlier discussion which has example scenarios, please
> point us to the relevant thread.
>
> If we are not sending PREPARED transactions that's fine, but sending
On Tue, Feb 9, 2021 at 8:32 AM Amit Kapila wrote:
>
> On Mon, Feb 8, 2021 at 8:36 PM Markus Wanner
> wrote:
> >
> > Hello Amit,
> >
> > thanks for your very quick response.
> >
> > On 08.02.21 11:13, Amit Kapila wrote:
> > > /*
> > > * It is possible that this transaction is not decoded at prep
On Mon, Feb 8, 2021 at 8:36 PM Markus Wanner
wrote:
>
> Hello Amit,
>
> thanks for your very quick response.
>
> On 08.02.21 11:13, Amit Kapila wrote:
> > /*
> > * It is possible that this transaction is not decoded at prepare time
> > * either because by that time we didn't have a consistent
Hello Amit,
thanks for your very quick response.
On 08.02.21 11:13, Amit Kapila wrote:
/*
* It is possible that this transaction is not decoded at prepare time
* either because by that time we didn't have a consistent snapshot or it
* was decoded earlier but we have restarted. We can't di
On Mon, Feb 8, 2021 at 2:01 PM Markus Wanner
wrote:
>
> Amit, Ajin, hackers,
>
> testing logical decoding for two-phase transactions, I stumbled over
> what I first thought is a bug. But comments seems to indicate this is
> intended behavior. Could you please clarify or elaborate on the design
>
Amit, Ajin, hackers,
testing logical decoding for two-phase transactions, I stumbled over
what I first thought is a bug. But comments seems to indicate this is
intended behavior. Could you please clarify or elaborate on the design
decision? Or indicate this indeed is a bug?
What puzzled m
72 matches
Mail list logo
