On Mon, Feb 26, 2024 at 3:54 PM Andrei Lepikhov <a.lepik...@postgrespro.ru> wrote:
> On 26/2/2024 12:44, Tender Wang wrote: > > Make sense. I found MemoizeState already has a MemoryContext, so I used > it. > > I update the patch. > This approach is better for me. In the next version of this patch, I > included a test case. I am still unsure about the context chosen and the > stability of the test case. Richard, you recently fixed some Memoize > issues, could you look at this problem and patch? I looked at this issue a bit. It seems to me what happens is that at first the memory areas referenced by probeslot->tts_values[] are allocated in the per tuple context (see prepare_probe_slot). And then in MemoizeHash_hash, after we've calculated the hashkey, we will reset the per tuple context. However, later in MemoizeHash_equal, we still need to reference the values in probeslot->tts_values[], which have been cleared. Actually the context would always be reset in MemoizeHash_equal, for both binary and logical mode. So I kind of wonder if it's necessary to reset the context in MemoizeHash_hash. The ResetExprContext call in MemoizeHash_hash was introduced in 0b053e78b to fix a memory leak issue. commit 0b053e78b5990cd01e7169ee5bd2bb8e4045deea Author: David Rowley <drow...@postgresql.org> Date: Thu Oct 5 20:30:47 2023 +1300 Fix memory leak in Memoize code It seems to me that switching to the per-tuple memory context is sufficient to fix the memory leak. Calling ResetExprContext in MemoizeHash_hash each time seems too aggressive. I tried to remove the ResetExprContext call in MemoizeHash_hash and did not see the memory leak with the repro query in [1]. diff --git a/src/backend/executor/nodeMemoize.c b/src/backend/executor/nodeMemoize.c index 18870f10e1..f2f025520d 100644 --- a/src/backend/executor/nodeMemoize.c +++ b/src/backend/executor/nodeMemoize.c @@ -207,7 +207,6 @@ MemoizeHash_hash(struct memoize_hash *tb, const MemoizeKey *key) } } - ResetExprContext(econtext); MemoryContextSwitchTo(oldcontext); return murmurhash32(hashkey); } Looping in David to have a look. [1] https://www.postgresql.org/message-id/flat/83281eed63c74e4f940317186372abfd%40cft.ru Thanks Richard