Re: [PATCH] Incremental sort (was: PoC: Partial sort)
James Coleman <jtc331@gmail.com>
On Fri, Mar 13, 2020 at 8:23 PM James Coleman <jtc331@gmail.com> wrote: > > On Friday, March 13, 2020, Tomas Vondra <tomas.vondra@2ndquadrant.com> wrote: >> >> On Fri, Mar 13, 2020 at 04:31:16PM -0400, James Coleman wrote: >>> >>> On Fri, Mar 13, 2020 at 2:23 PM James Coleman <jtc331@gmail.com> wrote: >>>> >>>> >>>> On Tue, Mar 10, 2020 at 10:44 PM Tomas Vondra >>>> <tomas.vondra@2ndquadrant.com> wrote: >>>> > 3) Most of the execution plans look reasonable, except that some of the >>>> > plans look like this: >>>> > >>>> > >>>> > QUERY PLAN >>>> > --------------------------------------------------------- >>>> > Limit >>>> > -> GroupAggregate >>>> > Group Key: t.a, t.b, t.c, t.d >>>> > -> Incremental Sort >>>> > Sort Key: t.a, t.b, t.c, t.d >>>> > Presorted Key: t.a, t.b, t.c >>>> > -> Incremental Sort >>>> > Sort Key: t.a, t.b, t.c >>>> > Presorted Key: t.a, t.b >>>> > -> Index Scan using t_a_b_idx on t >>>> > (10 rows) >>>> > >>>> > i.e. there are two incremental sorts on top of each other, with >>>> > different prefixes. But this this is not a new issue - it happens with >>>> > queries like this: >>>> > >>>> > SELECT a, b, c, d, count(*) FROM ( >>>> > SELECT * FROM t ORDER BY a, b, c >>>> > ) foo GROUP BY a, b, c, d limit 1000; >>>> > >>>> > i.e. there's a subquery with a subset of pathkeys. Without incremental >>>> > sort the plan looks like this: >>>> > >>>> > QUERY PLAN >>>> > --------------------------------------------- >>>> > Limit >>>> > -> GroupAggregate >>>> > Group Key: t.a, t.b, t.c, t.d >>>> > -> Sort >>>> > Sort Key: t.a, t.b, t.c, t.d >>>> > -> Sort >>>> > Sort Key: t.a, t.b, t.c >>>> > -> Seq Scan on t >>>> > (8 rows) >>>> > >>>> > so essentially the same plan shape. What bugs me though is that there >>>> > seems to be some sort of memory leak, so that this query consumes >>>> > gigabytes os RAM before it gets killed by OOM. But the memory seems not >>>> > to be allocated in any memory context (at least MemoryContextStats don't >>>> > show anything like that), so I'm not sure what's going on. >>>> > >>>> > Reproducing it is fairly simple: >>>> > >>>> > CREATE TABLE t (a bigint, b bigint, c bigint, d bigint); >>>> > INSERT INTO t SELECT >>>> > 1000*random(), 1000*random(), 1000*random(), 1000*random() >>>> > FROM generate_series(1,10000000) s(i); >>>> > CREATE INDEX idx ON t(a,b); >>>> > ANALYZE t; >>>> > >>>> > EXPLAIN ANALYZE SELECT a, b, c, d, count(*) >>>> > FROM (SELECT * FROM t ORDER BY a, b, c) foo GROUP BY a, b, c, d >>>> > LIMIT 100; >>>> >>>> While trying to reproduce this, instead of lots of memory usage, I got >>>> the attached assertion failure instead. >>> >>> >>> And, without the EXPLAIN ANALYZE was able to get this one, which will >>> probably be a lot more helpful. >>> >> >> Hmmm, I'll try reproducing it, but can you investigate the values in the >> Assert? I mean, it fails on this: >> >> Assert(total_allocated == context->mem_allocated); >> >> so can you get a core or attach to the process using gdb, and see what's >> the expected / total value? I've reproduced this on multiple machines (though all are Ubuntu or Debian derivatives...I don't think that's likely to matter). A core dump is ~150MB, so I've uploaded to Dropbox [1]. I didn't find an obvious first-level member of Tuplesortstate that was covered by either of the two blocks in the AllocSet (both are 8KB in size). James [1]: https://www.dropbox.com/s/jwndwp4634hzywk/aset_assertion_failure.core?dl=0
Commits
-
Further adjustments to Hashagg EXPLAIN ANALYZE output
- 40efbf8706cd 14.0 cited
-
Rework EXPLAIN format for incremental sort
- 6a918c3ac8a6 13.0 landed
-
Fix typos and improve incremental sort comments
- 1a40d37a9faf 13.0 landed
-
Stabilize incremental_sort tests
- cea09246e578 13.0 landed
-
Minor improvements in Incremental Sort explain
- d22782a5392f 13.0 landed
-
Consider Incremental Sort paths at additional places
- ba3e76cc571e 13.0 landed
-
Fix representation of SORT_TYPE_STILL_IN_PROGRESS.
- c7654f6a3779 13.0 landed
-
Fix failures in incremental_sort due to number of workers
- 23ba3b5ee278 13.0 landed
-
Fix show_incremental_sort_info with force_parallel_mode
- 7d6d82a52493 13.0 landed
-
Implement Incremental Sort
- d2d8a229bc58 13.0 landed
-
Fix handling of "Subplans Removed" field in EXPLAIN output.
- 7d91b604d9b5 13.0 cited
-
Fix EXPLAIN (SETTINGS) to follow policy about when to print empty fields.
- 3ec20c7091e9 13.0 cited
-
Ensure plpgsql result tuples have the right composite type marking.
- 5683b34956b4 13.0 cited
-
Propagate sort instrumentation from workers back to leader.
- bf11e7ee2e36 11.0 cited
-
Make new regression test case parallel-safe, and improve its output.
- 1177ab1dabf7 11.0 cited
-
Push limit through subqueries to underlying sort, where possible.
- 1f6d515a67ec 11.0 cited
-
Fix inappropriate printing of never-measured times in EXPLAIN.
- 4b234fd8bf21 9.6.0 cited
-
Fix some infelicities in EXPLAIN output for parallel query plans.
- 8ebb69f85445 9.6.0 cited