From: | Michael Paquier <michael(dot)paquier(at)gmail(dot)com> |
---|---|
To: | Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr> |
Cc: | PostgreSQL Developers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: pgbench - minor fix for meta command only scripts |
Date: | 2016-07-11 04:41:00 |
Message-ID: | CAB7nPqRZmRbsfdShOLPMGuqU3BeZFbYG5u0jsXv1q+LxLsP5uw@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Sat, Jul 9, 2016 at 4:09 PM, Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr> wrote:
>
> While testing meta-command pgbench only scripts, I noticed that there is an
> infinite loop in threadRun, which means that other tasks such as reporting
> progress do not get a chance.
>
> The attached patch breaks this loop by always returning at the end of a
> script.
>
> On "pgbench -T 3 -P 1 -f noop.sql", before this patch, the progress is not
> shown, after it is.
You may want to name your patches with .patch or .diff. Using .sql is
disturbing style :)
Indeed, not reporting the progress back to the client in the case of a
script with only meta commands is non-intuitive.
- /* after a meta command, immediately proceed with next command */
- goto top;
+ /*
+ * After a meta command, immediately proceed with next command...
+ * although not if last. This exception ensures that a meta command
+ * only script does not always loop in doCustom, so that other tasks
+ * in threadRun, eg progress reporting or switching client,
get a chance.
+ */
+ if (commands[st->state + 1] != NULL)
+ goto top;
This looks good to me. I'd just rewrite the comment block with
something like that, more simplified:
+ /*
+ * After a meta command, immediately proceed with next command.
+ * But if this is the last command, just leave.
+ */
--
Michael
From | Date | Subject | |
---|---|---|---|
Next Message | Michael Paquier | 2016-07-11 04:51:03 | Re: pgbench - minor doc improvements |
Previous Message | Michael Paquier | 2016-07-11 04:04:34 | Re: Fix Error Message for allocate_recordbuf() Failure |