On Sat, May 08, 2021 at 02:26:44PM +1200, David Rowley wrote:
> On Sat, 8 May 2021 at 09:16, Tomas Vondra <tomas.vondra@enterprisedb.com> wrote:
> > On 5/7/21 11:04 PM, David Rowley wrote:
> > > Another thought I have is that maybe it would be ok just to move
> > > memory accounting debug code so it only runs once in
> > > ExecEndResultCache. I struggling to imagine that if the memory
> > > tracking did go out of whack, that the problem would have accidentally
> > > fixed itself by the time we got to ExecEndResultCache(). I guess even
> > > if the accounting was counting far too much memory and we'd evicted
> > > everything from the cache to try and get the memory usage down, we'd
> > > still find the problem during ExecEndResultCache(), even if the cache
> > > had become completely empty as a result.
> >
> > I don't think postponing the debug code until much later is a great
> > idea. When something goes wrong it's good to know ASAP, otherwise it's
> > much more difficult to identify the issue.
>
> I thought about this a bit and I was about to agree, but then I changed my mind.
> Yes, we might just need to do a bit more work to find
> out exactly where the problem is, but some investigation would need to
> happen anyway. I think if anyone changes anything which breaks the
> memory accounting then they'll be aware of it quite quickly and they
> can just look at what they did wrong.
You could put this into a separate function called by ExecEndResultCache().
Then anyone that breaks the memory accounting can also call the function in the
places they changed to help figure out what they broke.
- * Validate the memory accounting code is correct in assert builds. XXX is
- * this too expensive for USE_ASSERT_CHECKING?
--
Justin