Hi, On Tue, Dec 10, 2024 at 11:55:41AM -0600, Nathan Bossart wrote: > On Mon, Dec 09, 2024 at 04:41:03PM +0000, Bertrand Drouvot wrote: > > + <structfield>time_delayed</structfield> <type>bigint</type> > > I think it's also worth considering names like total_delay and > cumulative_delay.
That's fine by me. Then I think that total_delay is the way to go (I don't see any existing "cumulative_"). > > + Total amount of time spent in milliseconds waiting during <xref > > linkend="guc-vacuum-cost-delay"/> > > + or <xref linkend="guc-autovacuum-vacuum-cost-delay"/>. In case of > > parallel > > + vacuum the reported time is across all the workers and the leader. > > The > > + workers update the column no more frequently than once per second, > > so it > > + could show slightly old values. > > I wonder if it makes sense to provide this value as an interval instead of > the number of milliseconds to make it more human-readable. Yeah we could do so, but that would mean: 1. Write a dedicated "pg_stat_get_progress_info()" function for VACUUM. Indeed, the current pg_stat_get_progress_info() is shared across multiple "commands" and then we wouldn't be able to change it's output types in pg_proc.dat. Or 2. Make use of make_interval() in the pg_stat_progress_vacuum view creation. I don't like 1. that much and given that that would be as simple as: " select make_interval(secs => time_delayed / 1000) from pg_stat_progress_vacuum; " for an end user to display an interval, I'm not sure we should provide an interval by default. That said, I agree that milliseconds is not really human-readable and does not provide that much added value (except flexibility), so I'd vote for 2. if you feel we should provide an interval by default. > I might also > suggest some changes to the description: > > Total accumulated time spent sleeping due to the cost-based vacuum > delay settings (e.g., vacuum_cost_delay, vacuum_cost_limit). This > includes the time that any associated parallel workers have slept, too. > However, parallel workers report their sleep time no more frequently > than once per second, so the reported value may be slightly stale. > Yeah I like it, thanks! Now, I'm wondering if we should not also add something like this: " Since multiple workers can sleep simultaneously, the total sleep time can exceed the actual duration of the vacuum operation. " As that could be surprising to see this behavior in action. Thoughts? I'll provide an updated patch version once we agree on the above points. Regards, -- Bertrand Drouvot PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
