Message-ID: <3cccb1c0-4852-478c-9c5d-fbffea56ad53@iki.fi>
Date: Mon, 6 Jul 2026 02:21:56 +0300
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: pg_dump: use threads for parallel workers on all platforms
To: Bryan Green <dbryan.green@gmail.com>,
 PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Cc: Thomas Munro <thomas.munro@gmail.com>,
 Andrew Dunstan <andrew@dunslane.net>
References: <8c712d76-ecf7-4749-a6d8-dddc01f298ec@gmail.com>
Content-Language: en-US
From: Heikki Linnakangas <hlinnaka@iki.fi>
In-Reply-To: <8c712d76-ecf7-4749-a6d8-dddc01f298ec@gmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Archived-At: 
 <https://www.postgresql.org/message-id/3cccb1c0-4852-478c-9c5d-fbffea56ad53%40iki.fi>
Precedence: bulk

On 02/07/2026 19:30, Bryan Green wrote:
> None of this is broken; it works. It's threads pretending to be
> processes because the code was written for processes, and the port kept
> the protocol rather than rethinking it. I'd like to stop.
> 
> One model everywhere should be threads on all platforms, coordinated by
> an in-process work queue-- a mutex and a couple of condition variables--
> instead of two worker models bridged by an inter-process protocol.

+1

> To be clear, the unification is on the queue, not on what Windows does
> today. Teaching the non-Windows side to talk to its own threads over a
> socket, a byte at a time, would just be the same trick on more
> platforms-- that's the part worth deleting, not copying.
> 
> I've done the Windows half, both to prove it out and because it's the
> coordination layer the non-Windows side would adopt. ...

I see why you developed this that way, and it makes a lot of sense. 
However, it has one downside: the Windows-only code cannot be tested 
without Windows. At quick glance, it looks reasonable, but I'm a little 
nervous committing more Windows-only code without being able to easily 
play with it myself.

Once you have the final patch ready to switch non-Windows systems to 
threaded model too, that gets easier.

> With the thread rework, fmtId's static return value, is now
> _Thread_local.
+1. This is the first _Thread_local in our codebase. It's in C11, so it 
should just work, but we'll see if the buildfarm shows any surprises...

With this, the getLocalPQExpBuffer hook is never set. I think we can 
just remove it, and rename defaultGetLocalPQExpBuffer() to 
getLocalPQExpBuffer() directly.

I noticed that we currently call setFmtEncoding() in multiple places in 
src/bin/pg_dump. I haven't looked at them closely, but I wonder if 
there's some kind of thread-safety hazards there even without these patches.

And what about the 'quote_all_identifiers' global variable? Is that set 
correctly in both models? I guess it's inherited through fork().

- Heikki