BUG #19616: pgoutput sends stream abort ('A') to clients that did not enable streaming
Hackorum builds and tests every patch posted to the lists, not only commitfest submissions. This is Hackorum's own CI rather than the PostgreSQL project's, and it is still under testing - please report anything that looks wrong.
This thread has been committed, so CI has stopped here. Anything below is the last result it produced.
You can run a PostgreSQL built from this patch straight from Docker, with no checkout and no build:
docker run --rm -p 5432:5432 ghcr.io/hackorum-dev/postgres-patch:t253401psql -h localhost -U postgresBuilt from patchset v13 (message #13), August 17, 2026 at 08:57 PM.
Every patchset is also pushed to a branch of our PostgreSQL fork, so you can check out the same tree CI built. Without a PostgreSQL checkout:
git clone --branch t253401_13 https://github.com/hackorum-dev/postgres.gitIn a checkout you already have, add the fork once:
git remote add hackorum https://github.com/hackorum-dev/postgres.gitthen, for this patchset and every later one:
git fetch hackorum t253401_13 && git checkout t253401_13Patchset v13 (message #13) is on t253401_13
The following bug has been logged on the website:
Bug reference: 19616
Logged by: Tyler Smart
Email address: tyler@smarts.io
PostgreSQL version: 18.4
Operating system: Linux (Docker; also Google Cloud SQL)
Description:
Since PostgreSQL 18, pgoutput can send a Stream Abort ('A') message to a
client that connected with proto_version 1 and never enabled streaming.
Protocol 1 clients do not implement the stream message set, so consumers
fail on it: Debezium (and tools that embed it, like Airbyte) dies with
"Unsupported message type: A", and because the crash repeats at the same WAL
position on every restart, the slot stops advancing until
max_slot_wal_keep_size invalidates it.
I reproduced this on 18.4 (Debian 18.4-1.pgdg13+1, official Docker image)
and on Cloud SQL 18.1. The identical script does not reproduce it on 17.10.
The trigger needs three conditions in one decode run:
1. logical_decoding_work_mem is exceeded, so eviction runs.
2. The eviction candidate has already aborted in clog.
3. That transaction has a subtransaction with changes still in memory.
From reading REL_18_STABLE, the cause appears to be commit 072ee847ad4
("Skip logical decoding of already-aborted transactions"). It added
ReorderBufferCheckAndTruncateAbortedTXN, which discards aborted transactions
at eviction time via ReorderBufferTruncateTXN. That function marks every
subtransaction that still has in-memory changes as streamed
(ReorderBufferMaybeMarkTXNStreamed, in the subtxn loop near
reorderbuffer.c:1675) without checking whether the connection streams at
all. The top-level transaction is handled correctly, since its marking
happens at call sites guarded by the streaming flag. Only the subtransaction
marking is unconditional.
When decoding later reaches the abort record, ReorderBufferAbort sees
rbtxn_is_streamed on the subtransaction and invokes the stream_abort
callback (near reorderbuffer.c:3092). pgoutput_stream_abort is guarded only
by assertions, so production builds write 'A' onto a proto_version 1
stream. I expect a cassert build to fail Assert(rbtxn_is_streamed(toptxn))
there instead, since the top-level transaction is not marked, though I have
not verified that.
client that connected with proto_version 1 and never enabled streaming.
Protocol 1 clients do not implement the stream message set, so consumers
fail on it: Debezium (and tools that embed it, like Airbyte) dies with
"Unsupported message type: A", and because the crash repeats at the same WAL
position on every restart, the slot stops advancing until
max_slot_wal_keep_size invalidates it.
I reproduced this on 18.4 (Debian 18.4-1.pgdg13+1, official Docker image)
and on Cloud SQL 18.1. The identical script does not reproduce it on 17.10.
The trigger needs three conditions in one decode run:
1. logical_decoding_work_mem is exceeded, so eviction runs.
2. The eviction candidate has already aborted in clog.
3. That transaction has a subtransaction with changes still in memory.
From reading REL_18_STABLE, the cause appears to be commit 072ee847ad4
("Skip logical decoding of already-aborted transactions"). It added
ReorderBufferCheckAndTruncateAbortedTXN, which discards aborted transactions
at eviction time via ReorderBufferTruncateTXN. That function marks every
subtransaction that still has in-memory changes as streamed
(ReorderBufferMaybeMarkTXNStreamed, in the subtxn loop near
reorderbuffer.c:1675) without checking whether the connection streams at
all. The top-level transaction is handled correctly, since its marking
happens at call sites guarded by the streaming flag. Only the subtransaction
marking is unconditional.
When decoding later reaches the abort record, ReorderBufferAbort sees
rbtxn_is_streamed on the subtransaction and invokes the stream_abort
callback (near reorderbuffer.c:3092). pgoutput_stream_abort is guarded only
by assertions, so production builds write 'A' onto a proto_version 1
stream. I expect a cassert build to fail Assert(rbtxn_is_streamed(toptxn))
there instead, since the top-level transaction is not marked, though I have
not verified that.
Self-contained reproduction (the SQL decoding interface acts as a
non-streaming client, so no replication client is needed):
docker run -d -e POSTGRES_PASSWORD=pw postgres:18 -c wal_level=logical
CREATE TABLE t(id int, filler text);
CREATE PUBLICATION pub FOR TABLE t;
SELECT pg_create_logical_replication_slot('s', 'pgoutput');
BEGIN;
SAVEPOINT sp;
INSERT INTO t VALUES (0, 'subtransaction-change');
RELEASE SAVEPOINT sp;
INSERT INTO t SELECT g, repeat('x', 1000) FROM generate_series(1, 5000) g;
ROLLBACK;
INSERT INTO t VALUES (1, 'after');
SET logical_decoding_work_mem = '64kB';
SELECT chr(get_byte(data,0)) AS msgtype, count(*)
FROM pg_logical_slot_peek_binary_changes('s', NULL, NULL,
'proto_version','1','publication_names','pub')
GROUP BY 1 ORDER BY 2 DESC;
Actual output on 18.4:
msgtype | count
---------+-------
B | 1
R | 1
C | 1
I | 1
A | 1
Expected: no A row. A proto_version 1 client must never receive stream
messages, and an aborted transaction should produce no output at all.
PG 17.10 produces the expected output with the same script, as does 18.4
when logical_decoding_work_mem is raised enough that eviction never fires.
Impact: any protocol 1 consumer on a busy PG 18 server can hit this with a
single canceled or deadlocked transaction that used savepoints, decoded
while the buffer is past logical_decoding_work_mem. We hit it in production
through Debezium, where the retry loop pinned the slot until Postgres
invalidated it. Raising logical_decoding_work_mem only lowers the
probability.
Suggested direction: ReorderBufferTruncateTXN should mark subtransactions
as streamed only when truncating on behalf of streaming, the same way the
top-level marking is already gated, or the abort-discard path should skip
the marking entirely.
Hi, Tyler!
Thanks for the report and analysis.
ReorderBufferTruncateTXN is used both after streaming and when discarding
already-aborted transactions at eviction. Streaming callers mark the
top-level xact before truncating. The abort-discard path does not mark
it. The attached patch marks a subxact as streamed only when the
top-level xact already is. A later abort then does not emit stream_abort
for XIDs that were never sent downstream.
Patch with regress tests attached.
чт, 13 авг. 2026 г. в 20:30, PG Bug reporting form <noreply@postgresql.org>:
Show quoted text
The following bug has been logged on the website:
Bug reference: 19616
Logged by: Tyler Smart
Email address: tyler@smarts.io
PostgreSQL version: 18.4
Operating system: Linux (Docker; also Google Cloud SQL)
Description:Since PostgreSQL 18, pgoutput can send a Stream Abort ('A') message to a
client that connected with proto_version 1 and never enabled streaming.
Protocol 1 clients do not implement the stream message set, so consumers
fail on it: Debezium (and tools that embed it, like Airbyte) dies with
"Unsupported message type: A", and because the crash repeats at the same
WAL
position on every restart, the slot stops advancing until
max_slot_wal_keep_size invalidates it.I reproduced this on 18.4 (Debian 18.4-1.pgdg13+1, official Docker image)
and on Cloud SQL 18.1. The identical script does not reproduce it on 17.10.The trigger needs three conditions in one decode run:
1. logical_decoding_work_mem is exceeded, so eviction runs.
2. The eviction candidate has already aborted in clog.
3. That transaction has a subtransaction with changes still in memory.From reading REL_18_STABLE, the cause appears to be commit 072ee847ad4
("Skip logical decoding of already-aborted transactions"). It added
ReorderBufferCheckAndTruncateAbortedTXN, which discards aborted
transactions
at eviction time via ReorderBufferTruncateTXN. That function marks every
subtransaction that still has in-memory changes as streamed
(ReorderBufferMaybeMarkTXNStreamed, in the subtxn loop near
reorderbuffer.c:1675) without checking whether the connection streams at
all. The top-level transaction is handled correctly, since its marking
happens at call sites guarded by the streaming flag. Only the
subtransaction
marking is unconditional.When decoding later reaches the abort record, ReorderBufferAbort sees
rbtxn_is_streamed on the subtransaction and invokes the stream_abort
callback (near reorderbuffer.c:3092). pgoutput_stream_abort is guarded only
by assertions, so production builds write 'A' onto a proto_version 1
stream. I expect a cassert build to fail Assert(rbtxn_is_streamed(toptxn))
there instead, since the top-level transaction is not marked, though I have
not verified that.client that connected with proto_version 1 and never enabled streaming.
Protocol 1 clients do not implement the stream message set, so consumers
fail on it: Debezium (and tools that embed it, like Airbyte) dies with
"Unsupported message type: A", and because the crash repeats at the same
WAL
position on every restart, the slot stops advancing until
max_slot_wal_keep_size invalidates it.I reproduced this on 18.4 (Debian 18.4-1.pgdg13+1, official Docker image)
and on Cloud SQL 18.1. The identical script does not reproduce it on 17.10.The trigger needs three conditions in one decode run:
1. logical_decoding_work_mem is exceeded, so eviction runs.
2. The eviction candidate has already aborted in clog.
3. That transaction has a subtransaction with changes still in memory.From reading REL_18_STABLE, the cause appears to be commit 072ee847ad4
("Skip logical decoding of already-aborted transactions"). It added
ReorderBufferCheckAndTruncateAbortedTXN, which discards aborted
transactions
at eviction time via ReorderBufferTruncateTXN. That function marks every
subtransaction that still has in-memory changes as streamed
(ReorderBufferMaybeMarkTXNStreamed, in the subtxn loop near
reorderbuffer.c:1675) without checking whether the connection streams at
all. The top-level transaction is handled correctly, since its marking
happens at call sites guarded by the streaming flag. Only the
subtransaction
marking is unconditional.When decoding later reaches the abort record, ReorderBufferAbort sees
rbtxn_is_streamed on the subtransaction and invokes the stream_abort
callback (near reorderbuffer.c:3092). pgoutput_stream_abort is guarded only
by assertions, so production builds write 'A' onto a proto_version 1
stream. I expect a cassert build to fail Assert(rbtxn_is_streamed(toptxn))
there instead, since the top-level transaction is not marked, though I have
not verified that.Self-contained reproduction (the SQL decoding interface acts as a
non-streaming client, so no replication client is needed):docker run -d -e POSTGRES_PASSWORD=pw postgres:18 -c wal_level=logical
CREATE TABLE t(id int, filler text);
CREATE PUBLICATION pub FOR TABLE t;
SELECT pg_create_logical_replication_slot('s', 'pgoutput');BEGIN;
SAVEPOINT sp;
INSERT INTO t VALUES (0, 'subtransaction-change');
RELEASE SAVEPOINT sp;
INSERT INTO t SELECT g, repeat('x', 1000) FROM generate_series(1, 5000)
g;
ROLLBACK;INSERT INTO t VALUES (1, 'after');
SET logical_decoding_work_mem = '64kB';
SELECT chr(get_byte(data,0)) AS msgtype, count(*)
FROM pg_logical_slot_peek_binary_changes('s', NULL, NULL,
'proto_version','1','publication_names','pub')
GROUP BY 1 ORDER BY 2 DESC;Actual output on 18.4:
msgtype | count
---------+-------
B | 1
R | 1
C | 1
I | 1
A | 1Expected: no A row. A proto_version 1 client must never receive stream
messages, and an aborted transaction should produce no output at all.
PG 17.10 produces the expected output with the same script, as does 18.4
when logical_decoding_work_mem is raised enough that eviction never fires.Impact: any protocol 1 consumer on a busy PG 18 server can hit this with a
single canceled or deadlocked transaction that used savepoints, decoded
while the buffer is past logical_decoding_work_mem. We hit it in production
through Debezium, where the retry loop pinned the slot until Postgres
invalidated it. Raising logical_decoding_work_mem only lowers the
probability.Suggested direction: ReorderBufferTruncateTXN should mark subtransactions
as streamed only when truncating on behalf of streaming, the same way the
top-level marking is already gated, or the abort-discard path should skip
the marking entirely.
There are two shapes for the fix and I would like opinions on which is
preferred. Both are attached, they are mutually exclusive, and each
carries the same regress case.
0001-Don-t-mark-discarded-aborted-subxacts-as-streamed.patch adds the
guard at the call site in ReorderBufferTruncateTXN. It is a one-line
change and it does not touch the shared helper.
v2-0001-Don-t-mark-discarded-aborted-subxacts-as-streamed.patch moves the
rule into ReorderBufferMaybeMarkTXNStreamed itself. A subxact is marked
only when it has changes and its top-level xact is already streamed. This
states the invariant in one place and reads well on its own.
Thoughts?
чт, 13 авг. 2026 г. в 22:56, Andrey Rachitskiy <pl0h0yp1@gmail.com>:
Hi, Tyler!
Thanks for the report and analysis.
ReorderBufferTruncateTXN is used both after streaming and when discarding
already-aborted transactions at eviction. Streaming callers mark the
top-level xact before truncating. The abort-discard path does not mark
it. The attached patch marks a subxact as streamed only when the
top-level xact already is. A later abort then does not emit stream_abort
for XIDs that were never sent downstream.Patch with regress tests attached.
чт, 13 авг. 2026 г. в 20:30, PG Bug reporting form <noreply@postgresql.org
:
The following bug has been logged on the website:
Bug reference: 19616
Logged by: Tyler Smart
Email address: tyler@smarts.io
PostgreSQL version: 18.4
Operating system: Linux (Docker; also Google Cloud SQL)
Description:Since PostgreSQL 18, pgoutput can send a Stream Abort ('A') message to a
client that connected with proto_version 1 and never enabled streaming.
Protocol 1 clients do not implement the stream message set, so consumers
fail on it: Debezium (and tools that embed it, like Airbyte) dies with
"Unsupported message type: A", and because the crash repeats at the same
WAL
position on every restart, the slot stops advancing until
max_slot_wal_keep_size invalidates it.I reproduced this on 18.4 (Debian 18.4-1.pgdg13+1, official Docker image)
and on Cloud SQL 18.1. The identical script does not reproduce it on
17.10.The trigger needs three conditions in one decode run:
1. logical_decoding_work_mem is exceeded, so eviction runs.
2. The eviction candidate has already aborted in clog.
3. That transaction has a subtransaction with changes still in memory.From reading REL_18_STABLE, the cause appears to be commit 072ee847ad4
("Skip logical decoding of already-aborted transactions"). It added
ReorderBufferCheckAndTruncateAbortedTXN, which discards aborted
transactions
at eviction time via ReorderBufferTruncateTXN. That function marks every
subtransaction that still has in-memory changes as streamed
(ReorderBufferMaybeMarkTXNStreamed, in the subtxn loop near
reorderbuffer.c:1675) without checking whether the connection streams at
all. The top-level transaction is handled correctly, since its marking
happens at call sites guarded by the streaming flag. Only the
subtransaction
marking is unconditional.When decoding later reaches the abort record, ReorderBufferAbort sees
rbtxn_is_streamed on the subtransaction and invokes the stream_abort
callback (near reorderbuffer.c:3092). pgoutput_stream_abort is guarded
only
by assertions, so production builds write 'A' onto a proto_version 1
stream. I expect a cassert build to fail Assert(rbtxn_is_streamed(toptxn))
there instead, since the top-level transaction is not marked, though I
have
not verified that.client that connected with proto_version 1 and never enabled streaming.
Protocol 1 clients do not implement the stream message set, so consumers
fail on it: Debezium (and tools that embed it, like Airbyte) dies with
"Unsupported message type: A", and because the crash repeats at the same
WAL
position on every restart, the slot stops advancing until
max_slot_wal_keep_size invalidates it.I reproduced this on 18.4 (Debian 18.4-1.pgdg13+1, official Docker image)
and on Cloud SQL 18.1. The identical script does not reproduce it on
17.10.The trigger needs three conditions in one decode run:
1. logical_decoding_work_mem is exceeded, so eviction runs.
2. The eviction candidate has already aborted in clog.
3. That transaction has a subtransaction with changes still in memory.From reading REL_18_STABLE, the cause appears to be commit 072ee847ad4
("Skip logical decoding of already-aborted transactions"). It added
ReorderBufferCheckAndTruncateAbortedTXN, which discards aborted
transactions
at eviction time via ReorderBufferTruncateTXN. That function marks every
subtransaction that still has in-memory changes as streamed
(ReorderBufferMaybeMarkTXNStreamed, in the subtxn loop near
reorderbuffer.c:1675) without checking whether the connection streams at
all. The top-level transaction is handled correctly, since its marking
happens at call sites guarded by the streaming flag. Only the
subtransaction
marking is unconditional.When decoding later reaches the abort record, ReorderBufferAbort sees
rbtxn_is_streamed on the subtransaction and invokes the stream_abort
callback (near reorderbuffer.c:3092). pgoutput_stream_abort is guarded
only
by assertions, so production builds write 'A' onto a proto_version 1
stream. I expect a cassert build to fail Assert(rbtxn_is_streamed(toptxn))
there instead, since the top-level transaction is not marked, though I
have
not verified that.Self-contained reproduction (the SQL decoding interface acts as a
non-streaming client, so no replication client is needed):docker run -d -e POSTGRES_PASSWORD=pw postgres:18 -c wal_level=logical
CREATE TABLE t(id int, filler text);
CREATE PUBLICATION pub FOR TABLE t;
SELECT pg_create_logical_replication_slot('s', 'pgoutput');BEGIN;
SAVEPOINT sp;
INSERT INTO t VALUES (0, 'subtransaction-change');
RELEASE SAVEPOINT sp;
INSERT INTO t SELECT g, repeat('x', 1000) FROM generate_series(1, 5000)
g;
ROLLBACK;INSERT INTO t VALUES (1, 'after');
SET logical_decoding_work_mem = '64kB';
SELECT chr(get_byte(data,0)) AS msgtype, count(*)
FROM pg_logical_slot_peek_binary_changes('s', NULL, NULL,
'proto_version','1','publication_names','pub')
GROUP BY 1 ORDER BY 2 DESC;Actual output on 18.4:
msgtype | count
---------+-------
B | 1
R | 1
C | 1
I | 1
A | 1Expected: no A row. A proto_version 1 client must never receive stream
messages, and an aborted transaction should produce no output at all.
PG 17.10 produces the expected output with the same script, as does 18.4
when logical_decoding_work_mem is raised enough that eviction never fires.Impact: any protocol 1 consumer on a busy PG 18 server can hit this with a
single canceled or deadlocked transaction that used savepoints, decoded
while the buffer is past logical_decoding_work_mem. We hit it in
production
through Debezium, where the retry loop pinned the slot until Postgres
invalidated it. Raising logical_decoding_work_mem only lowers the
probability.Suggested direction: ReorderBufferTruncateTXN should mark subtransactions
as streamed only when truncating on behalf of streaming, the same way the
top-level marking is already gated, or the abort-discard path should skip
the marking entirely.
--
Regards,
Rachitskiy Andrey
Dear Tyler, Andrey,
(Adding Sawada-san in CC)
Good catch and thanks for the report. I confirmed that your reproducer causes an
Assert failure for the debug build. See:
```
(gdb) bt
#0 ReorderBufferMaybeMarkTXNStreamed (rb=0x3385430, txn=0x33a55a8)
at ../postgres/src/backend/replication/logical/reorderbuffer.c:2154
#1 0x0000000000973d05 in ReorderBufferTruncateTXN (rb=0x3385430, txn=0x33a5440,
txn_prepared=false) at ../postgres/src/backend/replication/logical/reorderbuffer.c:1677
#2 0x00000000009740ec in ReorderBufferCheckAndTruncateAbortedTXN (rb=0x3385430, txn=0x33a5440)
at ../postgres/src/backend/replication/logical/reorderbuffer.c:1816
#3 0x0000000000977aa1 in ReorderBufferCheckMemoryLimit (rb=0x3385430)
at ../postgres/src/backend/replication/logical/reorderbuffer.c:3985
#4 0x0000000000972345 in ReorderBufferQueueChange (rb=0x3385430, xid=696, lsn=24970056,
change=0x33a8be0, toast_insert=false)
...
```
Few comments for the code:
```
+ * A top-level transaction is always marked. A subtransaction is marked only
+ * when it has changes and its top-level transaction is already streamed.
```The last sentence can be "its top-level transaction is already marked as streamed."
```
+ /*
+ * A subtransaction is marked only when it has changes, and only when its
+ * top-level transaction has already been marked as streamed. We never
+ * stream XIDs of empty subxacts, and we must not send an abort for an XID
+ * the downstream has never heard of.
*
- * We do it this way because of aborts - we don't want to send aborts for
- * XIDs the downstream is not aware of. And of course, it always knows
- * about the top-level xact (we send the XID in all messages), but we
- * never stream XIDs of empty subxacts.
+ * The top-level check matters because ReorderBufferTruncateTXN is also
+ * used to discard already-aborted transactions at eviction, where the
+ * top-level xact is not streamed. Marking a subxact there would make a
+ * later abort emit stream_abort to a client that never enabled streaming.
*/
- if (rbtxn_is_toptxn(txn) || (txn->nentries_mem != 0))
+ if (txn->nentries_mem != 0 && rbtxn_is_streamed(rbtxn_get_toptxn(txn)))
```I feel the code comment might be too detail: second paragraph is not needed
for me.
Best regards,
Hayato Kuroda
FUJITSU LIMITED
Dear Kuroda-san!
Thanks for the review.
I addressed both comments in the attached v3.
пт, 14 авг. 2026 г. в 08:03, Hayato Kuroda (Fujitsu) <
kuroda.hayato@fujitsu.com>:
Dear Tyler, Andrey,
(Adding Sawada-san in CC)Good catch and thanks for the report. I confirmed that your reproducer
causes an
Assert failure for the debug build. See:```
(gdb) bt
#0 ReorderBufferMaybeMarkTXNStreamed (rb=0x3385430, txn=0x33a55a8)
at ../postgres/src/backend/replication/logical/reorderbuffer.c:2154
#1 0x0000000000973d05 in ReorderBufferTruncateTXN (rb=0x3385430,
txn=0x33a5440,
txn_prepared=false) at
../postgres/src/backend/replication/logical/reorderbuffer.c:1677
#2 0x00000000009740ec in ReorderBufferCheckAndTruncateAbortedTXN
(rb=0x3385430, txn=0x33a5440)
at ../postgres/src/backend/replication/logical/reorderbuffer.c:1816
#3 0x0000000000977aa1 in ReorderBufferCheckMemoryLimit (rb=0x3385430)
at ../postgres/src/backend/replication/logical/reorderbuffer.c:3985
#4 0x0000000000972345 in ReorderBufferQueueChange (rb=0x3385430, xid=696,
lsn=24970056,
change=0x33a8be0, toast_insert=false)
...
```Few comments for the code:
``` + * A top-level transaction is always marked. A subtransaction is marked only + * when it has changes and its top-level transaction is already streamed. ```The last sentence can be "its top-level transaction is already marked as
streamed."``` + /* + * A subtransaction is marked only when it has changes, and only when its + * top-level transaction has already been marked as streamed. We never + * stream XIDs of empty subxacts, and we must not send an abort for an XID + * the downstream has never heard of. * - * We do it this way because of aborts - we don't want to send aborts for - * XIDs the downstream is not aware of. And of course, it always knows - * about the top-level xact (we send the XID in all messages), but we - * never stream XIDs of empty subxacts. + * The top-level check matters because ReorderBufferTruncateTXN is also + * used to discard already-aborted transactions at eviction, where the + * top-level xact is not streamed. Marking a subxact there would make a + * later abort emit stream_abort to a client that never enabled streaming. */ - if (rbtxn_is_toptxn(txn) || (txn->nentries_mem != 0)) + if (txn->nentries_mem != 0 && rbtxn_is_streamed(rbtxn_get_toptxn(txn))) ```I feel the code comment might be too detail: second paragraph is not needed
for me.Best regards,
Hayato Kuroda
FUJITSU LIMITED
--
Regards,
Rachitskiy Andrey
Dear Andrey,
Thanks for the update. One more comment for the test.
```
+-- Aborted xact discarded at eviction, with a subxact still in memory.
+-- proto_version 1 must not see Stream Abort ('A').
```I think we should describe bit more what happened there. How about like:
-- bug #19616
-- pgoutput protocol compatibility could be broken for an aborted xact
-- discarded at spill eviction while a subxact remained in memory.
-- Stream Abort ('A'), valid only since protocol version 2, could be seen
-- with protocol version 1.
BTW, our community tries not to do top-post reply. See the guideline:
https://wiki.postgresql.org/wiki/Mailing_Lists
Best regards,
Hayato Kuroda
FUJITSU LIMITED
I think we should describe bit more what happened there. How about like:
-- bug #19616
-- pgoutput protocol compatibility could be broken for an aborted xact
-- discarded at spill eviction while a subxact remained in memory.
-- Stream Abort ('A'), valid only since protocol version 2, could be seen
-- with protocol version 1.
I have updated the patch with the suggested changes.
BTW, our community tries not to do top-post reply.
Dear Kuroda-San,
Thank you for pointing this out. I will keep that in mind for future
messages.
--
Regards,
Rachitskiy Andrey
Dear Andrey,
Thanks for the update. For now the patch LGTM.
Let’s see how others say.
Dear Kuroda-San,
Thank you for pointing this out. I will keep that in mind for future messages.
FYI, your style is good :-).
Best regards,
Hayato Kuroda
FUJITSU LIMITED
On Fri, Aug 14, 2026 at 3:45 PM Andrey Rachitskiy <pl0h0yp1@gmail.com> wrote:
I have updated the patch with the suggested changes.
Thanks for updating the patch! LGTM.
One minor comment: ReorderBufferMaybeMarkTXNStreamed() seems to rely on
the assumption that it is never called for a top-level transaction when
streaming is disabled. If that's correct, how about documenting this
assumption in the function comment and/or adding an assertion such as:
if (rbtxn_is_toptxn(txn))
{
Assert(ReorderBufferCanStream(rb));
txn->txn_flags |= RBTXN_IS_STREAMED;
Regards,
--
Fujii Masao
пт, 14 авг. 2026 г. в 17:33, Fujii Masao <masao.fujii@gmail.com>:
One minor comment: ReorderBufferMaybeMarkTXNStreamed() seems to rely on
the assumption that it is never called for a top-level transaction when
streaming is disabled. If that's correct, how about documenting this
assumption in the function comment and/or adding an assertion such as:if (rbtxn_is_toptxn(txn))
{
Assert(ReorderBufferCanStream(rb));
txn->txn_flags |= RBTXN_IS_STREAMED;Dear Fujii-san,
Thanks for the review.
v4 with assert and comment, in attachment.
--
Regards,
Rachitskiy Andrey
Attachments:
t253401_10v4-0001-Don-t-mark-discarded-aborted-subxacts-as-streamed.patchtext/x-patch; charset=US-ASCII; name=v4-0001-Don-t-mark-discarded-aborted-subxacts-as-streamed.patchDownload+83-15
On Fri, Aug 14, 2026 at 7:35 AM Andrey Rachitskiy <pl0h0yp1@gmail.com> wrote:
пт, 14 авг. 2026 г. в 17:33, Fujii Masao <masao.fujii@gmail.com>:
One minor comment: ReorderBufferMaybeMarkTXNStreamed() seems to rely on
the assumption that it is never called for a top-level transaction when
streaming is disabled. If that's correct, how about documenting this
assumption in the function comment and/or adding an assertion such as:if (rbtxn_is_toptxn(txn))
{
Assert(ReorderBufferCanStream(rb));
txn->txn_flags |= RBTXN_IS_STREAMED;Dear Fujii-san,
Thanks for the review.
v4 with assert and comment, in attachment.
Thank you for creating the patch.
IIUC it's not only a protocol compatibility issue but also an issue
that streaming messages are sent even with the streaming mode being
disabled. Therefore, it can be hit even with the latest protocol
version if subscribers disable the streaming option. While the fix
looks good to me, I have a few review comments on the regression test
part:
+-- bug #19616
+-- pgoutput protocol compatibility could be broken for an aborted xact
+-- discarded at spill eviction while a subxact remained in memory.
+-- Stream Abort ('A'), valid only since protocol version 2, could be seen
+-- with protocol version 1.
...
+SELECT chr(get_byte(data, 0)) AS msgtype, count(*)
+FROM pg_logical_slot_peek_binary_changes('regression_slot_pgoutput',
NULL, NULL,
+ 'proto_version', '1', 'publication_names', 'spill_pub')
+GROUP BY 1 ORDER BY 1;I'm not sure we should test the case against proto_version=1 for the
reason I mentioned above. I think we can use proto_version=4 and
streaming=0 instead. Also, the comment needs to be adjusted.
stream.sql would be more suitable to put this test.
Regards,
--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com
сб, 15 авг. 2026 г. в 03:15, Masahiko Sawada <sawada.mshk@gmail.com>:
While the fix
looks good to me, I have a few review comments on the regression test
part:+-- bug #19616 +-- pgoutput protocol compatibility could be broken for an aborted xact +-- discarded at spill eviction while a subxact remained in memory. +-- Stream Abort ('A'), valid only since protocol version 2, could be seen +-- with protocol version 1. ... +SELECT chr(get_byte(data, 0)) AS msgtype, count(*) +FROM pg_logical_slot_peek_binary_changes('regression_slot_pgoutput', NULL, NULL, + 'proto_version', '1', 'publication_names', 'spill_pub') +GROUP BY 1 ORDER BY 1;I'm not sure we should test the case against proto_version=1 for the
reason I mentioned above. I think we can use proto_version=4 and
streaming=0 instead. Also, the comment needs to be adjusted.stream.sql would be more suitable to put this test.
Dear Sawada-san,
You are right, and these remarks have been addressed in v5 of the patch.
The spurious Stream Abort is not really about the protocol version.
So the real invariant is that a client which did not enable streaming
must not receive streaming messages. proto_version=1 only tested a
special case. proto_version=4 with streaming=0 exercises the
invariant directly. I confirmed the reformulated test still catches the bug.
I moved the test to stream.sql as you suggested, keeping the structure of
the already-reviewed comment.
--
Regards,
Rachitskiy Andrey
Attachments:
t253401_12v5-0001-Don-t-mark-discarded-aborted-subxacts-as-streamed.patchtext/x-patch; charset=US-ASCII; name=v5-0001-Don-t-mark-discarded-aborted-subxacts-as-streamed.patchDownload+87-15
On Fri, Aug 14, 2026 at 7:27 PM Andrey Rachitskiy <pl0h0yp1@gmail.com> wrote:
сб, 15 авг. 2026 г. в 03:15, Masahiko Sawada <sawada.mshk@gmail.com>:
While the fix
looks good to me, I have a few review comments on the regression test
part:+-- bug #19616 +-- pgoutput protocol compatibility could be broken for an aborted xact +-- discarded at spill eviction while a subxact remained in memory. +-- Stream Abort ('A'), valid only since protocol version 2, could be seen +-- with protocol version 1. ... +SELECT chr(get_byte(data, 0)) AS msgtype, count(*) +FROM pg_logical_slot_peek_binary_changes('regression_slot_pgoutput', NULL, NULL, + 'proto_version', '1', 'publication_names', 'spill_pub') +GROUP BY 1 ORDER BY 1;I'm not sure we should test the case against proto_version=1 for the
reason I mentioned above. I think we can use proto_version=4 and
streaming=0 instead. Also, the comment needs to be adjusted.stream.sql would be more suitable to put this test.
Dear Sawada-san,
You are right, and these remarks have been addressed in v5 of the patch.
The spurious Stream Abort is not really about the protocol version.
So the real invariant is that a client which did not enable streaming
must not receive streaming messages. proto_version=1 only tested a
special case. proto_version=4 with streaming=0 exercises the
invariant directly. I confirmed the reformulated test still catches the bug.
I moved the test to stream.sql as you suggested, keeping the structure of
the already-reviewed comment.
Thank you for updating the patch!
We can verify that stream_abort callback is not called when the
streaming mode is disabled, and we can use test_decoding for the
regression test at the end of the day. Which is simpler. I've updated
the regression test part accordingly and the commit message. Please
review it.
Regards,
--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com
Attachments:
t253401_13v6-0001-Don-t-mark-discarded-aborted-subxacts-as-streamed.patchtext/x-patch; charset=US-ASCII; name=v6-0001-Don-t-mark-discarded-aborted-subxacts-as-streamed.patchDownload+64-15
вт, 18 авг. 2026 г. в 01:46, Masahiko Sawada <sawada.mshk@gmail.com>:
Please review it.
Dear Sawada-san,
Thanks for the review. LGTM
--
Regards,
Rachitskiy Andrey
On Tue, Aug 18, 2026 at 1:29 AM Andrey Rachitskiy <pl0h0yp1@gmail.com> wrote:
вт, 18 авг. 2026 г. в 01:46, Masahiko Sawada <sawada.mshk@gmail.com>:
Please review it.
Dear Sawada-san,
Thanks for the review. LGTM
Thank you for reviewing the patch. Pushed.
Regards,
--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com