Re: Logging parallel worker draught - Mailing list pgsql-hackers

Hi, thank you for the review and sorry for the delayed answer.

On 12/28/24 05:17, Sami Imseih wrote:> Thinking about this further, it 
seems to me this logging only makes sense
 > for parallel query and not maintenance commands. This is because
 > for the latter case, the commands are executed manually and a user 
can issue
 > VACUUM VERBOSE ( for the case of vacuum ). For CREATE INDEX, one
 > can enable DEBUG1. For example:
 >
 > postgres=# CREATE INDEX tbl_c1 ON tbl(c1);
 > DEBUG:  building index "tbl_c1" on table "tbl" with request for 1
 > parallel workers
 > DEBUG:  index "tbl_c1" can safely use deduplication
 > CREATE INDEX


In my opinion, it's useful:

* Centralization of logs: The maintenance operation can be planned in
cron jobs. In that context, it could certainly be logged separately via
the script. However, when I am doing an audit on a client database, I
think it's useful to have all the relevant information in PostgreSQL logs.

* Centralization of the configuration: If I am auditing the usage of
parallelism, I would like to have a single GUC to enable the logging
for all relevant information.

* Context: Maintenance workers are part of the global pool of workers.
To know why there is a shortage after the fact, having all the information
on what was using workers could be useful. I tied this argument when we
added the parallel worker info to pg_stat_database and wasn't successful.
It would be nice to have the info somewhere.

The logging for CREATE INDEX and VACUUM is in separate patches. If you
don't mind, I would like to keep it as long as possible.

 > 2/ For logging parallel query workers, the log line could be simpler.
 >
 > Instead of:
 >
 > + elog(LOG, "%i parallel nodes planned (%i obtained all their workers,
 > %i obtained none), "
 > + "%i workers planned to be launched (%i workers launched)",
 >
 > what about something like:
 >
 > "launched %d parallel workers (planned: %)"
 >
 > For example, if I have 2 gather nodes and they each plan and launch
 > 2 workers, the log line should be as simple as
 >
 > "launched 4 parallel workers (planned: 4)"
 >
 > This behavior in which workers planned and launched are aggregated in the
 > log can be described in the documentation.

"%i parallel nodes planned (%i obtained all their workers, %i obtained 
none),"

I added this part during my first reorganization. I was trying to extract
as much information as possible and thought that the special case where a
parallel node was planned but no workers were obtained was the worst-case
scenario, which might be interesting to log.

If you think it doesn't bring much value, we can strip it.

 > 3/ Also, log_parallel_query_workers as the name of the GUC will make 
more sense
 > if we go logging parallel query only.

I agree that log_parallel_query_workers would make more sense if we focus
on logging parallel queries only. Since the maintenance-related logging
was added later, I missed this point. Thank you for bringging this up.

If agreeable, I will rename the GUC to log_parallel_workers as long as
the argument regarding maintenance workers is unresolved. If we decide
to strip them, I will revert it to log_parallel_query_workers.

 >> I don't think "failure" is a good name for the setting as it's
 >> a bit too harsh. What we really have is a "shortage" of
 >> workers.

I agree that "failure" is too harsh. The term "shortage" better describes
the situation.

Note: I added Tomas Vondra to the recipients of the email since he advised
me on this topic initially and might have an opinion on this.

-- 
Benoit Lobréau
Consultant
http://dalibo.com