v4-0002-Do-not-reset-statement_timeout-indicator-outside-.patch

application/octet-stream

Filename: v4-0002-Do-not-reset-statement_timeout-indicator-outside-.patch
Type: application/octet-stream
Part: 1
Message: Re: [EXTERNAL] Re: Add non-blocking version of PQcancel

Patch

Format: format-patch
Series: patch v4-0002
Subject: Do not reset statement_timeout indicator outside of ProcessInterupts
File+
src/backend/tcop/postgres.c 16 5
From b87758a8bbfde9a035de65bf9048c1521afd4ca7 Mon Sep 17 00:00:00 2001
From: Jelte Fennema-Nio <jelte.fennema@microsoft.com>
Date: Mon, 24 Jun 2024 00:29:39 +0200
Subject: [PATCH v4 2/2] Do not reset statement_timeout indicator outside of
 ProcessInterupts

The only way that ProcessInterupts can know why QueryCancelPending is
set is by looking at the indicator bits of the various timeouts. We were
resetting the one for STATEMENT_TIMEOUT in various places, thus possibly
causing ProcessInterupts to fail with the wrong error message.
---
 src/backend/tcop/postgres.c | 21 ++++++++++++++++-----
 1 file changed, 16 insertions(+), 5 deletions(-)

diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c
index e39c6804a7..03d1eb63ff 100644
--- a/src/backend/tcop/postgres.c
+++ b/src/backend/tcop/postgres.c
@@ -5150,22 +5150,33 @@ enable_statement_timeout(void)
 	if (StatementTimeout > 0
 		&& (StatementTimeout < TransactionTimeout || TransactionTimeout == 0))
 	{
-		if (!get_timeout_active(STATEMENT_TIMEOUT))
+		/*
+		 * We check both if it's active or if it's already triggered. If it's
+		 * already triggered we don't want to restart it because that clears
+		 * the indicator flag, which in turn would cause the wrong error
+		 * message to be used by ProcessInterrupts() on the next
+		 * CHECK_FOR_INTERRUPTS() call. Restarting the timer in that case
+		 * would be pointless anyway, because the statement timeout error is
+		 * going to trigger on the next CHECK_FOR_INTERRUPTS() call.
+		 */
+		if (!get_timeout_active(STATEMENT_TIMEOUT)
+			&& !get_timeout_indicator(STATEMENT_TIMEOUT, false))
 			enable_timeout_after(STATEMENT_TIMEOUT, StatementTimeout);
 	}
 	else
 	{
-		if (get_timeout_active(STATEMENT_TIMEOUT))
-			disable_timeout(STATEMENT_TIMEOUT, false);
+		disable_statement_timeout();
 	}
 }
 
 /*
- * Disable statement timeout, if active.
+ * Disable statement timeout, if active. We preserve the indicator flag
+ * though, otherwise we'd lose the knowledge in ProcessInterupts that the
+ * SIGINT came from a statement timeout.
  */
 static void
 disable_statement_timeout(void)
 {
 	if (get_timeout_active(STATEMENT_TIMEOUT))
-		disable_timeout(STATEMENT_TIMEOUT, false);
+		disable_timeout(STATEMENT_TIMEOUT, true);
 }
-- 
2.34.1