public inbox for [email protected]help / color / mirror / Atom feed
pg_recvlogical: send final feedback on SIGINT/SIGTERM exit 3+ messages / 2 participants [nested] [flat]
* pg_recvlogical: send final feedback on SIGINT/SIGTERM exit @ 2026-05-29 00:11 Fujii Masao <[email protected]> 0 siblings, 1 reply; 3+ messages in thread From: Fujii Masao @ 2026-05-29 00:11 UTC (permalink / raw) To: PostgreSQL Hackers <[email protected]> Hi, I'd like to propose an improvement for pg_recvlogical. Currently, when pg_recvlogical exits due to SIGINT or SIGTERM, it can terminate after writing decoded output locally but before sending feedback that reflects the latest written position to the server. If pg_recvlogical is restarted after that, the server-side logical replication slot may still remain behind those already-written changes, and the same decoded data can be sent again. Attached patch makes pg_recvlogical send final feedback once more during SIGINT/SIGTERM exit, before sending CopyDone. That gives the server one more chance to advance the slot far enough to avoid resending already-written data after pg_recvlogical is restarted. One note is that this is still only a best-effort improvement. Depending on when SIGINT or SIGTERM arrives, pg_recvlogical may already have written decoded output that the server cannot yet safely treat as confirmed, so duplicate data can still be received after restart. In other words, this patch reduces the likelihood of duplicates, but does not guarantee that they will never happen. I see this as an improvement to pg_recvlogical behavior rather than a bug fix, so I think it should target v20. Regards, -- Fujii Masao Attachments: [application/octet-stream] v1-0001-pg_recvlogical-send-final-feedback-on-SIGINT-SIGT.patch (5.0K, ../../CAHGQGwE83z9O=X7ADMsSa3e1EuP3_GgqHjFt5SmPDNxZo_wgJA@mail.gmail.com/2-v1-0001-pg_recvlogical-send-final-feedback-on-SIGINT-SIGT.patch) download | inline diff: From ad98280be52f37acb23d8cd14fc26dd04e1cd3e9 Mon Sep 17 00:00:00 2001 From: Fujii Masao <[email protected]> Date: Thu, 28 May 2026 23:19:42 +0900 Subject: [PATCH v1] pg_recvlogical: send final feedback on SIGINT/SIGTERM shutdown Previously, when pg_recvlogical exited due to SIGINT or SIGTERM, it could terminate without sending final feedback for the last decoded changes it had already written locally. So, if pg_recvlogical was restarted afterwards, the server-side logical replication slot could still point behind those changes, causing them to be sent again. Make pg_recvlogical send final feedback once more during SIGINT/SIGTERM shutdown, before sending CopyDone. This gives the server one more chance to advance the slot far enough to avoid resending already-written data, so users are less likely to see duplicate decoded output after stopping and restarting pg_recvlogical. This remains a best-effort improvement rather than a guarantee. Depending on when the signal arrives, pg_recvlogical can already have written decoded output that the server cannot yet safely treat as confirmed, so a later restart can still receive duplicate data. --- src/bin/pg_basebackup/pg_recvlogical.c | 23 ++++++ src/bin/pg_basebackup/t/030_pg_recvlogical.pl | 74 +++++++++++++++++++ 2 files changed, 97 insertions(+) diff --git a/src/bin/pg_basebackup/pg_recvlogical.c b/src/bin/pg_basebackup/pg_recvlogical.c index 2fdf64bcadb..11abdbc274e 100644 --- a/src/bin/pg_basebackup/pg_recvlogical.c +++ b/src/bin/pg_basebackup/pg_recvlogical.c @@ -1071,6 +1071,29 @@ static void prepareToTerminate(PGconn *conn, XLogRecPtr endpos, StreamStopReason reason, XLogRecPtr lsn) { + /* + * If pg_recvlogical is terminated by a signal, we can reach here without + * sending final feedback. In that case, send feedback once more before + * sending CopyDone so the replication slot can advance far enough to + * reduce the chance of resending duplicate data when pg_recvlogical is + * restarted. + * + * This is still only a best-effort attempt. Depending on when the signal + * arrives, the receiver may have written decoded output that the server + * cannot yet safely treat as confirmed, so a later restart can still see + * duplicate data. + * + * For other termination cases, such as STREAM_STOP_KEEPALIVE and + * STREAM_STOP_END_OF_WAL, feedback has already been sent before reaching + * here, so there is no need to call flushAndSendFeedback() again. + */ + if (reason == STREAM_STOP_SIGNAL) + { + TimestampTz now = feGetCurrentTimestamp(); + + (void) flushAndSendFeedback(conn, &now); + } + (void) PQputCopyEnd(conn, NULL); (void) PQflush(conn); diff --git a/src/bin/pg_basebackup/t/030_pg_recvlogical.pl b/src/bin/pg_basebackup/t/030_pg_recvlogical.pl index 945a242bdad..5e3e36cc4f3 100644 --- a/src/bin/pg_basebackup/t/030_pg_recvlogical.pl +++ b/src/bin/pg_basebackup/t/030_pg_recvlogical.pl @@ -282,6 +282,80 @@ SKIP: 'pg_recvlogical output file respects group permissions (0640)'); } +SKIP: +{ + skip "signals not supported on Windows", 4 + if ($Config{osname} eq 'MSWin32' || $Config{osname} eq 'cygwin'); + + my $signal_outfile = $node->basedir . '/signal_shutdown.out'; + + $node->command_ok( + [ + 'pg_recvlogical', + '--slot' => 'signal_shutdown_test', + '--dbname' => $node->connstr('postgres'), + '--create-slot', + ], + 'slot created for signal shutdown test'); + + @pg_recvlogical_cmd = ( + 'pg_recvlogical', + '--slot' => 'signal_shutdown_test', + '--dbname' => $node->connstr('postgres'), + '--start', + '--file' => $signal_outfile, + '--fsync-interval' => '100', + '--status-interval' => '100'); + + $recv = IPC::Run::start( + [@pg_recvlogical_cmd], + '>' => \$stdout, + '2>' => \$stderr); + + $node->safe_psql('postgres', 'INSERT INTO test_table VALUES (42)'); + + # Wait for not only INSERT but also COMMIT because the inserted + # change might not yet be safely confirmable by final feedback until + # the transaction has committed. + wait_for_file($signal_outfile, + qr/test_table: INSERT: x\[integer\]:42\b.*?\bCOMMIT\b/s); + + $recv->signal('TERM'); + $recv->finish(); + + $nextlsn = + $node->safe_psql('postgres', 'SELECT pg_current_wal_insert_lsn()'); + chomp($nextlsn); + + $node->command_ok( + [ + 'pg_recvlogical', + '--slot' => 'signal_shutdown_test', + '--dbname' => $node->connstr('postgres'), + '--start', + '--endpos' => $nextlsn, + '--no-loop', + '--file' => $signal_outfile, + ], + 'pg_recvlogical exits after signal without replaying flushed data'); + + my $signal_data = slurp_file($signal_outfile); + my $signal_count = + (() = $signal_data =~ /test_table: INSERT: x\[integer\]:42\b/g); + is($signal_count, 1, + 'pg_recvlogical does not duplicate decoded changes after signal shutdown' + ); + + $node->command_ok( + [ + 'pg_recvlogical', + '--slot' => 'signal_shutdown_test', + '--dbname' => $node->connstr('postgres'), + '--drop-slot' + ], + 'signal_shutdown_test slot dropped'); +} + $node->command_ok( [ 'pg_recvlogical', -- 2.53.0 ^ permalink raw reply [nested|flat] 3+ messages in thread
* Re: pg_recvlogical: send final feedback on SIGINT/SIGTERM exit @ 2026-06-15 13:41 Ayush Tiwari <[email protected]> parent: Fujii Masao <[email protected]> 0 siblings, 1 reply; 3+ messages in thread From: Ayush Tiwari @ 2026-06-15 13:41 UTC (permalink / raw) To: Fujii Masao <[email protected]>; +Cc: PostgreSQL Hackers <[email protected]> Hi, On Fri, 29 May 2026 at 05:42, Fujii Masao <[email protected]> wrote: > > Currently, when pg_recvlogical exits due to SIGINT or SIGTERM, it can > terminate after writing decoded output locally but before sending feedback > that reflects the latest written position to the server. If pg_recvlogical > is restarted after that, the server-side logical replication slot may still > remain behind those already-written changes, and the same decoded data can > be sent again. > > Attached patch makes pg_recvlogical send final feedback once more during > SIGINT/SIGTERM exit, before sending CopyDone. That gives the server one > more > chance to advance the slot far enough to avoid resending already-written > data after pg_recvlogical is restarted. > Thanks for the patch! The problem statement and solution makes sense to me. I applied v1 and tried it locally. It builds cleanly and the new block in 030_pg_recvlogical.pl passes. As a sanity check I reverted just the prepareToTerminate() change and kept the test: the "does not duplicate decoded changes after signal shutdown" assertion then fails (the row is decoded twice on the restart), so the test is clearly exercising the fix too. I don't see any downside of having the fix attached. > One note is that this is still only a best-effort improvement. > Depending on when SIGINT or SIGTERM arrives, pg_recvlogical may already > have > written decoded output that the server cannot yet safely treat as > confirmed, > so duplicate data can still be received after restart. In other words, this > patch reduces the likelihood of duplicates, but does not guarantee that > they > will never happen. > Agreed that this is a best-effort improvement rather than a guarantee, and targeting v20 seems reasonable. Regards, Ayush ^ permalink raw reply [nested|flat] 3+ messages in thread
* Re: pg_recvlogical: send final feedback on SIGINT/SIGTERM exit @ 2026-07-08 03:21 Fujii Masao <[email protected]> parent: Ayush Tiwari <[email protected]> 0 siblings, 0 replies; 3+ messages in thread From: Fujii Masao @ 2026-07-08 03:21 UTC (permalink / raw) To: Ayush Tiwari <[email protected]>; +Cc: PostgreSQL Hackers <[email protected]> On Mon, Jun 15, 2026 at 10:42 PM Ayush Tiwari <[email protected]> wrote: > > Hi, > > On Fri, 29 May 2026 at 05:42, Fujii Masao <[email protected]> wrote: >> >> >> Currently, when pg_recvlogical exits due to SIGINT or SIGTERM, it can >> terminate after writing decoded output locally but before sending feedback >> that reflects the latest written position to the server. If pg_recvlogical >> is restarted after that, the server-side logical replication slot may still >> remain behind those already-written changes, and the same decoded data can >> be sent again. >> >> Attached patch makes pg_recvlogical send final feedback once more during >> SIGINT/SIGTERM exit, before sending CopyDone. That gives the server one more >> chance to advance the slot far enough to avoid resending already-written >> data after pg_recvlogical is restarted. > > > Thanks for the patch! > > The problem statement and solution makes sense to me. > > I applied v1 and tried it locally. It builds cleanly and the new block in > 030_pg_recvlogical.pl passes. As a sanity check I reverted just the > prepareToTerminate() change and kept the test: the "does not duplicate decoded > changes after signal shutdown" assertion then fails (the row is decoded twice > on the restart), so the test is clearly exercising the fix too. I don't see any > downside of having the fix attached. Thanks for the review! I've pushed the patch. Regards, -- Fujii Masao ^ permalink raw reply [nested|flat] 3+ messages in thread
end of thread, other threads:[~2026-07-08 03:21 UTC | newest] Thread overview: 3+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2026-05-29 00:11 pg_recvlogical: send final feedback on SIGINT/SIGTERM exit Fujii Masao <[email protected]> 2026-06-15 13:41 ` Ayush Tiwari <[email protected]> 2026-07-08 03:21 ` Fujii Masao <[email protected]>
This inbox is served by agora; see mirroring instructions for how to clone and mirror all data and code used for this inbox