| From: | Etsuro Fujita <etsuro(dot)fujita(at)gmail(dot)com> |
|---|---|
| To: | Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com> |
| Cc: | PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
| Subject: | Re: Clarify wait versus poll behavior in ExecAppendAsyncEventWait |
| Date: | 2026-11-09 09:15:46 |
| Message-ID: | CAPmGK144fg2XeeYaviP0x7Ge-h-MHYBw-efm1FW3ObOL=eSGXA@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
Hi Chao,
On Thu, Nov 5, 2026 at 2:59 PM Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com> wrote:
> This is a follow-up to patch [1] that I recently reviewed. The header comment of ExecAppendAsyncEventWait can be enhanced, but Etsuro-san considered that a pure improvement, so he asked for a separate patch.
Thanks for the patch! Here are my comments:
* ExecAppendAsyncEventWait
*
* Wait or poll for file descriptor events and fire callbacks.
+ *
+ * If node->as_syncdone is true, wait until at least one event
+ * occurs; otherwise, poll without waiting so synchronous
+ * execution can proceed.
I don't think this is 100% correct, because even when polling for
events, we will do async execution if some were ready.
BTW: I noticed we are clarifying this by this comment within that function:
/*
* If the timeout is -1, wait until at least one event occurs. If the
* timeout is 0, poll for events, but do not wait at all.
*/
This seems more accurate to me, so how about modifying the latter of
the proposed comments using this, like the attached? This change
makes the above comment redundant, so I removed it and instead added a
brief comment.
Best regards,
Etsuro Fujita
| Attachment | Content-Type | Size |
|---|---|---|
| v2-0001-Clarify-when-ExecAppendAsyncEventWait-waits-or-po.patch | application/octet-stream | 1.1 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Bertrand Drouvot | 2026-11-09 09:16:39 | Re: Redesign per-backend statistics |
| Previous Message | Jim Jones | 2026-11-09 09:06:12 | Re: Reject access to unlogged tables in contrib modules |