From: Andy Fan <zhihuifan1213@163.com>
To: pgsql-hackers <pgsql-hackers@postgresql.org>
Subject: Re: pgbench error: (setshell) of script 0; execution of
 meta-command failed
In-Reply-To: <87msfz5qv2.fsf@163.com> (Andy Fan's message of "Fri, 10 Jan 2025
	02:45:21 +0000")
References: <87msfz5qv2.fsf@163.com>
Date: Fri, 10 Jan 2025 07:09:52 +0000
Message-ID: <87ikqn5em7.fsf@163.com>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="=-=-="
Archived-At: <https://www.postgresql.org/message-id/87ikqn5em7.fsf%40163.com>
Precedence: bulk

--=-=-=
Content-Type: text/plain

Andy Fan <zhihuifan1213@163.com> writes:

> Hi:
>
> I run into the {subject} issue with the below setup.
>
> cat foo.sql
>
> \setshell txn_mode echo ${TXN_MODE}
> \setshell speed echo ${SPEED}
> \setshell sleep_ms echo ${SLEEP_MS}
> \setshell subtxn_mode echo ${SUBTXN_MODE}
>
> select 1;
>
> $ TXN_MODE=-1 SPEED=1 SLEEP_MS=0 SUBTXN_MODE=-1 pgbench -n -ffoo.sql postgres -T5 -c4 --exit-on-abort
>
> I *randomly*(7/8) get errors like:
>
> pgbench (18devel)
> pgbench: error: client 2 aborted in command 0 (setshell) of script 0; execution of meta-command failed
> pgbench: error: Run was aborted due to an error in thread 0

I think I have figured out the issue, if you want reproduce it quicker,
you can change the '-T5' to '-T1' in the pgbench command and run many times.

Here is the patch to fix it, would someone take a look at? 

    pgbench: Avoid misleading error for \[set]shell when timer_exceeded.
    
    fgets in executeMetaCommand may return NULL if it receives a signal
    during the shell command is executing. Before this commit, pgbench
    client raises ERROR like below.
    
    pgbench: error: client 3 aborted in command 3 (setshell) of script 0;
    execution of meta-command failed
    
    This behavior is misleading since people may think something is
    wrong. In this commit, we just ignore fgets return NULL when
    timer_exceeded.


-- 
Best Regards
Andy Fan


--=-=-=
Content-Type: text/x-diff
Content-Disposition: attachment;
 filename=0001-pgbench-Avoid-misleading-error-for-set-shell-when-ti.patch

From 9b03ff66e45d019631ed9fd3322c0911ce14a8a7 Mon Sep 17 00:00:00 2001
From: Andy Fan <zhihuifan1213@163.com>
Date: Fri, 10 Jan 2025 06:56:11 +0000
Subject: [PATCH 1/1] pgbench: Avoid misleading error for \[set]shell when
 timer_exceeded.

fgets in executeMetaCommand may return NULL if it receives a signal
during the shell command is executing. Before this commit, pgbench
client raises ERROR like below.

pgbench: error: client 3 aborted in command 3 (setshell) of script 0;
execution of meta-command failed

This behavior is misleading since people may think something is
wrong. In this commit, we just ignore fgets return NULL when
timer_exceeded.
---
 src/bin/pgbench/pgbench.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/src/bin/pgbench/pgbench.c b/src/bin/pgbench/pgbench.c
index c415e0f32c..7f61cd3aea 100644
--- a/src/bin/pgbench/pgbench.c
+++ b/src/bin/pgbench/pgbench.c
@@ -3851,7 +3851,9 @@ advanceConnectionState(TState *thread, CState *st, StatsData *agg)
 				Assert(st->state == CSTATE_WAIT_RESULT ||
 					   st->state == CSTATE_END_COMMAND ||
 					   st->state == CSTATE_SLEEP ||
-					   st->state == CSTATE_ABORTED);
+					   st->state == CSTATE_ABORTED ||
+					   st->state == CSTATE_FINISHED);
+
 				break;
 
 				/*
@@ -4414,6 +4416,8 @@ executeMetaCommand(CState *st, pg_time_usec_t *now)
 	{
 		if (!runShellCommand(&st->variables, argv[1], argv + 2, argc - 2))
 		{
+			if (timer_exceeded)
+				return CSTATE_FINISHED;
 			commandFailed(st, "setshell", "execution of meta-command failed");
 			return CSTATE_ABORTED;
 		}
@@ -4422,6 +4426,8 @@ executeMetaCommand(CState *st, pg_time_usec_t *now)
 	{
 		if (!runShellCommand(&st->variables, NULL, argv + 1, argc - 1))
 		{
+			if (timer_exceeded)
+				return CSTATE_FINISHED;
 			commandFailed(st, "shell", "execution of meta-command failed");
 			return CSTATE_ABORTED;
 		}
-- 
2.45.1


--=-=-=--