Hi, On 2022-11-17 22:13:23 +0100, Tomas Vondra wrote: > On 11/17/22 18:07, Andres Freund wrote: > > On 2022-11-17 12:39:49 +0100, Tomas Vondra wrote: > >> On 11/17/22 03:43, Andres Freund wrote: > >>> I assume that part of the initial sync would have to be a new sequence > >>> synchronization step that reads all the sequence states on the publisher > >>> and > >>> ensures that the subscriber sequences are at the same point. There's a > >>> bit of > >>> trickiness there, but it seems entirely doable. The logical replication > >>> replay > >>> support for sequences will have to be a bit careful about not decreasing > >>> the > >>> subscriber's sequence values - the standby initially will be ahead of the > >>> increments we'll see in the WAL. But that seems inevitable given the > >>> non-transactional nature of sequences. > >>> > >> > >> See fetch_sequence_data / copy_sequence in the patch. The bit about > >> ensuring the sequence does not go away (say, using page LSN and/or LSN > >> of the increment) is not there, however isn't that pretty much what I > >> proposed doing for "reconciling" the sequence state logged at COMMIT? > > > > Well, I think the approach of logging all sequence increments at commit is > > the > > wrong idea... > > > > But we're not logging all sequence increments, no?
I was imprecise - I meant streaming them out at commit. > Yeah, I think you're right. I looked at this again, with fresh mind, and > I came to the same conclusion. Roughly what the attached patch does. To me it seems a bit nicer to keep the SnapBuildGetOrBuildSnapshot() call in decode.c instead of moving it to reorderbuffer.c. Perhaps we should add a snapbuild.c helper similar to SnapBuildProcessChange() for non-transactional changes that also gets a snapshot? Could look something like Snapshot snapshot = NULL; if (message->transactional && !SnapBuildProcessChange(builder, xid, buf->origptr)) return; else if (!SnapBuildProcessStateNonTx(builder, &snapshot)) return; ... Or perhaps we should just bite the bullet and add an argument to SnapBuildProcessChange to deal with that? Greetings, Andres Freund