Re: Logging parallel worker draught

Alena Rybakina <a.rybakina@postgrespro.ru>

From: Alena Rybakina <a.rybakina@postgrespro.ru>
To: Benoit Lobréau <benoit.lobreau@dalibo.com>, pgsql-hackers <pgsql-hackers@postgresql.org>
Cc: "Imseih (AWS), Sami" <simseih@amazon.com>, Alvaro Herrera <alvherre@alvh.no-ip.org>, Robert Haas <robertmhaas@gmail.com>, Amit Kapila <amit.kapila16@gmail.com>, Melanie Plageman <melanieplageman@gmail.com>, "Andrey M. Borodin" <x4mmm@yandex-team.ru>
Date: 2024-10-14T20:42:09Z
Lists: pgsql-hackers

Attachments

Sorry, I was in a hurry and didn't check my patch properly.

On 14.10.2024 23:20, Alena Rybakina wrote:
> On 28.08.2024 15:58, Benoit Lobréau wrote:
>> Hi,
>>
>> Here is a new version of the patch. Sorry for the long delay, I was 
>> hit by a motivation drought and was quite busy otherwise.
>>
>> The guc is now called `log_parallel_workers` and has three possible 
>> values:
>>
>> * "none": disables logging
>> * "all": logs parallel worker info for all parallel queries or utilities
>> * "failure": logs only when the number of parallel workers planned 
>> couldn't be reached.
>>
>> For this, I added several members to the EState struct.
>>
>> Each gather node / gather merge node updates the values and the 
>> offending queries are displayed during standard_ExecutorEnd.
>>
>> For CREATE INDEX / REINDEX on btree and brin, I check the parallel 
>> context struct (pcxt) during _bt_end_parallel() or 
>> _brin_end_parallel() and display a log message when needed.
>>
>> For vacuum, I do the same in parallel_vacuum_end().
>>
>> I added some information to the error message for parallel queries as 
>> an experiment. I find it useful, but it can be removed, if you re not 
>> convinced.
>>
>> 2024-08-27 15:59:11.089 CEST [54585] LOG:  1 parallel nodes planned 
>> (1 obtained all their workers, 0 obtained none), 2 workers planned (2 
>> workers launched)
>> 2024-08-27 15:59:11.089 CEST [54585] STATEMENT:  EXPLAIN (ANALYZE)
>>         SELECT i, avg(j) FROM test_pql GROUP BY i;
>>
>> 2024-08-27 15:59:14.006 CEST [54585] LOG:  2 parallel nodes planned 
>> (0 obtained all their workers, 1 obtained none), 4 workers planned (1 
>> workers launched)
>> 2024-08-27 15:59:14.006 CEST [54585] STATEMENT:  EXPLAIN (ANALYZE)
>>         SELECT i, avg(j) FROM test_pql GROUP BY i
>>         UNION
>>         SELECT i, avg(j) FROM test_pql GROUP BY i;
>>
>> For CREATE INDEX / REDINDEX / VACUUMS:
>>
>> 2024-08-27 15:58:59.769 CEST [54521] LOG:  1 workers planned (0 
>> workers launched)
>> 2024-08-27 15:58:59.769 CEST [54521] STATEMENT:  REINDEX TABLE test_pql;
>>
>> Do you think this is better?
>>
>> I am not sure if a struct is needed to store the es_nworkers* and if 
>> the modification I did to parallel.h is ok.
>>
>> Thanks to: Jehan-Guillaume de Rorthais, Guillaume Lelarge and Franck 
>> Boudehen for the help and motivation boost. 
>
> Hi!
>
> I think it's a good idea to log this. I suggest some changes that 
> might improve your patch.
>
>
> 2. I think you can transfer nworkers_to_launch and nworkers_launched 
> vacuum parameters in the ParallelContext struct.

Sorry, I didn’t notice right away that the second point is impossible to 
fulfill here.


Attached is the correct version of the patch.

-- 
Regards,
Alena Rybakina
Postgres Professional