Re: Add index scan progress to pg_stat_progress_vacuum

From: "Imseih (AWS), Sami" <simseih(at)amazon(dot)com>
To: Andres Freund <andres(at)anarazel(dot)de>, Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>
Cc: Robert Haas <robertmhaas(at)gmail(dot)com>, "Bossart, Nathan" <bossartn(at)amazon(dot)com>, Peter Geoghegan <pg(at)bowt(dot)ie>, Justin Pryzby <pryzby(at)telsasoft(dot)com>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Add index scan progress to pg_stat_progress_vacuum
Date: 2022-11-11 19:10:16
Message-ID: 7C22F915-5BB8-4729-8FF1-98B774CD6C85@amazon.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

> I don't think any of these progress callbacks should be done while pinning a
> buffer and ...

Good point.

> I also don't understand why info->parallel_progress_callback exists? It's only
> set to parallel_vacuum_progress_report(). Why make this stuff more expensive
> than it has to already be?

Agree. Modified the patch to avoid the callback .

> So each of the places that call this need to make an additional external
> function call for each page, just to find that there's nothing to do or to
> make yet another indirect function call. This should probably a static inline
> function.

Even better to just remove a function call altogether.

> This is called, for every single page, just to read the number of indexes
> completed by workers? A number that barely ever changes?

I will take the initial suggestion by Sawada-san to update the progress
every 1GB of blocks scanned.

Also, It sems to me that we don't need to track progress in brin indexes,
Since it is rare, if ever, this type of index will go through very heavy
block scans. In fact, I noticed the vacuum AMs for brin don't invoke the
vacuum_delay_point at all.

The attached patch addresses the feedback.

Regards,

Sami Imseih
Amazon Web Services (AWS)

Attachment Content-Type Size
v15-0001-Add-2-new-columns-to-pg_stat_progress_vacuum.-Th.patch application/octet-stream 21.4 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2022-11-11 20:05:58 Making Bitmapsets be valid Nodes
Previous Message Peter Geoghegan 2022-11-11 18:38:57 Re: Reducing the WAL overhead of freezing in VACUUM by deduplicating per-tuple freeze plans