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.
The biggest concern I have on this topic is that we end up with zero
validation done for the memory accounting. If we can find a cheaper
place to put the Asserts that will at least bring our attention to the
fact that some problem exists, then more investigation can ensue. I
don't personally expect that every assert failure will show us the
exact location of the bug.
Additionally, I don't really think there is a huge amount of room for
bugs creeping in here as there's not all that many places that the
'mem_used' field gets updated, so there are not many places to forget
to do it.
Another way to look at this is that, where the Asserts are today,
there are zero memory accounting checks done in all cases that don't
evict any tuples. I feel by moving the tests to ExecEndResultCache()
we'll do memory validation for all plans using a Result Cache in
Assert builds. 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.
David