Feedback-ID: id4a34324:Fastmail
Date: Wed, 8 Nov 2023 09:20:44 -0800
From: Andres Freund <andres@anarazel.de>
To: Stephen Frost <sfrost@snowman.net>
Cc: John Morris <john.morris@crunchydata.com>,
	Andrei Lepikhov <a.lepikhov@postgrespro.ru>,
	"reid.thompson@crunchydata.com" <reid.thompson@crunchydata.com>,
	Arne Roland <A.Roland@index.de>,
	"pgsql-hackers@lists.postgresql.org" <pgsql-hackers@lists.postgresql.org>,
	vignesh C <vignesh21@gmail.com>,
	Justin Pryzby <pryzby@telsasoft.com>,
	Ibrar Ahmed <ibrar.ahmad@gmail.com>,
	"stephen.frost" <stephen.frost@crunchydata.com>
Subject: Re: Add the ability to limit the amount of memory that can be
 allocated to backends.
Message-ID: <20231108172044.u6i5lw5j2euuhsdr@awork3.anarazel.de>
References: <BYAPR13MB26776A35AB57940680D4CE0EA0A0A@BYAPR13MB2677.namprd13.prod.outlook.com>
 <20231104041900.27pgqsknb3yl45zy@awork3.anarazel.de>
 <ZUkqSqdGcu4OLoOd@tamriel.snowman.net>
 <20231107195506.4i2tk352cvfs44t6@awork3.anarazel.de>
 <ZUqkVEX9TBOkM8cq@tamriel.snowman.net>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <ZUqkVEX9TBOkM8cq@tamriel.snowman.net>
Archived-At: <https://www.postgresql.org/message-id/20231108172044.u6i5lw5j2euuhsdr%40awork3.anarazel.de>
Precedence: bulk

Hi,

On 2023-11-07 15:55:48 -0500, Stephen Frost wrote:
> * Andres Freund (andres@anarazel.de) wrote:
> > On 2023-11-06 13:02:50 -0500, Stephen Frost wrote:
> > > > > The max_total_memory limit is checked whenever the global counters are
> > > > > updated. There is no special error handling if a memory allocation exceeds
> > > > > the global limit. That allocation returns a NULL for malloc style
> > > > > allocations or an ENOMEM for shared memory allocations. Postgres has
> > > > > existing mechanisms for dealing with out of memory conditions.
> > > > 
> > > > I still think it's extremely unwise to do tracking of memory and limiting of
> > > > memory in one patch.  You should work towards and acceptable patch that just
> > > > tracks memory usage in an as simple and low overhead way as possible. Then we
> > > > later can build on that.
> > > 
> > > Frankly, while tracking is interesting, the limiting is the feature
> > > that's needed more urgently imv.
> > 
> > I agree that we need limiting, but that the tracking needs to be very robust
> > for that to be usable.
> 
> Is there an issue with the tracking in the patch that you saw?  That's
> certainly an area that we've tried hard to get right and to match up to
> numbers from the rest of the system, such as the memory context system.

There's some details I am pretty sure aren't right - the DSM tracking piece
seems bogus to me. But beyond that: I don't know. There's enough other stuff
in the patch that it's hard to focus on that aspect. That's why I'd like to
merge a patch doing just that, so we actually can collect numbers. If any of
the developers of the patch had focused on polishing that part instead of
focusing on the limiting, it'd have been ready to be merged a while ago, maybe
even in 16.  I think the limiting piece is unlikely to be ready for 17.

Greetings,

Andres Freund