Re: Logging parallel worker draught

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