Re: Use pgBufferUsage for block reporting in analyze

Hi,

On Tue, Jul 30, 2024 at 1:13 AM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
> I have one comment on 0001 patch:
> The comment should also be updated or removed.

Ok, I've removed the comment.

> However, as the above comment says, delay_in_ms can have a duration up
> to 25 days. I guess it would not be a problem for analyze cases but
> could be in vacuum cases as vacuum could sometimes be running for a
> very long time. I've seen vacuums running even for almost 1 month. So
> I think we should keep this part.

Good point, I've reverted to using TimestampDifference for vacuum.

>          /* measure elapsed time iff autovacuum logging requires it */
> -        if (AmAutoVacuumWorkerProcess() && params->log_min_duration >= 0)
> +        if (instrument)
>
> The comment should also be updated.

Updated.

> Could you split the 0002 patch into two patches? One is to have
> ANALYZE command (with VERBOSE option) write the buffer usage, and
> second one is to add WAL usage to both ANALYZE command and
> autoanalyze. I think adding WAL usage to ANALYZE could be
> controversial as it should not be WAL-intensive operation, so I'd like
> to keep them separate.

I've split the patch, 0002 makes verbose outputs the same as
autoanalyze logs with buffer/io/system while 0003 adds WAL usage
output.

> It seems to me that the current style is more concise and readable (3
> rows per table vs. 1 row per table). We might need to consider a
> better output format for partitioned tables as the number of
> partitions could be high. I don't have a good idea now, though.

A possible change would be to pass an inh flag when an acquirefunc is
called from acquire_inherited_sample_rows. The acquirefunc could then
use an alternative log format to append to logbuf. This way, we could
have a more compact format for partitioned tables.

Regards,
Anthonin