Date: Thu, 4 Mar 2021 15:23:33 +0800
From: Julien Rouhaud <rjuju123@gmail.com>
To: Michael Paquier <michael@paquier.xyz>
Cc: Tom Lane <tgl@sss.pgh.pa.us>, Georgios <gkokolatos@protonmail.com>,
	Julien Rouhaud <julien.rouhaud@free.fr>,
	PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>,
	"robertmhaas@gmail.com" <robertmhaas@gmail.com>
Subject: Re: Shared memory size computation oversight?
Message-ID: <20210304072333.4yydog47md7cfrda@nol>
References: <20210227080815.jryyhy6lrn6gahkm@nol>
 <SeVx6SxBxMHIm_66gE4M8rzAn5vqXw8akcyJ-26bJxHeNC_wW9fN-xDr_G3AGiSH328-rxMzXbNf6H4TsaNIG-OJFE8LvmdflG9Zb-yIKq4=@protonmail.com>
 <1183451.1614788634@sss.pgh.pa.us>
 <20210303164052.f336wina7qpchabd@nol>
 <YECGppkDKJArPOUb@paquier.xyz>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <YECGppkDKJArPOUb@paquier.xyz>
Archived-At: 
 <https://www.postgresql.org/message-id/20210304072333.4yydog47md7cfrda%40nol>
Precedence: bulk

On Thu, Mar 04, 2021 at 04:05:10PM +0900, Michael Paquier wrote:
> On Thu, Mar 04, 2021 at 12:40:52AM +0800, Julien Rouhaud wrote:
> > On Wed, Mar 03, 2021 at 11:23:54AM -0500, Tom Lane wrote:
> >> I have not looked at this patch, but I think the concern is basically that
> >> if we have space-estimation infrastructure that misestimates what it is
> >> supposed to estimate, then if that infrastructure is used to estimate the
> >> size of any of the "big hog" data structures, we might misestimate by
> >> enough that the slop factor wouldn't hide it.
> > 
> > Exactly.  And now that I looked deeper I can see that multiple estimates are
> > entirely ignoring the padding alignment (e.g. ProcGlobalShmemSize()), which can
> > exceed the 6kB originally estimated by Robert.
> 
> We are going to have a hard time catching up all the places that are
> doing an incorrect estimation, and have an even harder time making
> sure that similar errors don't happen in the future.  Should we add a 
> {add,mul}_shmem_size() to make sure that the size calculated is
> correctly aligned, that uses CACHELINEALIGN before returning the
> result size?

I was also considering adding new (add|mull)_*_size functions to avoid having
too messy code.  I'm not terribly happy with xxx_shm_size(), as not all call to
those functions would require an alignment.  Maybe (add|mull)shmemalign_size?

But before modifying dozens of calls, should we really fix those or only
increase a bit the "slop factor", or a mix of it?

For instance, I can see that for instance BackendStatusShmemSize() never had
any padding consideration, while others do.

Maybe only fixing contribs, some macro like PredXactListDataSize that already
do a MAXALIGN, SimpleLruShmemSize and hash_estimate_size() would be a short
patch and should significantly improve the estimation.