Re: logical decoding and replication of sequences, take 2

2024-03-06 Thread Tomas Vondra
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

Re: logical decoding and replication of sequences, take 2

2024-02-21 Thread Dilip Kumar
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

Re: logical decoding and replication of sequences, take 2

2024-02-21 Thread Robert Haas
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 ...

Re: logical decoding and replication of sequences, take 2

2024-02-21 Thread Dilip Kumar
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: logical decoding and replication of sequences, take 2

2024-02-20 Thread Robert Haas
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

Re: logical decoding and replication of sequences, take 2

2024-02-20 Thread Dilip Kumar
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

Re: logical decoding and replication of sequences, take 2

2024-02-20 Thread Amit Kapila
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

Re: logical decoding and replication of sequences, take 2

2024-02-20 Thread Amit Kapila
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

Re: logical decoding and replication of sequences, take 2

2024-02-20 Thread Tomas Vondra
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

Re: logical decoding and replication of sequences, take 2

2024-02-20 Thread Dilip Kumar
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

Re: logical decoding and replication of sequences, take 2

2024-02-20 Thread Robert Haas
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] >

Re: logical decoding and replication of sequences, take 2

2024-02-20 Thread Dilip Kumar
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

Re: logical decoding and replication of sequences, take 2

2024-02-19 Thread Amit Kapila
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

Re: logical decoding and replication of sequences, take 2

2024-02-19 Thread Robert Haas
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

Re: logical decoding and replication of sequences, take 2

2024-02-15 Thread Tomas Vondra
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. >

Re: logical decoding and replication of sequences, take 2

2024-02-14 Thread Robert Haas
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

Re: logical decoding and replication of sequences, take 2

2024-02-14 Thread Tomas Vondra
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 >

Re: logical decoding and replication of sequences, take 2

2024-02-13 Thread Robert Haas
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

Re: logical decoding and replication of sequences, take 2

2024-01-27 Thread Tomas Vondra
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) >> >>

Re: logical decoding and replication of sequences, take 2

2024-01-26 Thread Robert Haas
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

Re: logical decoding and replication of sequences, take 2

2024-01-24 Thread Tomas Vondra
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

Re: logical decoding and replication of sequences, take 2

2024-01-23 Thread Robert Haas
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

Re: logical decoding and replication of sequences, take 2

2023-12-21 Thread Tomas Vondra
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

Re: logical decoding and replication of sequences, take 2

2023-12-21 Thread Tomas Vondra
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)

Re: logical decoding and replication of sequences, take 2

2023-12-19 Thread Christophe Pettus
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

Re: logical decoding and replication of sequences, take 2

2023-12-14 Thread Amit Kapila
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

Re: logical decoding and replication of sequences, take 2

2023-12-14 Thread Euler Taveira
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/

Re: logical decoding and replication of sequences, take 2

2023-12-14 Thread Ashutosh Bapat
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

Re: logical decoding and replication of sequences, take 2

2023-12-14 Thread Amit Kapila
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

Re: logical decoding and replication of sequences, take 2

2023-12-14 Thread Ashutosh Bapat
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.

Re: logical decoding and replication of sequences, take 2

2023-12-14 Thread Amit Kapila
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

Re: logical decoding and replication of sequences, take 2

2023-12-13 Thread Dilip Kumar
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

Re: logical decoding and replication of sequences, take 2

2023-12-13 Thread Ashutosh Bapat
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

Re: logical decoding and replication of sequences, take 2

2023-12-13 Thread Dilip Kumar
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

RE: logical decoding and replication of sequences, take 2

2023-12-13 Thread Hayato Kuroda (Fujitsu)
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

Re: logical decoding and replication of sequences, take 2

2023-12-13 Thread Amit Kapila
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)

Re: logical decoding and replication of sequences, take 2

2023-12-12 Thread Tomas Vondra
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

Re: logical decoding and replication of sequences, take 2

2023-12-06 Thread Dilip Kumar
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

Re: logical decoding and replication of sequences, take 2

2023-12-06 Thread Amit Kapila
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

Re: logical decoding and replication of sequences, take 2

2023-12-06 Thread Dilip Kumar
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

Re: logical decoding and replication of sequences, take 2

2023-12-06 Thread Amit Kapila
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

Re: logical decoding and replication of sequences, take 2

2023-12-06 Thread Tomas Vondra
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

Re: logical decoding and replication of sequences, take 2

2023-12-06 Thread Tomas Vondra
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

Re: logical decoding and replication of sequences, take 2

2023-12-06 Thread Tomas Vondra
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

Re: logical decoding and replication of sequences, take 2

2023-12-06 Thread Tomas Vondra
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 >

Re: logical decoding and replication of sequences, take 2

2023-12-06 Thread Dilip Kumar
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

Re: logical decoding and replication of sequences, take 2

2023-12-06 Thread Amit Kapila
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

Re: logical decoding and replication of sequences, take 2

2023-12-06 Thread Amit Kapila
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

Re: logical decoding and replication of sequences, take 2

2023-12-06 Thread Dilip Kumar
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

Re: logical decoding and replication of sequences, take 2

2023-12-06 Thread Amit Kapila
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,

Re: logical decoding and replication of sequences, take 2

2023-12-05 Thread Dilip Kumar
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

Re: logical decoding and replication of sequences, take 2

2023-12-05 Thread Tomas Vondra
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

Re: logical decoding and replication of sequences, take 2

2023-12-05 Thread Amit Kapila
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,

Re: logical decoding and replication of sequences, take 2

2023-12-03 Thread Tomas Vondra
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

RE: logical decoding and replication of sequences, take 2

2023-12-03 Thread Hayato Kuroda (Fujitsu)
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 >

Re: logical decoding and replication of sequences, take 2

2023-12-01 Thread Tomas Vondra
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 >>

Re: logical decoding and replication of sequences, take 2

2023-12-01 Thread Tomas Vondra
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

RE: logical decoding and replication of sequences, take 2

2023-12-01 Thread Hayato Kuroda (Fujitsu)
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

Re: logical decoding and replication of sequences, take 2

2023-11-30 Thread Amit Kapila
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

Re: logical decoding and replication of sequences, take 2

2023-11-29 Thread Peter Smith
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

Re: logical decoding and replication of sequences, take 2

2023-11-29 Thread Tomas Vondra
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

Re: logical decoding and replication of sequences, take 2

2023-11-29 Thread Tomas Vondra
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

Re: logical decoding and replication of sequences, take 2

2023-11-29 Thread Amit Kapila
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

Re: logical decoding and replication of sequences, take 2

2023-11-29 Thread Tomas Vondra
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

Re: logical decoding and replication of sequences, take 2

2023-11-28 Thread Tomas Vondra
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

Re: logical decoding and replication of sequences, take 2

2023-11-28 Thread Amit Kapila
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

Re: logical decoding and replication of sequences, take 2

2023-11-27 Thread Peter Smith
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

Re: logical decoding and replication of sequences, take 2

2023-11-27 Thread Tomas Vondra
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

Re: logical decoding and replication of sequences, take 2

2023-11-27 Thread Tomas Vondra
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

Re: logical decoding and replication of sequences, take 2

2023-11-27 Thread Amit Kapila
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

Re: logical decoding and replication of sequences, take 2

2023-11-27 Thread Amit Kapila
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

Re: logical decoding and replication of sequences, take 2

2023-11-27 Thread Tomas Vondra
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

Re: logical decoding and replication of sequences, take 2

2023-11-27 Thread Amit Kapila
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

Re: logical decoding and replication of sequences, take 2

2023-11-26 Thread Amit Kapila
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

RE: logical decoding and replication of sequences, take 2

2023-10-24 Thread Zhijie Hou (Fujitsu)
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.

Re: logical decoding and replication of sequences, take 2

2023-10-14 Thread Amit Kapila
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

Re: logical decoding and replication of sequences, take 2

2023-10-12 Thread Tomas Vondra
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

Re: logical decoding and replication of sequences, take 2

2023-10-12 Thread Tomas Vondra
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: >

Re: logical decoding and replication of sequences, take 2

2023-10-12 Thread Tomas Vondra
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

RE: logical decoding and replication of sequences, take 2

2023-09-25 Thread Zhijie Hou (Fujitsu)
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

Re: logical decoding and replication of sequences, take 2

2023-09-22 Thread Dilip Kumar
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

Re: logical decoding and replication of sequences, take 2

2023-09-20 Thread Dilip Kumar
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

RE: logical decoding and replication of sequences, take 2

2023-09-14 Thread Zhijie Hou (Fujitsu)
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

Re: logical decoding and replication of sequences, take 2

2023-09-13 Thread Ashutosh Bapat
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

Re: logical decoding and replication of sequences, take 2

2023-09-13 Thread Ashutosh Bapat
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

Re: logical decoding and replication of sequences, take 2

2023-08-18 Thread Amit Kapila
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

Re: logical decoding and replication of sequences, take 2

2023-08-17 Thread Amit Kapila
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

Re: logical decoding and replication of sequences, take 2

2023-08-17 Thread Ashutosh Bapat
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

Re: logical decoding and replication of sequences, take 2

2023-08-11 Thread Ashutosh Bapat
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

Re: logical decoding and replication of sequences, take 2

2023-08-01 Thread Tomas Vondra
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

Re: logical decoding and replication of sequences, take 2

2023-07-31 Thread Amit Kapila
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

Re: logical decoding and replication of sequences, take 2

2023-07-31 Thread Tomas Vondra
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

Re: logical decoding and replication of sequences, take 2

2023-07-31 Thread Amit Kapila
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

Re: logical decoding and replication of sequences, take 2

2023-07-29 Thread Tomas Vondra
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

Re: logical decoding and replication of sequences, take 2

2023-07-29 Thread Tomas Vondra
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)

Re: logical decoding and replication of sequences, take 2

2023-07-28 Thread Amit Kapila
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

Re: logical decoding and replication of sequences, take 2

2023-07-28 Thread Tomas Vondra
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

Re: logical decoding and replication of sequences, take 2

2023-07-28 Thread Ashutosh Bapat
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

Re: logical decoding and replication of sequences, take 2

2023-07-28 Thread Tomas Vondra
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

Re: logical decoding and replication of sequences, take 2

2023-07-28 Thread Ashutosh Bapat
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

  1   2   3   4   >