Hi, Sorry for the late reply.
On Wed, Aug 12, 2026 at 12:16 AM torikoshia <[email protected]> wrote: > > On 2026-08-11 09:04, Masahiko Sawada wrote: > > Thanks for the comment! > > I'm still studying this patch but I want to clarify: was the problem > > you faced caused by logical decoding showing different types of > > changes(insert/update/delete/truncate/message) with the same LSN? > > Yes. The Debezium issue I encountered was caused by different types of > changes being reported with the same LSN. > > More specifically, consider a transaction like this: > > begin; > select pg_logical_emit_message(true, 'test1', 'xxx'); > insert into t1 values (1, 'aaa'); > commit; > > The problem occurred when Debezium received the logical message but > crashed before receiving the INSERT. > > After a restart, Debezium uses the LSN of the last record it received > to determine where to resume and which records have already been > processed. Since the INSERT is reported with the same LSN as the > logical message, it can be treated as already processed and skipped, > resulting in data loss. > > > IIUC > > logical decoding of MULTI_INSERT emits all INSERT changes with the > > same LSN, so I think showing the same LSN multiple times is fine. > > So I agree that having the same LSN for multiple changes is not in > itself a problem. The problem in this case is that changes originating > from different WAL positions can be reported with the same LSN. > > > One possible reason why the message uses endptr instead of origptr is > > for non-transactional messages; on receiver sides a non-transactional > > message would be handled as a separate transaction, so it would be > > useful to use endptr as the confirmed flush position. That said, it > > doesn't apply for transactional messages. > > > While I agree that logical decoding uses origptr of logical decoding > > messages as their LSN, I think having its endptr is also useful for > > the above reason. For instance, I proposed to extend logical decoding > > message handling[1] so that extension can define a function to handle > > logical decoding messages on the subscriber. If we use origptr as the > > flushed position, the same message is replicated again after the > > server restart even if the subscriber has committed the message as a > > separate transaction and sent an ack to the publisher. > > That makes sense. > > > So an alternative idea would be to use origptr for transactional > > messages and endptr for non-transactional ones. With this idea, a > > transactional message's LSN could coincide with the LSN reported for > > the preceding COMMIT, or for a preceding non-transactional message. I > > think that is acceptable for the same reason as MULTI_INSERT above: > > what matters is that the LSN identifies the record the change came > > from, not that it is unique. > > That approach seems reasonable to me. > > > I'm hesitant to backpatch it. What this issue tells us is that > > consumers do look at the LSN of individual changes and possibly > > persist it, and use it to decide where to resume and what has already > > been processed. If we change the LSN we report in a minor release, a > > position that a consumer recorded under the old behavior will be > > interpreted under the new one after the upgrade. Whether that ends up > > re-processing changes, skipping them, or failing to locate the resume > > point depends on the consumer, and none of those seems like something > > an operator should have to expect from a minor upgrade. > > > > There is no correctness problem within PostgreSQL here. So I think > > this is pushed to master only, with the current behavior documented in > > the back branches. Consumers hitting this can handle it on their side > > in the meantime. > > This may be a safer approach. > > One thing that still concerns me is that consumer developers may > assume that the LSN reported for a logical message identifies > the WAL record containing that message, just as it does for other > decoded changes. Even after consulting the documentation, one > could reach that conclusion from this description: > > https://www.postgresql.org/docs/devel/protocol-logicalrep-message-formats.html > > > Message > > .... > > Int64 (XLogRecPtr) > > The LSN of the logical decoding message. > > Also, as far as I imagine, the LSN of an individual decoded change is > typically relevant mainly when a consumer has to determine its resume > position after an unexpected interruption. Therefore, there may be > consumers for which this behavior has not caused a visible problem > during normal replication, and whose developers are not aware of this > difference for logical messages. > > If that assumption is common, treating the documentation as defining > the intended behavior and backpatching the change would make the fix > easier for consumers: they could get the corrected behavior simply by > upgrading PostgreSQL, without adding special handling for logical > message LSNs only for PostgreSQL versions before 19. > > That said, I don't know whether there are actually many such consumers. > It seems difficult to predict how changing the behavior in already > released major versions might affect existing consumers. > From that perspective, leaving the behavior unchanged in released > branches seems safer. > > So I'm fine with changing this only in master and documenting the > existing behavior in the back branches. Yeah, backpatching has another risk too: a consumer that has persisted the LSNs of decoded messages would find them meaning something different after a minor version update, and would have to behave differently depending on the publisher's minor version. I don't think we can expect consumers to handle that. > For the released branches, though, I think it would be useful not only > to update the documentation, but also to make the issue clearly visible > in the release notes, since some consumers may need to review or adjust > their logics. Agreed. I'll describe the behavior difference in the commit message so that it's picked up when the release notes are written. Regarding the patch: - ReorderBufferQueueMessage(ctx->reorder, xid, snapshot, buf->endptr, + /* + * Non-transactional messages are processed as separate transactions on + * the receiver, so use endptr as the confirmed flush position for them. + */ + ReorderBufferQueueMessage(ctx->reorder, xid, snapshot, + message->transactional ? buf->origptr : buf->endptr, message->transactional, I think this needs more comments: what we assume on the consumer side (a non-transactional message is applied and confirmed as a unit of its own, like a commit), and what happens if we used origptr for a non-transactional message instead; a consumer that reports the message's LSN as its flush position can get the message decoded again after a restart, since decoding resumes from the confirmed position inclusive. It's the same reason why the commit message carries both commit_lsn and end_lsn. Also, logicaldecoding.sgml says "The lsn has WAL location of the message" for message_cb, which needs the same update, in the back-branch patches too. Regards, -- Masahiko Sawada Amazon Web Services: https://aws.amazon.com
