Thread: Re: improve DEBUG1 logging of parallel workers for CREATE INDEX?

Re: improve DEBUG1 logging of parallel workers for CREATE INDEX?

From
Bertrand Drouvot
Date:
Hi,

On Fri, Jan 03, 2025 at 12:30:14PM -0600, Sami Imseih wrote:
> While reviewing patch [1], I realized that the DEBUG1 message
> for CREATE INDEX could do better in providing information
> about parallel workers launched. Currently, the message just
> shows how many workers are planned, but a user may want
> to ensure that they have the appropriate number of workers
> launched as well when they enable DEBUG1 logging.

Yeah, one could see how many workers are currently running but would not be
able to figure out once the index is created. So adding the info in the log
makes sense to me.

> I prepared a simple patch, attached, for this. The log message matches
> the format
> used in VACUUM VERBOSE ( for consistency sake ).

A few random comments:

=== 1

s/parallel vacuum workers for index vacuuming/parallel workers for index creation/?

(2 times)

=== 2

-  (errmsg_internal("building index \"%s\" on table \"%s\" with request for %d parallel workers",
+  (errmsg_internal("building index \"%s\" on table \"%s\"",

I'd add "in parallel" to match its counterpart "serially" above. That would
make it more clear in case one just look for "building index" in the log.

Regards,

-- 
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com



Re: improve DEBUG1 logging of parallel workers for CREATE INDEX?

From
Guillaume Lelarge
Date:
Hi,

Le mer. 8 janv. 2025 à 18:37, Sami Imseih <samimseih@gmail.com> a écrit :
> s/parallel vacuum workers for index vacuuming/parallel workers for index creation/?

oops, that's my oversight from copying the message from vacuum. fixed.

> I'd add "in parallel" to match its counterpart "serially" above. That would
> make it more clear in case one just look for "building index" in the log.

good point.

Below is the new output with the attached v2.

postgres=# create index foo_idx1 on foo(id);
DEBUG:  building index "foo_idx1" on table "foo" with parallel workers
DEBUG:  launched 1 parallel workers for index creation (planned: 1)
DEBUG:  index "foo_idx1" can safely use deduplication
CREATE INDEX


I tend to agree it might be better than Benoit's patch on the index messages, though I'm afraid that DEBUG1 level won't work for many users. DEBUGx are for developers, not users. A better log level for me would be LOG. For client messages, LOG messages won't be displayed by default. So there's still a need for a "SET client_min_messages to LOG" but it won't be necessary to go all the way to DEBUG1.

Regards.


--
Guillaume.

Re: improve DEBUG1 logging of parallel workers for CREATE INDEX?

From
Tom Lane
Date:
Guillaume Lelarge <guillaume@lelarge.info> writes:
> I tend to agree it might be better than Benoit's patch on the index
> messages, though I'm afraid that DEBUG1 level won't work for many users.
> DEBUGx are for developers, not users. A better log level for me would be
> LOG.

Please, no.  That would result in spamming the postmaster log
for perfectly ordinary actions, with no usable way to prevent it.

I think DEBUG is perfectly appropriate for this sort of message.

            regards, tom lane



Re: improve DEBUG1 logging of parallel workers for CREATE INDEX?

From
Sami Imseih
Date:
> I think DEBUG is perfectly appropriate for this sort of message.

I am curious what are the thoughts on introducing a
CREATE INDEX VERBOSE which can provide this info?
similar to users scripting VACUUM VERBOSE to log
more details about the vacuum operation including parallel
usage.

I have not convinced myself this is a good idea, but maybe
someone has an opinion. If not, we can just stick with a better
DEBUG1 message.

Regards,

Sami



Re: improve DEBUG1 logging of parallel workers for CREATE INDEX?

From
Guillaume Lelarge
Date:
Le mer. 8 janv. 2025 à 21:35, Tom Lane <tgl@sss.pgh.pa.us> a écrit :
Guillaume Lelarge <guillaume@lelarge.info> writes:
> I tend to agree it might be better than Benoit's patch on the index
> messages, though I'm afraid that DEBUG1 level won't work for many users.
> DEBUGx are for developers, not users. A better log level for me would be
> LOG.

Please, no.  That would result in spamming the postmaster log
for perfectly ordinary actions, with no usable way to prevent it.


Not sure why I was thinking it would only go to the client. You're right, it will also go to the logs, and it will be stored by default, which would be bad. Sorry about this...
 
I think DEBUG is perfectly appropriate for this sort of message.


Still think DEBUG level is pretty bad for a user experience. Anyway, I much prefer a DEBUG message than no message at all :)
 

--
Guillaume.

Re: improve DEBUG1 logging of parallel workers for CREATE INDEX?

From
Guillaume Lelarge
Date:
Le mer. 8 janv. 2025 à 21:44, Sami Imseih <samimseih@gmail.com> a écrit :
> I think DEBUG is perfectly appropriate for this sort of message.

I am curious what are the thoughts on introducing a
CREATE INDEX VERBOSE which can provide this info?
similar to users scripting VACUUM VERBOSE to log
more details about the vacuum operation including parallel
usage.

I have not convinced myself this is a good idea, but maybe
someone has an opinion. If not, we can just stick with a better
DEBUG1 message.


Sounds better to me than relying on DEBUG messages. So, yeah, VERBOSE has my vote.


--
Guillaume.

Re: improve DEBUG1 logging of parallel workers for CREATE INDEX?

From
Tom Lane
Date:
Sami Imseih <samimseih@gmail.com> writes:
> I am curious what are the thoughts on introducing a
> CREATE INDEX VERBOSE which can provide this info?
> similar to users scripting VACUUM VERBOSE to log
> more details about the vacuum operation including parallel
> usage.

What I can recall being discussed in the past is to extend
EXPLAIN and/or EXPLAIN ANALYZE to cover utility statements
that have nontrivial execution complexity --- for example,
ALTER TABLE has a lot of machinery underneath, and people
often wish to know things like whether a particular ALTER
will cause a table rewrite or not.

Of course, a patch for that would be a few orders of magnitude
larger than what you've got here :-(.  But if you're looking
for a framework for reporting these sorts of details, I'd
much rather go in that direction than follow the model of
VACUUM VERBOSE.  VACUUM VERBOSE is a kluge with little to
recommend it other than having been easy to implement.

            regards, tom lane



Re: improve DEBUG1 logging of parallel workers for CREATE INDEX?

From
Sami Imseih
Date:
> Of course, a patch for that would be a few orders of magnitude
> larger than what you've got here :-(.  But if you're looking
> for a framework for reporting these sorts of details, I'd
> much rather go in that direction than follow the model of
> VACUUM VERBOSE.  VACUUM VERBOSE is a kluge with little to
> recommend it other than having been easy to implement.

To my surprise, REINDEX does have a VERBOSE option.
should have check this earlier :)

postgres=# reindex (verbose) index t_idx1;
INFO:  index "t_idx1" was reindexed
DETAIL:  CPU: user: 5.33 s, system: 0.48 s, elapsed: 6.26 s
REINDEX

Is there a reason not to do the same for CREATE INDEX?

Also, we can improve the REINDEX verbose message by
also providing the parallel usage.

Regards,

Sami