From: Tom Lane <tgl@sss.pgh.pa.us>
To: "Bossart, Nathan" <bossartn@amazon.com>
cc: "pgsql-hackers@lists.postgresql.org" <pgsql-hackers@lists.postgresql.org>
Subject: Re: Reducing memory consumption for pending inval messages
In-reply-to: <584A570C-8B36-4A02-B8BC-2257DB5567A3@amazon.com>
References: <2380555.1622395376@sss.pgh.pa.us>
 <3380133.1626207700@sss.pgh.pa.us>
 <584A570C-8B36-4A02-B8BC-2257DB5567A3@amazon.com>
Comments: In-reply-to "Bossart, Nathan" <bossartn@amazon.com>
	message dated "Mon, 16 Aug 2021 20:14:25 -0000"
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <987628.1629145092.1@sss.pgh.pa.us>
Date: Mon, 16 Aug 2021 16:18:12 -0400
Message-ID: <987629.1629145092@sss.pgh.pa.us>
Archived-At: 
 <https://www.postgresql.org/message-id/987629.1629145092%40sss.pgh.pa.us>
Precedence: bulk

"Bossart, Nathan" <bossartn@amazon.com> writes:
> On 5/30/21, 10:22 AM, "Tom Lane" <tgl@sss.pgh.pa.us> wrote:
>> We can do a lot better, by exploiting what we know about the usage
>> patterns of invalidation requests.

> I spent some time looking through this patch, and it seems reasonable
> to me.

Thanks for reviewing!

>> There is one notable new assumption I had to make for this.  At end
>> of a subtransaction, we have to merge its inval events into the
>> "PriorCmd" list of the parent subtransaction.  (It has to be the
>> PriorCmd list, not the CurrentCmd list, because these events have
>> already been processed locally; we don't want to do that again.)
>> This means the parent's CurrentCmd list has to be empty at that
>> instant, else we'd be trying to merge sublists that aren't adjacent
>> in the array.  As far as I can tell, this is always true: the patch's
>> check for it doesn't trigger in a check-world run.  And there's an
>> argument that it must be true for semantic consistency (see comments
>> in patch).  So if that check ever fails, it probably means there is a
>> missing CommandCounterIncrement somewhere.  Still, this could use more
>> review and testing.

> I didn't discover any problems with this assumption in my testing,
> either.  Perhaps it'd be good to commit something like this sooner in
> the v15 development cycle to maximize the amount of coverage it gets.

Yeah, that's a good point.  I'll go push this.

			regards, tom lane