| From: | Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com> |
|---|---|
| To: | Fujii Masao <masao(dot)fujii(at)gmail(dot)com> |
| Cc: | Rintaro Ikeda <ikedarintarof(at)oss(dot)nttdata(dot)com>, Yugo Nagata <nagata(at)sraoss(dot)co(dot)jp>, Jakub Wartak <jakub(dot)wartak(at)enterprisedb(dot)com>, "Hayato Kuroda (Fujitsu)" <kuroda(dot)hayato(at)fujitsu(dot)com>, "slpmcf(at)gmail(dot)com" <slpmcf(at)gmail(dot)com>, "boekewurm+postgres(at)gmail(dot)com" <boekewurm+postgres(at)gmail(dot)com>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>, Srinath Reddy Sadipiralla <srinath2133(at)gmail(dot)com>, Dilip Kumar <dilipbalaut(at)gmail(dot)com> |
| Subject: | Re: Suggestion to add --continue-client-on-abort option to pgbench |
| Date: | 2025-11-11 02:49:16 |
| Message-ID: | A6E127C6-4C50-41CB-AF27-A681DCD03603@gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
> On Nov 10, 2025, at 12:45, Fujii Masao <masao(dot)fujii(at)gmail(dot)com> wrote:
>
> On Mon, Nov 10, 2025 at 11:07 AM Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com> wrote:
>> I just did a test. In the test, I inserted a tuple with the same primary key so that the inserts fails by the unique key constraint which breaks the pipeline, and some random select statements followed. And I added some debug messages in discardAvailableResults(), which showed me that the function will discard rest of statements’ results until \endpipeline. As there are anyway limited number of statements before \endpipeline, my concern is actually not valid. So, now I am good with this patch.
>
> Thanks a lot for testing!
>
Hi Fujii-san,
I just did more tests on both pipeline mode and non-pipeline mode, I think the main purpose of discardAvailableResults() is to drain results for pipeline mode. In non-pipeline mode, a NULL res indicates no more result to read; while in pipeline mode, when a pipeline is aborted, either a valid result or NULL could still be returned, thus we need to wait until pipeline state switch to PQ_PIPELINE_OK. From this perspective, the current inline comment is correct, but I feel it’s not clear enough.
So I am proposing the function comment and inline comment like the following:
```
/*
* Read and discard all available results from the connection.
*
* Non-pipeline mode:
* ------------------
* PQgetResult() returns each PGresult in order for the last command sent.
* When it returns NULL, that definitively means there are no more results
* for that command. We stop on NULL (or on CONNECTION_BAD).
*
* Pipeline mode:
* --------------
* If an earlier command in the pipeline errors, libpq enters the
* PQ_PIPELINE_ABORTED state. In this state, PQgetResult() may return
* either a valid PGresult or NULL, and a NULL return does NOT mean
* that the connection is drained. More results for later commands (or
* protocol housekeeping such as the pipeline sync result) can still
* arrive afterward. Therefore we must continue calling PQgetResult()
* while PQpipelineStatus(conn) == PQ_PIPELINE_ABORTED, even if we see
* intermittent NULLs.
*/
static void
discardAvailableResults(CState *st)
{
PGresult *res = NULL;
for (;;)
{
res = PQgetResult(st->con);
/*
* Stop when there are no more results *and* the pipeline is not
* in the aborted state, or if the connection has failed.
*/
if ((res == NULL && PQpipelineStatus(st->con) != PQ_PIPELINE_ABORTED) ||
PQstatus(st->con) == CONNECTION_BAD)
break;
PQclear(res);
}
PQclear(res);
}
```
What do you think?
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
| From | Date | Subject | |
|---|---|---|---|
| Next Message | jian he | 2025-11-11 03:06:11 | Re: CAST(... ON DEFAULT) - WIP build on top of Error-Safe User Functions |
| Previous Message | Chao Li | 2025-11-11 02:40:38 | Re: Suggestion to add --continue-client-on-abort option to pgbench |