From: Tomas Vondra <tomas.vondra@2ndquadrant.com>
To: James Coleman <jtc331@gmail.com>
Cc: Michael Paquier <michael@paquier.xyz>
Cc: Rafia Sabih <rafia.pghackers@gmail.com>
Cc: Peter Geoghegan <pg@bowt.ie>
Cc: Simon Riggs <simon@2ndquadrant.com>
Cc: Shaun Thomas <shaun.thomas@2ndquadrant.com>
Cc: Dmitry Dolgov <9erthalion6@gmail.com>
Cc: Alexander Korotkov <a.korotkov@postgrespro.ru>
Cc: Tom Lane <tgl@sss.pgh.pa.us>
Cc: PostgreSQL Developers <pgsql-hackers@lists.postgresql.org>
Cc: Alvaro Herrera <alvherre@2ndquadrant.com>
Subject: Re: [PATCH] Incremental sort (was: PoC: Partial sort)
Date: Sat, 14 Mar 2020 20:58:22 +0100
Message-ID: <20200314195822.bc2mqyvolo2w5l2w@development> (raw)
In-Reply-To: <CAAaqYe_NjXv9TZ3rjO=hMzL3KvEj-9Ado3qUKRS3F0DCcS3R4g@mail.gmail.com>
References: <CAAaqYe-BV=kuMmwHCnOfdAG_mKjCo=41MeFrdtBjB+iXd6-FJg@mail.gmail.com>
<20200311024355.5h2v56jpgzvd7yox@development>
<CAAaqYe8-TCKPskcMytCMX2aM8QnnrgcJP6=tSnJOuQ2CcuQJfg@mail.gmail.com>
<CAAaqYe-KCy3tW6j2VLi++C6Z+d1wgi81sOxiKo0+HWVR2noNLg@mail.gmail.com>
<20200314001254.gdusqeanl7wrxmr7@development>
<CAAaqYe_cVOS3akUic2PSaEW0noB=9bBxMkkDuj0HBZJqHHh5yQ@mail.gmail.com>
<CAAaqYe_ZpNFJXGTN1VFNqnOLxLfqib0OZ7L-UGP_MAdZi0WbiA@mail.gmail.com>
<CAAaqYe9Y6Hc35N2+eRMhPvc1YJ1LhdZjzT8k7J1KGcx_JC+_Kw@mail.gmail.com>
<CAAaqYe_9eQoZZ1rS8pXy+3AdkAHjejB0ANbh+-ipv04NcxxCYQ@mail.gmail.com>
<CAAaqYe_NjXv9TZ3rjO=hMzL3KvEj-9Ado3qUKRS3F0DCcS3R4g@mail.gmail.com>
On Sat, Mar 14, 2020 at 02:41:09PM -0400, James Coleman wrote:
>
>It looks like the issue is actually into the `tuplecontext`, which is
>currently a child context of `sortcontext`:
>
>#3 0x0000558cd153b565 in AllocSetCheck
>(context=context@entry=0x558cd28e0b70) at aset.c:1573
>1573 Assert(total_allocated == context->mem_allocated);
>(gdb) p total_allocated
>$1 = 16384
>(gdb) p context->mem_allocated
>$2 = 8192
>(gdb) p context->name
>$3 = 0x558cd16c8ccd "Caller tuples"
>
>I stuck in several more AllocSetCheck calls in aset.c and got the
>attached backtrace.
>
I think the problem is pretty simple - tuplesort_reset does call
tuplesort_reset, which resets the sortcontext. But that *deletes* the
tuplecontext, so the state->tuplecontext gets stale. I'd haven't looked
into the exact details, but it clearly confuses the accouting.
The attached patch fixes the issue for me - I'm not claiming it's the
right fix, but it's the simplest thing I could think of. Maybe the
tuplesort_rest should work differently, not sure.
And it seems to resolve the memory leak too - I suspect we've freed the
context (so it was not part of the tree of contexts) but the struct was
still valid and we kept allocating memory in it - but it was invisible
to MemoryContextDump etc.
regards
--
Tomas Vondra http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
diff --git a/src/backend/utils/sort/tuplesort.c b/src/backend/utils/sort/tuplesort.cindex c2bd38f39f..91c0189577 100644--- a/src/backend/utils/sort/tuplesort.c+++ b/src/backend/utils/sort/tuplesort.c@@ -1419,6 +1419,10 @@ tuplesort_reset(Tuplesortstate *state)
state->slabFreeHead = NULL;
state->growmemtuples = true;
+ state->tuplecontext = AllocSetContextCreate(state->sortcontext,+ "Caller tuples",+ ALLOCSET_DEFAULT_SIZES);+
if (state->memtupsize < INITIAL_MEMTUPSIZE)
{
if (state->memtuples)
Attachments:
[text/plain] tuplesort-fix.patch (536B, ../20200314195822.bc2mqyvolo2w5l2w@development/2-tuplesort-fix.patch)
download | inline diff:diff --git a/src/backend/utils/sort/tuplesort.c b/src/backend/utils/sort/tuplesort.cindex c2bd38f39f..91c0189577 100644--- a/src/backend/utils/sort/tuplesort.c+++ b/src/backend/utils/sort/tuplesort.c@@ -1419,6 +1419,10 @@ tuplesort_reset(Tuplesortstate *state)
state->slabFreeHead = NULL;
state->growmemtuples = true;
+ state->tuplecontext = AllocSetContextCreate(state->sortcontext,+ "Caller tuples",+ ALLOCSET_DEFAULT_SIZES);+
if (state->memtupsize < INITIAL_MEMTUPSIZE)
{
if (state->memtuples)
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: tomas.vondra@2ndquadrant.com, jtc331@gmail.com, michael@paquier.xyz, rafia.pghackers@gmail.com, pg@bowt.ie, simon@2ndquadrant.com, shaun.thomas@2ndquadrant.com, 9erthalion6@gmail.com, a.korotkov@postgrespro.ru, tgl@sss.pgh.pa.us, pgsql-hackers@lists.postgresql.org, alvherre@2ndquadrant.com
Subject: Re: [PATCH] Incremental sort (was: PoC: Partial sort)
In-Reply-To: <20200314195822.bc2mqyvolo2w5l2w@development>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox