Message-ID: <0d9c391f-5014-48f5-a680-45d7cfabe8a8@gmail.com>
Date: Sat, 4 Jul 2026 10:24:24 -0500
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: Can we get rid of TerminateThread() in pg_dump?
To: Jelte Fennema-Nio <postgres@jeltef.nl>,
 Thomas Munro <thomas.munro@gmail.com>
Cc: Heikki Linnakangas <hlinnaka@iki.fi>,
 pgsql-hackers <pgsql-hackers@postgresql.org>, Tom Lane <tgl@sss.pgh.pa.us>,
 Kyotaro Horiguchi <horikyota.ntt@gmail.com>
References: 
 <CA+hUKGJgO=o-vLFahGdR2WesuX3h1-0j=a8z72fChc-MG1Hveg@mail.gmail.com>
 <56821e0d-bfd5-4619-b13f-12e28c78289b@iki.fi>
 <faea6e93-32ab-46ad-8caa-4f502cb3e63c@gmail.com>
 <CA+hUKGL0ZeiO0ND+ksAi5inRY98GEGtyM4t8CVhF1mZaAqYjVA@mail.gmail.com>
 <DJPQS3FYSD4U.3DBTXA6U8IQ0Q@jeltef.nl>
Content-Language: en-US
From: Bryan Green <dbryan.green@gmail.com>
In-Reply-To: <DJPQS3FYSD4U.3DBTXA6U8IQ0Q@jeltef.nl>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Archived-At: 
 <https://www.postgresql.org/message-id/0d9c391f-5014-48f5-a680-45d7cfabe8a8%40gmail.com>
Precedence: bulk

On 7/4/2026 6:15 AM, Jelte Fennema-Nio wrote:
> On Sat, 4 Jul 2026 at 02:51, Thomas Munro <thomas.munro@gmail.com> wrote:
>> We don't actually care about the threads
>> themselves, and it doesn't seem that great if we have to introduce an
>> IPC ping-pong of some kind with each thread.  
> 
> Agreed. But I do agree with Heikki that swapping out stderr seems pretty
> hacky. At the very least because now the main thread cannot write to
> stderr either anymore (which is why you removed the "terminated by user"
> write I guess).
One correction from running it on Windows -- the "terminated by user"
line still prints.  Thomas moved that write ahead of the dup2(), so it
goes out before stderr is redirected.
> 
> How about instead we do something like the attached?
> 
> To be clear, I do think we should stop using TerminateThread because I
> wanna replace PQcancel there with PQcancelBlocking[1] PQcancelBlocking
> does a whole TLS handshake, which is almost certainly taking some locks.
> 
> Note that the way to achieve that I moved the Ctrl+C handler to a
> dedicated thread on Unix too, so it starts behaving the same as Windows
> in that respect. I think combined with you changing pg_dump to use
> worker *threads* on Unix too, we would then get pretty much identical
> behaviour across OSes for pg_dump.
> 
> P.S. I now realize that anything involving the (already existing)
> CancelRequested flag is actually not actually safe/correct on Windows,
> because it's not using read nor written using atomic operations while
> the consoleHandler runs on its own thread. Would be good to fix that too
> I guess, but it seems that for now it has worked in practice at least.
> 
> [1]: https://www.postgresql.org/message-id/flat/
> DJPAH0WPJV3K.1PYZ8P0QXZVMX%40jeltef.nl#5642e337a4e4d04b21c66e089484f80d
> 

I ran v2 (both patches) on Windows: parallel restore, -j4, into a dump
with PK + secondary indexes so there is a CREATE INDEX phase to
interrupt. I then ^C in the middle of the index build.  The workers
canceled cleanly (no orphaned backends, process exits, no hang.  No
"query failed" spam. Your guard swallows them.

I don't know whether this is worth restructuring the code for, but at -v
you will get (as an example)
   pg_restore: while PROCESSING TOC:
   pg_restore: from TOC entry NNNN; ... INDEX t1_v_idx postgres

Those context lines sit above is_cancel_in_progress() check in
warn_or_exit_horribly... so the preamble does not get suppressed.

The die_on_query_failure is unguarded, it might spam if a ^C happens to
land for metadata/SET queries. I did not test that.

I may roll your patches onto my branch to test with the multi-threaded
queue.

-- 
Bryan Green
EDB: https://www.enterprisedb.com