Re: Logging parallel worker draught
Benoit Lobréau <benoit.lobreau@dalibo.com>
From: Benoit Lobréau <benoit.lobreau@dalibo.com>
To: Melanie Plageman <melanieplageman@gmail.com>,
Sami Imseih <samimseih@gmail.com>
Cc: Tomas Vondra <tomas@vondra.me>, Guillaume Lelarge
<guillaume@lelarge.info>, Alena Rybakina <a.rybakina@postgrespro.ru>,
pgsql-hackers <pgsql-hackers@postgresql.org>,
"Imseih (AWS), Sami" <simseih@amazon.com>,
Alvaro Herrera <alvherre@alvh.no-ip.org>, Robert Haas
<robertmhaas@gmail.com>, Amit Kapila <amit.kapila16@gmail.com>,
"Andrey M. Borodin" <x4mmm@yandex-team.ru>
Date: 2025-04-14T14:41:31Z
Lists: pgsql-hackers
Attachments
- v9-0001-Add-a-guc-for-parallel-worker-logging.patch (text/x-patch) patch v9-0001
- v9-0002-Implements-logging-for-parallel-worker-usage-in-util.patch (text/x-patch) patch v9-0002
- v9-0003-Implements-logging-for-parallel-worker-usage-in-quer.patch (text/x-patch) patch v9-0003
On 4/7/25 6:41 PM, Melanie Plageman wrote: > On Mon, Feb 3, 2025 at 12:37 AM Sami Imseih <samimseih@gmail.com> wrote: > I started looking at this, and I like the idea. Thanks for taking a look! > A few comments: I don't understand what 0002 is. For starters, the > commit message says something about pg_stat_database, and there are no > changes related to that. I had originally split this part out while working on the patch to add parallel worker stats in pg_stat_database [1], in order to isolate the common components. In the end, that patch only accounted for user queries. I merged it into "Implements logging for parallel worker usage in utilities" for v9. > Also, we already have basically identical logging coming from > parallel_vacuum_process_all_indexes() and viewable in existing output. > Not only does your implementation not replace this, it is odd that > setting your new guc to none does not disable this. It seems a bit > inconsistent. I'm not sure what the exact right behavior is here, > though. That logging is used for the VERBOSE mode of VACUUM. There was also dicussion to add similar info for parallel index creation. The use case here is different — the goal is to audit parallel worker usage across the entire instance, without needing every call site to use VACUUM (VERBOSE) along with SET log_min_messages = info. I avoided reusing that part of the code because I thought the expectation was to aggregate worker counts and display them in parallel_vacuum_end(). Sami also mentionned that using the same log line everywhere in the patch would make parsing easier, which I tought was a good idea. > Since your last update, it seems parallel gin index build has been > committed, so perhaps you want to add that. Thanks for the heads-up! I've added logging in _gin_end_parallel(). You’ll find the updated patch attached. [1] https://commitfest.postgresql.org/patch/5212/ -- Benoit