Date: Wed, 18 Sep 2024 16:04:53 -0500
From: Nathan Bossart <nathandbossart@gmail.com>
To: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Cc: "Imseih (AWS), Sami" <simseih@amazon.com>,
	Masahiko Sawada <sawada.mshk@gmail.com>,
	"pgsql-hackers@lists.postgresql.org" <pgsql-hackers@lists.postgresql.org>,
	Robert Haas <robertmhaas@gmail.com>
Subject: Re: Track the amount of time waiting due to cost_delay
Message-ID: <ZutAdT9aIpHOlloy@nathan>
References: <ZmgKL8zUm3qxbY9V@ip-10-97-1-34.eu-west-3.compute.internal>
 <ZmreasV0BKZ5BHIM@ip-10-97-1-34.eu-west-3.compute.internal>
 <ZnbIIUQpFJKAJx2W@ip-10-97-1-34.eu-west-3.compute.internal>
 <ZnlPZZZJCRu/8fka@ip-10-97-1-34.eu-west-3.compute.internal>
 <42B4800A-D8D0-4734-930A-4D94D0EA90C7@amazon.com>
 <8D4DC535-1C51-44A9-ABDD-420F975AE8A9@amazon.com>
 <ZoI3rS2BdNi+kCWS@ip-10-97-1-34.eu-west-3.compute.internal>
 <ZsSQnS9OW9EWSOk4@ip-10-97-1-34.eu-west-3.compute.internal>
 <ZtVJCIu1MvS214po@ip-10-97-1-34.eu-west-3.compute.internal>
 <Ztk6yqPYAzAM7bkf@ip-10-97-1-34.eu-west-3.compute.internal>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <Ztk6yqPYAzAM7bkf@ip-10-97-1-34.eu-west-3.compute.internal>
Archived-At: <https://www.postgresql.org/message-id/ZutAdT9aIpHOlloy%40nathan>
Precedence: bulk

On Thu, Sep 05, 2024 at 04:59:54AM +0000, Bertrand Drouvot wrote:
> Please find attached v6, a mandatory rebase due to catversion bump conflict.
> I'm removing the catversion bump from the patch as it generates too frequent
> conflicts (just mention it needs to be done in the commit message).

v6 looks generally reasonable to me.  I think the
nap_time_since_last_report variable needs to be marked static, though.

One thing that occurs to me is that this information may not be
particularly useful when parallel workers are used.  Without parallelism,
it's easy enough to figure out the percentage of time that your VACUUM is
spending asleep, but when there are parallel workers, it may be hard to
deduce much of anything from the value.  I'm not sure that this is a
deal-breaker for the patch, though, if for no other reason than it'll most
likely be used for autovacuum, which doesn't use parallel vacuum yet.

If there are no other concerns, I'll plan on committing this one soon after
a bit of editorialization.

-- 
nathan