| From: | Yugo Nagata <nagata(at)sraoss(dot)co(dot)jp> |
|---|---|
| To: | Fujii Masao <masao(dot)fujii(at)gmail(dot)com> |
| Cc: | Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com>, Rintaro Ikeda <ikedarintarof(at)oss(dot)nttdata(dot)com>, 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-13 07:09:29 |
| Message-ID: | 20251113160929.87534bc0cd6418060417dbb4@sraoss.co.jp |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Thu, 13 Nov 2025 11:55:25 +0900
Fujii Masao <masao(dot)fujii(at)gmail(dot)com> wrote:
> On Wed, Nov 12, 2025 at 6:34 PM Yugo Nagata <nagata(at)sraoss(dot)co(dot)jp> wrote:
> > I've attached an updated patch.
>
> Thanks for updating the patch!
>
> > The comment for the PQpipelineSync() call has been also updated to clarify
> > why it is necessary.
>
> + /*
> + * If a PGRES_PIPELINE_SYNC is followed by something other than
> + * PGRES_PIPELINE_SYNC or NULL, another PGRES_PIPELINE_SYNC will
> + * eventually follow.
> + */
>
> LGTM. I'd like to append "Reset received_sync to false to wait for
> it." into this comment.
>
>
> > In addition, I added a connection status check in the loop to avoid an
> > infinte loop that waiting for PQpipelineSync after a connection failure.
>
> Would it be better to move this status check right after PQgetResult()
> so that connection failures can be detected regardless of what result
> it returns?
>
> + pg_log_error("client %d aborted: the backend died while rolling back
> the failed transaction after",
>
> The trailing “after” seems unnecessary.
>
> Since there's no guarantee the backend actually died in this case,
> it might be better to use something like "client %d aborted while rolling back
> the transaction after an error; perhaps the backend died while processing"
> which matches the wording used under CSTATE_WAIT_ROLLBACK_RESULT
> in advanceConnectionState().
Thank you for your review!
I've attached an updated patch reflecting your suggestion.
Regards,
Yugo Nagata
--
Yugo Nagata <nagata(at)sraoss(dot)co(dot)jp>
| Attachment | Content-Type | Size |
|---|---|---|
| v3-0001-pgbench-Fix-assertion-failure-with-multiple-syncp.patch | text/x-diff | 3.2 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Joel Jacobson | 2025-11-13 07:13:28 | Re: Optimize LISTEN/NOTIFY |
| Previous Message | Peter Smith | 2025-11-13 07:02:20 | Re: [WIP]Vertical Clustered Index (columnar store extension) - take2 |