| From: | "Jelte Fennema-Nio" <postgres(at)jeltef(dot)nl> |
|---|---|
| To: | "Thomas Munro" <thomas(dot)munro(at)gmail(dot)com> |
| Cc: | "Heikki Linnakangas" <hlinnaka(at)iki(dot)fi>, "Bryan Green" <dbryan(dot)green(at)gmail(dot)com>, "pgsql-hackers" <pgsql-hackers(at)postgresql(dot)org>, "Tom Lane" <tgl(at)sss(dot)pgh(dot)pa(dot)us>, "Kyotaro Horiguchi" <horikyota(dot)ntt(at)gmail(dot)com> |
| Subject: | Re: Can we get rid of TerminateThread() in pg_dump? |
| Date: | 2026-07-08 08:47:52 |
| Message-ID: | DJT25I9FT73D.2I002MPUQ1ZM0@jeltef.nl |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Wed, Jul 8, 2026, 01:41 Thomas Munro <thomas(dot)munro(at)gmail(dot)com> wrote:
> On Tue, Jul 7, 2026 at 9:23 AM Heikki Linnakangas <hlinnaka(at)iki(dot)fi> wrote:
> > In this case, though, I think all we need is a "volatile sigatomic_t"
> > flag. Sending the query cancellation over the network surely acts as a
> > full compiler and memory barrier in the cancelling thread. And similarly
> > receiving the error message from the network acts as a full barrier in
> > the other threads that might receive the cancellation error from the
> > backend.
>
> You're right. So basically Jelte's patch, except it doesn't need the
> Win32 atomics stuff, just volatile, and a comment to explain that
> assumption. (Then some later version could use an explicit barrier
> instead of a comment, I guess, just to be clearer.)
Without any synchronization primitives there's still room for a
data-race, right? But I guess in practice that doesn't matter for the
messages we actually care about. Attached is a patch with that change,
altough I'm using "volatile bool" instead of "volatile sigatomic_t"
since there are no signal handlers involved here.
I also added !is_cancel_in_progress() checks in a few more places to
silence the places that Bryan had found.
To be clear: putting all of these threads together I'd like to remove
this function again completely. Instead I'd like this to use
CancelRequested for this, but that requires my PQblockingCancel patchset
to be merged first. And then I'd like to make CancelRequested a C11
atomic_flag instead of a volatile.
| Attachment | Content-Type | Size |
|---|---|---|
| v3-0001-pg_dump-Remove-TerminateThread-call.patch | text/x-patch | 12.8 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | shveta malik | 2026-07-08 08:52:31 | Re: Support EXCEPT for TABLES IN SCHEMA publications |
| Previous Message | Bertrand Drouvot | 2026-07-08 08:15:26 | Re: Add per-backend AIO statistics |