Date: Sat, 12 Mar 2022 13:20:04 -0800
From: Nathan Bossart <nathandbossart@gmail.com>
To: "Imseih (AWS), Sami" <simseih@amazon.com>
Cc: Masahiko Sawada <sawada.mshk@gmail.com>, Peter Geoghegan <pg@bowt.ie>,
	"Bossart, Nathan" <bossartn@amazon.com>,
	Justin Pryzby <pryzby@telsasoft.com>,
	"pgsql-hackers@postgresql.org" <pgsql-hackers@postgresql.org>
Subject: Re: Add index scan progress to pg_stat_progress_vacuum
Message-ID: <20220312212004.GA848429@nathanxps13>
References: <20220225195253.GA663211@nathanxps13>
 <B741F5D1-2B29-4CD6-8938-E6265AB45476@amazon.com>
 <CAD21AoA4kNdG6Qi8Qr4vFpvrGKd9MqjVPLUVZMEoBWHdCqfu9Q@mail.gmail.com>
 <F5F2CD8C-49D4-4B9A-983F-E3FF0E8CF048@amazon.com>
 <CAD21AoBW6SMJ96CNoMeu+f_BR4jmatPcfVA016FdD2hkLDsaTA@mail.gmail.com>
 <D4BBFE30-AB76-49E1-9A47-5A2851CCCB9F@amazon.com>
 <CAD21AoDf4pf89i7fgzrHDdJueNHLm4x92xHjcDeYaV+pdhem-Q@mail.gmail.com>
 <99A5CBE8-EC10-42AA-ABB4-4C812EEFD294@amazon.com>
 <20220310005237.GA307769@nathanxps13>
 <DD7EEE19-EFFB-481C-A6EC-26B1E0356F7A@amazon.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <DD7EEE19-EFFB-481C-A6EC-26B1E0356F7A@amazon.com>
Archived-At: 
 <https://www.postgresql.org/message-id/20220312212004.GA848429%40nathanxps13>
Precedence: bulk

On Sat, Mar 12, 2022 at 07:00:06AM +0000, Imseih (AWS), Sami wrote:
>> nitpick: Can we remove the extra spaces in the parentheses?
> 
> fixed
> 
>> What does it mean if there isn't an entry in the map?  Is this actually
>> expected, or should we ERROR instead?
> 
> I cleaned up the code here and added comments. 
> 
>> I think the number of entries should be shared between
>> VacuumWorkerProgressShmemInit() and VacuumWorkerProgressShmemSize().
>> Otherwise, we might update one and not the other.
> 
> Fixed
> 
>> I think we should elaborate a bit more in this comment.  It's difficult to
>> follow what this is doing without referencing the comment above
>> set_vacuum_worker_progress().
> 
> More comments added
> 
> I also simplified the 0002 patch as well.

These patches look pretty good to me.  Barring additional feedback, I
intend to mark this as ready-for-committer early next week.

-- 
Nathan Bossart
Amazon Web Services: https://aws.amazon.com