Hi,
Let me share a bit of an update regarding this patch and PG17. I have
discussed this patch and how to move it forward with a couple hackers
(both within EDB and outside), and my takeaway is that the patch is not
quite baked yet, not enough to make it into PG17 :-(
There are two main reasons /
On Wed, Feb 21, 2024 at 2:52 PM Robert Haas wrote:
>
> On Wed, Feb 21, 2024 at 2:43 PM Dilip Kumar wrote:
> > So the problem is that we might consider the transaction change as
> > non-transaction and mark this flag as true.
>
> But it's not "might" right? It's absolutely 100% certain that we wil
On Wed, Feb 21, 2024 at 2:43 PM Dilip Kumar wrote:
> So the problem is that we might consider the transaction change as
> non-transaction and mark this flag as true.
But it's not "might" right? It's absolutely 100% certain that we will
consider that transaction's changes as non-transactional ...
On Wed, Feb 21, 2024 at 1:24 PM Robert Haas wrote:
>
> On Wed, Feb 21, 2024 at 1:06 PM Dilip Kumar wrote:
> > > But I am wondering why this flag is always set to true in
> > > DecodeTXNNeedSkip() irrespective of the commit or abort. Because the
> > > aborted transactions are not supposed to be re
On Wed, Feb 21, 2024 at 1:06 PM Dilip Kumar wrote:
> > But I am wondering why this flag is always set to true in
> > DecodeTXNNeedSkip() irrespective of the commit or abort. Because the
> > aborted transactions are not supposed to be replayed? So if my
> > observation is correct that for the abor
On Tue, Feb 20, 2024 at 4:32 PM Dilip Kumar wrote:
>
> On Tue, Feb 20, 2024 at 3:38 PM Robert Haas wrote:
>
> > Let's say fast_forward is true. Then smgr_decode() is going to skip
> > recording anything about the relfilenode, so we'll identify all
> > sequence changes as non-transactional. But lo
On Wed, Feb 14, 2024 at 10:21 PM Tomas Vondra
wrote:
>
> On 2/13/24 17:37, Robert Haas wrote:
>
> > In other words, the fact that some sequence changes are
> > non-transactional creates ordering hazards that don't exist if there
> > are no non-transactional changes. So in that way, sequences are
>
On Tue, Feb 20, 2024 at 5:39 PM Tomas Vondra
wrote:
>
> On 2/20/24 06:54, Amit Kapila wrote:
> > On Thu, Dec 21, 2023 at 6:47 PM Tomas Vondra
> > wrote:
> >>
> >> On 12/19/23 13:54, Christophe Pettus wrote:
> >>> Hi,
> >>>
> >>> I wanted to hop in here on one particular issue:
> >>>
> On Dec
On 2/20/24 06:54, Amit Kapila wrote:
> On Thu, Dec 21, 2023 at 6:47 PM Tomas Vondra
> wrote:
>>
>> On 12/19/23 13:54, Christophe Pettus wrote:
>>> Hi,
>>>
>>> I wanted to hop in here on one particular issue:
>>>
On Dec 12, 2023, at 02:01, Tomas Vondra
wrote:
- desirability of t
On Tue, Feb 20, 2024 at 3:38 PM Robert Haas wrote:
> Let's say fast_forward is true. Then smgr_decode() is going to skip
> recording anything about the relfilenode, so we'll identify all
> sequence changes as non-transactional. But look at how this case is
> handled in seq_decode():
>
> + if (ctx
On Tue, Feb 20, 2024 at 1:32 PM Dilip Kumar wrote:
> You might be interested in more detail [1] where I first reported this
> problem and also [2] where we concluded why this is not creating a
> real problem.
>
> [1]
> https://www.postgresql.org/message-id/CAFiTN-vAx-Y%2B19ROKOcWnGf7ix2VOTUebpzte
On Tue, Feb 20, 2024 at 10:30 AM Robert Haas wrote:
>
> Is the rule that changes are transactional if and only if the current
> transaction has assigned a new relfilenode to the sequence?
Yes, thats the rule.
> Why does the logic get confused if the state of the snapshot changes?
The rule doesn
On Thu, Dec 21, 2023 at 6:47 PM Tomas Vondra
wrote:
>
> On 12/19/23 13:54, Christophe Pettus wrote:
> > Hi,
> >
> > I wanted to hop in here on one particular issue:
> >
> >> On Dec 12, 2023, at 02:01, Tomas Vondra
> >> wrote:
> >> - desirability of the feature: Random IDs (UUIDs etc.) are likely
On Fri, Feb 16, 2024 at 1:57 AM Tomas Vondra
wrote:
> For me, the part that I feel most uneasy about is the decoding while the
> snapshot is still being built (and can flip to consistent snapshot
> between the relfilenode creation and sequence change, confusing the
> logic that decides which chang
On 2/15/24 05:16, Robert Haas wrote:
> On Wed, Feb 14, 2024 at 10:21 PM Tomas Vondra
> wrote:
>> The way I think about non-transactional sequence changes is as if they
>> were tiny transactions that happen "fully" (including commit) at the LSN
>> where the LSN change is logged.
>
> 100% this.
>
On Wed, Feb 14, 2024 at 10:21 PM Tomas Vondra
wrote:
> The way I think about non-transactional sequence changes is as if they
> were tiny transactions that happen "fully" (including commit) at the LSN
> where the LSN change is logged.
100% this.
> It does not mean we can arbitrarily reorder the
On 2/13/24 17:37, Robert Haas wrote:
> On Sun, Jan 28, 2024 at 1:07 AM Tomas Vondra
> wrote:
>> Right, locks + apply in commit order gives us this guarantee (I can't
>> think of a case where it wouldn't be the case).
>
> I couldn't find any cases of inadequate locking other than the one I
> m
On Sun, Jan 28, 2024 at 1:07 AM Tomas Vondra
wrote:
> Right, locks + apply in commit order gives us this guarantee (I can't
> think of a case where it wouldn't be the case).
I couldn't find any cases of inadequate locking other than the one I mentioned.
> Doesn't the whole logical replication cr
On 1/26/24 15:39, Robert Haas wrote:
> On Wed, Jan 24, 2024 at 12:46 PM Tomas Vondra
> wrote:
>> I did try to explain how this works (and why) in a couple places:
>>
>> 1) the commit message
>> 2) reorderbuffer header comment
>> 3) ReorderBufferSequenceIsTransactional comment (and nearby)
>>
>> It
On Wed, Jan 24, 2024 at 12:46 PM Tomas Vondra
wrote:
> I did try to explain how this works (and why) in a couple places:
>
> 1) the commit message
> 2) reorderbuffer header comment
> 3) ReorderBufferSequenceIsTransactional comment (and nearby)
>
> It's possible this does not meet your expectations
On 1/23/24 21:47, Robert Haas wrote:
> On Thu, Jan 11, 2024 at 11:27 AM Tomas Vondra
> wrote:
>> 1) desirability: We want a built-in way to handle sequences in logical
>> replication. I think everyone agrees this is not a way to do distributed
>> sequences in an active-active setups, but that t
On Thu, Jan 11, 2024 at 11:27 AM Tomas Vondra
wrote:
> 1) desirability: We want a built-in way to handle sequences in logical
> replication. I think everyone agrees this is not a way to do distributed
> sequences in an active-active setups, but that there are other use cases
> that need this featu
On 12/15/23 03:33, Amit Kapila wrote:
> On Thu, Dec 14, 2023 at 9:14 PM Ashutosh Bapat
> wrote:
>>
>> On Thu, Dec 14, 2023 at 2:51 PM Amit Kapila wrote:
>>>
>>> It can only be cleaned if we process it but xact_decode won't allow us
>>> to process it and I don't think it would be a good idea to ad
On 12/19/23 13:54, Christophe Pettus wrote:
> Hi,
>
> I wanted to hop in here on one particular issue:
>
>> On Dec 12, 2023, at 02:01, Tomas Vondra
>> wrote:
>> - desirability of the feature: Random IDs (UUIDs etc.) are likely a much
>> better solution for distributed (esp. active-active) syste
Hi,
I wanted to hop in here on one particular issue:
> On Dec 12, 2023, at 02:01, Tomas Vondra wrote:
> - desirability of the feature: Random IDs (UUIDs etc.) are likely a much
> better solution for distributed (esp. active-active) systems. But there
> are important use cases that are likely to
On Thu, Dec 14, 2023 at 9:14 PM Ashutosh Bapat
wrote:
>
> On Thu, Dec 14, 2023 at 2:51 PM Amit Kapila wrote:
> >
> > It can only be cleaned if we process it but xact_decode won't allow us
> > to process it and I don't think it would be a good idea to add another
> > hack for sequences here. See b
On Thu, Dec 14, 2023, at 12:44 PM, Ashutosh Bapat wrote:
> I haven't found the code path from where the sequence cleanup gets
> called. But it's being called. Am I missing something?
ReorderBufferCleanupTXN.
--
Euler Taveira
EDB https://www.enterprisedb.com/
On Thu, Dec 14, 2023 at 2:51 PM Amit Kapila wrote:
>
> On Thu, Dec 14, 2023 at 2:45 PM Ashutosh Bapat
> wrote:
> >
> > On Thu, Dec 14, 2023 at 12:37 PM Dilip Kumar wrote:
> > >
> > > I think you forgot to attach the patch.
> >
> > Sorry. Here it is.
> >
> > On Thu, Dec 14, 2023 at 2:36 PM Amit K
On Thu, Dec 14, 2023 at 2:45 PM Ashutosh Bapat
wrote:
>
> On Thu, Dec 14, 2023 at 12:37 PM Dilip Kumar wrote:
> >
> > I think you forgot to attach the patch.
>
> Sorry. Here it is.
>
> On Thu, Dec 14, 2023 at 2:36 PM Amit Kapila wrote:
> >
> > >
> > It looks like the solution works. But this is
On Thu, Dec 14, 2023 at 12:37 PM Dilip Kumar wrote:
>
> I think you forgot to attach the patch.
Sorry. Here it is.
On Thu, Dec 14, 2023 at 2:36 PM Amit Kapila wrote:
>
> >
> It looks like the solution works. But this is the only place where we
> process a change before SNAPSHOT reaches FULL. Bu
On Thu, Dec 14, 2023 at 12:31 PM Ashutosh Bapat
wrote:
>
> On Thu, Dec 14, 2023 at 10:53 AM Dilip Kumar wrote:
>
> > > >
> > >
> > > It is correct that we can make a wrong decision about whether a change
> > > is transactional or non-transactional when sequence DDL happens before
> > > the SNAPBU
On Thu, Dec 14, 2023 at 12:31 PM Ashutosh Bapat
wrote:
>
> On Thu, Dec 14, 2023 at 10:53 AM Dilip Kumar wrote:
>
> > > >
> > >
> > > It is correct that we can make a wrong decision about whether a change
> > > is transactional or non-transactional when sequence DDL happens before
> > > the SNAPBU
On Thu, Dec 14, 2023 at 10:53 AM Dilip Kumar wrote:
> > >
> >
> > It is correct that we can make a wrong decision about whether a change
> > is transactional or non-transactional when sequence DDL happens before
> > the SNAPBUILD_FULL_SNAPSHOT state and the sequence operation happens
> > after th
On Wed, Dec 13, 2023 at 6:26 PM Amit Kapila wrote:
>
> > > But can this even happen? Can we start decoding in the middle of a
> > > transaction? How come this wouldn't affect e.g. XLOG_HEAP2_NEW_CID,
> > > which is also skipped until SNAPBUILD_FULL_SNAPSHOT. Or logical
> > > messages, where we als
Dear hackers,
> It is correct that we can make a wrong decision about whether a change
> is transactional or non-transactional when sequence DDL happens before
> the SNAPBUILD_FULL_SNAPSHOT state and the sequence operation happens
> after that state.
I found a workload which decoder distinguish w
On Thu, Dec 7, 2023 at 10:41 AM Dilip Kumar wrote:
>
> On Wed, Dec 6, 2023 at 7:09 PM Tomas Vondra
> wrote:
> >
> > Yes, if something like this happens, that'd be a problem:
> >
> > 1) decoding starts, with
> >
> >SnapBuildCurrentState(builder) < SNAPBUILD_FULL_SNAPSHOT
> >
> > 2) transaction
Hi,
There's been a lot discussed over the past month or so, and it's become
difficult to get a good idea what's the current state - what issues
remain to be solved, what's unrelated to this patch, and how to move if
forward. Long-running threads tend to be confusing, so I had a short
call with Ami
On Wed, Dec 6, 2023 at 7:09 PM Tomas Vondra
wrote:
>
> Yes, if something like this happens, that'd be a problem:
>
> 1) decoding starts, with
>
>SnapBuildCurrentState(builder) < SNAPBUILD_FULL_SNAPSHOT
>
> 2) transaction that creates a new refilenode gets decoded, but we skip
>it because w
On Wed, Dec 6, 2023 at 7:17 PM Tomas Vondra
wrote:
>
> On 12/6/23 12:05, Dilip Kumar wrote:
> > On Wed, Dec 6, 2023 at 3:36 PM Amit Kapila wrote:
> >>
> >>> Why can't we use the same concept of
> >>> SnapBuildDistributeNewCatalogSnapshot(), I mean we keep queuing the
> >>> non-transactional chang
On Wed, Dec 6, 2023 at 7:17 PM Tomas Vondra
wrote:
>
> On 12/6/23 12:05, Dilip Kumar wrote:
> > On Wed, Dec 6, 2023 at 3:36 PM Amit Kapila wrote:
> >>
> >>> Why can't we use the same concept of
> >>> SnapBuildDistributeNewCatalogSnapshot(), I mean we keep queuing the
> >>> non-transactional chang
On Wed, Dec 6, 2023 at 7:20 PM Tomas Vondra
wrote:
>
> On 12/6/23 11:19, Amit Kapila wrote:
> > On Sun, Dec 3, 2023 at 11:56 PM Tomas Vondra
> > wrote:
> >>
> >> On 12/3/23 18:52, Tomas Vondra wrote:
> >>> ...
> >>>
> >>
> >> Another idea is that maybe we could somehow inform ReorderBuffer whethe
On 12/6/23 09:56, Amit Kapila wrote:
> On Tue, Dec 5, 2023 at 10:23 PM Tomas Vondra
> wrote:
>>
>> On 12/5/23 13:17, Amit Kapila wrote:
>>
>>> (b) for transactional
>>> cases, we see overhead due to traversing all the top-level txns and
>>> check the hash table for each one to find whether change
On 12/6/23 11:19, Amit Kapila wrote:
> On Sun, Dec 3, 2023 at 11:56 PM Tomas Vondra
> wrote:
>>
>> On 12/3/23 18:52, Tomas Vondra wrote:
>>> ...
>>>
>>
>> Another idea is that maybe we could somehow inform ReorderBuffer whether
>> the output plugin even is interested in sequences. That'd help with
On 12/6/23 12:05, Dilip Kumar wrote:
> On Wed, Dec 6, 2023 at 3:36 PM Amit Kapila wrote:
>>
>>> Why can't we use the same concept of
>>> SnapBuildDistributeNewCatalogSnapshot(), I mean we keep queuing the
>>> non-transactional changes (have some base snapshot before the first
>>> change), and when
On 12/6/23 10:05, Dilip Kumar wrote:
> On Wed, Dec 6, 2023 at 11:12 AM Dilip Kumar wrote:
>>
>> On Sun, Dec 3, 2023 at 11:22 PM Tomas Vondra
>> wrote:
>>>
>
> I was also wondering what happens if the sequence changes are
> transactional but somehow the snap builder state changes to
> SNAPBUILD_F
On Wed, Dec 6, 2023 at 3:36 PM Amit Kapila wrote:
>
> > Why can't we use the same concept of
> > SnapBuildDistributeNewCatalogSnapshot(), I mean we keep queuing the
> > non-transactional changes (have some base snapshot before the first
> > change), and whenever there is any catalog change, queue
On Sun, Dec 3, 2023 at 11:56 PM Tomas Vondra
wrote:
>
> On 12/3/23 18:52, Tomas Vondra wrote:
> > ...
> >
>
> Another idea is that maybe we could somehow inform ReorderBuffer whether
> the output plugin even is interested in sequences. That'd help with
> cases where we don't even want/need to repl
On Wed, Dec 6, 2023 at 11:12 AM Dilip Kumar wrote:
>
> On Sun, Dec 3, 2023 at 11:22 PM Tomas Vondra
> wrote:
> >
>
> > Some time ago I floated the idea of maybe "queuing" the sequence changes
> > and only replay them on the next commit, somehow. But we did ran into
> > problems with which snapsho
On Wed, Dec 6, 2023 at 11:12 AM Dilip Kumar wrote:
>
> On Sun, Dec 3, 2023 at 11:22 PM Tomas Vondra
> wrote:
> >
I was also wondering what happens if the sequence changes are
transactional but somehow the snap builder state changes to
SNAPBUILD_FULL_SNAPSHOT in between processing of the smgr_dec
On Tue, Dec 5, 2023 at 10:23 PM Tomas Vondra
wrote:
>
> On 12/5/23 13:17, Amit Kapila wrote:
>
> > (b) for transactional
> > cases, we see overhead due to traversing all the top-level txns and
> > check the hash table for each one to find whether change is
> > transactional.
> >
>
> Not really, no
On Sun, Dec 3, 2023 at 11:22 PM Tomas Vondra
wrote:
>
> Some time ago I floated the idea of maybe "queuing" the sequence changes
> and only replay them on the next commit, somehow. But we did ran into
> problems with which snapshot to use, that I didn't know how to solve.
> Maybe we should try ag
On 12/5/23 13:17, Amit Kapila wrote:
> ...
>> I was hopeful the global hash table would be an improvement, but that
>> doesn't seem to be the case. I haven't done much profiling yet, but I'd
>> guess most of the overhead is due to ReorderBufferQueueSequence()
>> starting and aborting a transaction
On Sun, Dec 3, 2023 at 11:22 PM Tomas Vondra
wrote:
>
> Thanks for the script. Are you also measuring the time it takes to
> decode this using test_decoding?
>
> FWIW I did more comprehensive suite of tests over the weekend, with a
> couple more variations. I'm attaching the updated scripts, runni
On 12/3/23 18:52, Tomas Vondra wrote:
> ...
>
> Some time ago I floated the idea of maybe "queuing" the sequence changes
> and only replay them on the next commit, somehow. But we did ran into
> problems with which snapshot to use, that I didn't know how to solve.
> Maybe we should try again. The
Dear Tomas,
> > I did also performance tests (especially case 3). First of all, there are
> > some
> > variants from yours.
> >
> > 1. patch 0002 was reverted because it has an issue. So this test checks
> > whether
> >refactoring around ReorderBufferSequenceIsTransactional seems really
> ne
On 12/1/23 12:08, Hayato Kuroda (Fujitsu) wrote:
> Dear Tomas,
>
>> I did some micro-benchmarking today, trying to identify cases where this
>> would cause unexpected problems, either due to having to maintain all
>> the relfilenodes, or due to having to do hash lookups for every sequence
>> ch
On 11/30/23 12:56, Amit Kapila wrote:
> On Thu, Nov 30, 2023 at 5:28 AM Tomas Vondra
> wrote:
>>
>> 3) "bad case" - small transactions that generate a lot of relfilenodes
>>
>> select alter_sequence();
>>
>> where the function is defined like this (I did create 1000 sequences
>> before the tes
Dear Tomas,
> I did some micro-benchmarking today, trying to identify cases where this
> would cause unexpected problems, either due to having to maintain all
> the relfilenodes, or due to having to do hash lookups for every sequence
> change. But I think it's fine, mostly ...
>
I did also perfor
On Thu, Nov 30, 2023 at 5:28 AM Tomas Vondra
wrote:
>
> 3) "bad case" - small transactions that generate a lot of relfilenodes
>
> select alter_sequence();
>
> where the function is defined like this (I did create 1000 sequences
> before the test):
>
> CREATE OR REPLACE FUNCTION alter_sequence
On Wed, Nov 29, 2023 at 11:45 PM Tomas Vondra
wrote:
>
>
>
> On 11/27/23 23:06, Peter Smith wrote:
> > FWIW, here are some more minor review comments for v20231127-3-0001
> >
> > ==
> > .../replication/logical/reorderbuffer.c
> >
> > 3.
> > + * To decide if a sequence change is transactional
On 11/29/23 15:41, Tomas Vondra wrote:
> ...
>>
>> One thing that worries me about that approach is that it can suck with
>> the workload that has a lot of DDLs that create XLOG_SMGR_CREATE
>> records. We have previously fixed some such workloads in logical
>> decoding where decoding a transaction
On 11/29/23 14:42, Amit Kapila wrote:
> On Wed, Nov 29, 2023 at 2:59 AM Tomas Vondra
> wrote:
>>
>> I have been hacking on improving the improvements outlined in my
>> preceding e-mail, but I have some bad news - I ran into an issue that I
>> don't know how to solve :-(
>>
>> Consider this transac
On Wed, Nov 29, 2023 at 2:59 AM Tomas Vondra
wrote:
>
> I have been hacking on improving the improvements outlined in my
> preceding e-mail, but I have some bad news - I ran into an issue that I
> don't know how to solve :-(
>
> Consider this transaction:
>
> BEGIN;
> ALTER SEQUENCE s RESTART
On 11/27/23 23:06, Peter Smith wrote:
> FWIW, here are some more minor review comments for v20231127-3-0001
>
> ==
> doc/src/sgml/logicaldecoding.sgml
>
> 1.
> + The txn parameter contains meta information
> about
> + the transaction the sequence change is part of. Note however
On 11/28/23 12:32, Amit Kapila wrote:
> On Mon, Nov 27, 2023 at 11:45 PM Tomas Vondra
> wrote:
>>
>> I spent a bit of time looking at the proposed change, and unfortunately
>> logging just the boolean flag does not work. A good example is this bit
>> from a TAP test added by the patch for built-in
On Mon, Nov 27, 2023 at 11:45 PM Tomas Vondra
wrote:
>
> I spent a bit of time looking at the proposed change, and unfortunately
> logging just the boolean flag does not work. A good example is this bit
> from a TAP test added by the patch for built-in replication (which was
> not included with th
FWIW, here are some more minor review comments for v20231127-3-0001
==
doc/src/sgml/logicaldecoding.sgml
1.
+ The txn parameter contains meta information about
+ the transaction the sequence change is part of. Note however that for
+ non-transactional updates, the transaction m
On 11/27/23 13:08, Hayato Kuroda (Fujitsu) wrote:
> Dear Amit, Tomas,
>
I am wondering that instead of building the infrastructure to know
whether a particular change is transactional on the decoding side,
can't we have some flag in the WAL record to note whether the change
>>>
On 11/27/23 12:11, Amit Kapila wrote:
> On Mon, Nov 27, 2023 at 4:17 PM Tomas Vondra
> wrote:
>>
>> On 11/27/23 11:13, Amit Kapila wrote:
>>> On Mon, Nov 27, 2023 at 11:34 AM Amit Kapila
>>> wrote:
On Mon, Nov 27, 2023 at 6:41 AM Tomas Vondra
wrote:
>
> While going ove
On Mon, Nov 27, 2023 at 4:41 PM Amit Kapila wrote:
>
> On Mon, Nov 27, 2023 at 4:17 PM Tomas Vondra
> wrote:
> >
>
> > FWIW I think one of the earlier patch versions did something like this,
> > by adding a "created" flag in the xlog record. And we concluded doing
> > this on the decoding side is
On Mon, Nov 27, 2023 at 4:17 PM Tomas Vondra
wrote:
>
> On 11/27/23 11:13, Amit Kapila wrote:
> > On Mon, Nov 27, 2023 at 11:34 AM Amit Kapila
> > wrote:
> >>
> >> On Mon, Nov 27, 2023 at 6:41 AM Tomas Vondra
> >> wrote:
> >>>
> >>> While going over 0001, I realized there might be an optimizati
On 11/27/23 11:13, Amit Kapila wrote:
> On Mon, Nov 27, 2023 at 11:34 AM Amit Kapila wrote:
>>
>> On Mon, Nov 27, 2023 at 6:41 AM Tomas Vondra
>> wrote:
>>>
>>> While going over 0001, I realized there might be an optimization for
>>> ReorderBufferSequenceIsTransactional. As coded in 0001, it a
On Mon, Nov 27, 2023 at 11:34 AM Amit Kapila wrote:
>
> On Mon, Nov 27, 2023 at 6:41 AM Tomas Vondra
> wrote:
> >
> > While going over 0001, I realized there might be an optimization for
> > ReorderBufferSequenceIsTransactional. As coded in 0001, it always
> > searches through all top-level trans
On Mon, Nov 27, 2023 at 6:41 AM Tomas Vondra
wrote:
>
> I've been cleaning up the first two patches to get them committed soon
> (adding the decoding infrastructure + test_decoding), cleaning up stale
> comments, updating commit messages etc. And I think it's ready to go,
> but it's too late over,
On Thursday, October 12, 2023 11:06 PM Tomas Vondra
wrote:
>
Hi,
I have been reviewing the patch set, and here are some initial comments.
1.
I think we need to mark the RBTXN_HAS_STREAMABLE_CHANGE flag for transactional
sequence change in ReorderBufferQueueChange().
2.
ReorderBufferSequence
On Thu, Oct 12, 2023 at 9:03 PM Tomas Vondra
wrote:
>
> On 7/25/23 12:20, Amit Kapila wrote:
> > ...
> >
> > I have used the debugger to reproduce this as it needs quite some
> > coordination. I just wanted to see if the sequence can go backward and
> > didn't catch up completely before the sequen
On 7/25/23 12:20, Amit Kapila wrote:
> ...
>
> I have used the debugger to reproduce this as it needs quite some
> coordination. I just wanted to see if the sequence can go backward and
> didn't catch up completely before the sequence state is marked
> 'ready'. On the publisher side, I created a pu
On 9/13/23 15:18, Ashutosh Bapat wrote:
> On Fri, Aug 18, 2023 at 4:28 PM Amit Kapila wrote:
>>
>> On Fri, Aug 18, 2023 at 10:37 AM Amit Kapila wrote:
>>>
>>> On Thu, Aug 17, 2023 at 7:13 PM Ashutosh Bapat
>>> wrote:
On Wed, Aug 16, 2023 at 7:56 PM Tomas Vondra
wrote:
>
>>
On 9/20/23 11:53, Dilip Kumar wrote:
> On Wed, Aug 16, 2023 at 7:57 PM Tomas Vondra
> wrote:
>>
>
> I was reading through 0001, I noticed this comment in
> ReorderBufferSequenceIsTransactional() function
>
> + * To decide if a sequence change should be handled as transactional or
> applied
>
On Friday, September 15, 2023 11:11 AM Zhijie Hou (Fujitsu)
wrote:
>
> On Wednesday, August 16, 2023 10:27 PM Tomas Vondra
> wrote:
>
> Hi,
>
> >
> >
> > I guess we could update the origin, per attached 0004. We don't have
> > timestamp to set replorigin_session_origin_timestamp, but it seems
On Wed, Sep 20, 2023 at 3:23 PM Dilip Kumar wrote:
>
> On Wed, Aug 16, 2023 at 7:57 PM Tomas Vondra
> wrote:
> >
>
> I was reading through 0001, I noticed this comment in
> ReorderBufferSequenceIsTransactional() function
>
> + * To decide if a sequence change should be handled as transactional or
On Wed, Aug 16, 2023 at 7:57 PM Tomas Vondra
wrote:
>
I was reading through 0001, I noticed this comment in
ReorderBufferSequenceIsTransactional() function
+ * To decide if a sequence change should be handled as transactional or applied
+ * immediately, we track (sequence) relfilenodes created b
On Wednesday, August 16, 2023 10:27 PM Tomas Vondra
wrote:
Hi,
>
>
> I guess we could update the origin, per attached 0004. We don't have
> timestamp to set replorigin_session_origin_timestamp, but it seems we don't
> need that.
>
> The attached patch merges the earlier improvements, except
On Fri, Aug 18, 2023 at 4:28 PM Amit Kapila wrote:
>
> On Fri, Aug 18, 2023 at 10:37 AM Amit Kapila wrote:
> >
> > On Thu, Aug 17, 2023 at 7:13 PM Ashutosh Bapat
> > wrote:
> > >
> > > On Wed, Aug 16, 2023 at 7:56 PM Tomas Vondra
> > > wrote:
> > > >
> > > > >
> > > > > But whether or not that'
On Thu, Aug 17, 2023 at 7:13 PM Ashutosh Bapat
wrote:
> >
> > The attached patch merges the earlier improvements, except for the part
> > that experimented with adding a "fake" transaction (which turned out to
> > have a number of difficult issues).
>
> 0004 looks good to me. But I need to review
On Fri, Aug 18, 2023 at 10:37 AM Amit Kapila wrote:
>
> On Thu, Aug 17, 2023 at 7:13 PM Ashutosh Bapat
> wrote:
> >
> > On Wed, Aug 16, 2023 at 7:56 PM Tomas Vondra
> > wrote:
> > >
> > > >
> > > > But whether or not that's the case, downstream should not request (and
> > > > hence receive) any
On Thu, Aug 17, 2023 at 7:13 PM Ashutosh Bapat
wrote:
>
> On Wed, Aug 16, 2023 at 7:56 PM Tomas Vondra
> wrote:
> >
> > >
> > > But whether or not that's the case, downstream should not request (and
> > > hence receive) any changes that have been already applied (and
> > > committed) downstream a
On Wed, Aug 16, 2023 at 7:56 PM Tomas Vondra
wrote:
>
> >
> > But whether or not that's the case, downstream should not request (and
> > hence receive) any changes that have been already applied (and
> > committed) downstream as a principle. I think a way to achieve this is
> > to update the replo
On Tue, Aug 1, 2023 at 8:46 PM Tomas Vondra
wrote:
>
> Anyway, I think this is "just" a matter of efficiency, not correctness.
> IMHO there are bigger questions regarding the "going back" behavior
> after apply restart.
sequence_decode() has the following code
/* Skip the change if already proce
On 8/1/23 04:59, Amit Kapila wrote:
> On Mon, Jul 31, 2023 at 5:04 PM Tomas Vondra
> wrote:
>>
>> On 7/31/23 11:25, Amit Kapila wrote:
>>> ...
>>>
>>> Yeah, I also think this needs a review. This is a sort of new concept
>>> where we don't use the LSN of the slot (for cases where copy returned
On Mon, Jul 31, 2023 at 5:04 PM Tomas Vondra
wrote:
>
> On 7/31/23 11:25, Amit Kapila wrote:
> > On Sat, Jul 29, 2023 at 5:53 PM Tomas Vondra
> > wrote:
> >>
> >> On 7/28/23 14:44, Ashutosh Bapat wrote:
> >>> On Wed, Jul 26, 2023 at 8:48 PM Tomas Vondra
> >>> wrote:
>
> Anyway, I was t
On 7/31/23 11:25, Amit Kapila wrote:
> On Sat, Jul 29, 2023 at 5:53 PM Tomas Vondra
> wrote:
>>
>> On 7/28/23 14:44, Ashutosh Bapat wrote:
>>> On Wed, Jul 26, 2023 at 8:48 PM Tomas Vondra
>>> wrote:
Anyway, I was thinking about this a bit more, and it seems it's not as
difficult
On Sat, Jul 29, 2023 at 5:53 PM Tomas Vondra
wrote:
>
> On 7/28/23 14:44, Ashutosh Bapat wrote:
> > On Wed, Jul 26, 2023 at 8:48 PM Tomas Vondra
> > wrote:
> >>
> >> Anyway, I was thinking about this a bit more, and it seems it's not as
> >> difficult to use the page LSN to ensure sequences don't
On 7/29/23 06:54, Amit Kapila wrote:
> On Fri, Jul 28, 2023 at 6:12 PM Tomas Vondra
> wrote:
>>
>> On 7/28/23 11:42, Amit Kapila wrote:
>>> On Wed, Jul 26, 2023 at 8:48 PM Tomas Vondra
>>> wrote:
On 7/26/23 09:27, Amit Kapila wrote:
> On Wed, Jul 26, 2023 at 9:37 AM Amit Kapila
On 7/28/23 14:44, Ashutosh Bapat wrote:
> On Wed, Jul 26, 2023 at 8:48 PM Tomas Vondra
> wrote:
>>
>> Anyway, I was thinking about this a bit more, and it seems it's not as
>> difficult to use the page LSN to ensure sequences don't go backwards.
>> The 0005 change does that, by:
>>
>> 1) adding
On Fri, Jul 28, 2023 at 6:12 PM Tomas Vondra
wrote:
>
> On 7/28/23 11:42, Amit Kapila wrote:
> > On Wed, Jul 26, 2023 at 8:48 PM Tomas Vondra
> > wrote:
> >>
> >> On 7/26/23 09:27, Amit Kapila wrote:
> >>> On Wed, Jul 26, 2023 at 9:37 AM Amit Kapila
> >>> wrote:
> >>
> >> Anyway, I was thinking
On 7/28/23 14:35, Ashutosh Bapat wrote:
> On Tue, Jul 25, 2023 at 10:02 PM Tomas Vondra
> wrote:
>>
>> On 7/24/23 14:57, Ashutosh Bapat wrote:
>>> ...
>>>
2) Currently, the sequences hash table is in reorderbuffer, i.e. global.
I was thinking maybe we should have it in the t
On Wed, Jul 26, 2023 at 8:48 PM Tomas Vondra
wrote:
>
> Anyway, I was thinking about this a bit more, and it seems it's not as
> difficult to use the page LSN to ensure sequences don't go backwards.
> The 0005 change does that, by:
>
> 1) adding pg_sequence_state, that returns both the sequence st
On 7/28/23 11:42, Amit Kapila wrote:
> On Wed, Jul 26, 2023 at 8:48 PM Tomas Vondra
> wrote:
>>
>> On 7/26/23 09:27, Amit Kapila wrote:
>>> On Wed, Jul 26, 2023 at 9:37 AM Amit Kapila wrote:
>>
>> Anyway, I was thinking about this a bit more, and it seems it's not as
>> difficult to use the page
On Tue, Jul 25, 2023 at 10:02 PM Tomas Vondra
wrote:
>
> On 7/24/23 14:57, Ashutosh Bapat wrote:
> > ...
> >
> >>
> >>
> >> 2) Currently, the sequences hash table is in reorderbuffer, i.e. global.
> >> I was thinking maybe we should have it in the transaction (because we
> >> need to do cleanup at
1 - 100 of 320 matches
Mail list logo
