From 4001131d30a5c18e49749f534b6016a18b19ed0c Mon Sep 17 00:00:00 2001 From: Anthonin Bonnefoy Date: Mon, 13 Jan 2025 09:41:43 +0100 Subject: Accept recovery conflict interrupt on blocked writing Previously, all interrupts except process dying were ignored while a process was blocked writing to a socket. If the connection to the client was broken (no clean FIN nor RST), a process sending results to the client could be stuck for 924s until the TCP retransmission timeout is reached. During this time, it was possible for the process to conflict with recovery: For example, the process returning results can have a conflicting buffer pin. To avoid blocking recovery for an extended period of time, this patch changes client write interrupts by handling recovery conflict interrupts instead of ignoring them. Since the interrupt happens while we're likely to have partially written results on the socket, there's no easy way to keep protocol sync so the session needs to be terminated. --- src/backend/tcop/postgres.c | 42 ++++++++++++++++++++++++++++++++++--- 1 file changed, 39 insertions(+), 3 deletions(-) diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c index 5655348a2e2..e563a0cc486 100644 --- a/src/backend/tcop/postgres.c +++ b/src/backend/tcop/postgres.c @@ -134,6 +134,12 @@ static bool xact_started = false; */ static bool DoingCommandRead = false; +/* + * Flag to indicate that we are doing a blocking write as the socket doesn't have + * the necessary space + */ +static bool DoingBlockingWrite = false; + /* * Flags to implement skip-till-Sync-after-error behavior for messages of * the extended query protocol. @@ -581,6 +587,29 @@ ProcessClientWriteInterrupt(bool blocked) else SetLatch(MyLatch); } + else if (blocked && RecoveryConflictPending) + { + /* + * We're conflicting with recovery while being blocked writing. This + * can happen when the process is returning results and no ACK is + * received (broken connection, client overloaded...), eventually + * saturating the socket buffer while the process holds a page pin + * that eventually conflict with recovery. + */ + if (InterruptHoldoffCount == 0 && CritSectionCount == 0) + { + if (whereToSendOutput == DestRemote) + whereToSendOutput = DestNone; + + /* + * Set blocking write flag to terminate the session as it is not + * possible to keep protocol sync + */ + DoingBlockingWrite = true; + CHECK_FOR_INTERRUPTS(); + DoingBlockingWrite = false; + } + } errno = save_errno; } @@ -3167,10 +3196,17 @@ ProcessRecoveryConflictInterrupt(ProcSignalReason reason) * If a recovery conflict happens while we are waiting for * input from the client, the client is presumably just * sitting idle in a transaction, preventing recovery from - * making progress. We'll drop through to the FATAL case - * below to dislodge it, in that case. + * making progress. + * + * Similarly, if we are in a blocking write, we could be + * waiting for a TCP ACK from the client to make the socket + * writable but the client may not be here anymore, leaving us + * stuck in TCP retransmission. + * + * In those cases, we'll drop through to the FATAL case below + * to dislodge it. */ - if (!DoingCommandRead) + if (!DoingCommandRead && !DoingBlockingWrite) { /* Avoid losing sync in the FE/BE protocol. */ if (QueryCancelHoldoffCount != 0) -- 2.39.5 (Apple Git-154)