Horton

Re: [PATCH] Incremental sort (was: PoC: Partial sort)

Tomas Vondra <tomas.vondra@2ndquadrant.com>

View thread View on postgr.es
From: Tomas Vondra <tomas.vondra@2ndquadrant.com>
To: James Coleman <jtc331@gmail.com>
Cc: Alvaro Herrera <alvherre@2ndquadrant.com>, Michael Paquier <michael@paquier.xyz>, Rafia Sabih <rafia.pghackers@gmail.com>, Peter Geoghegan <pg@bowt.ie>, Simon Riggs <simon@2ndquadrant.com>, Shaun Thomas <shaun.thomas@2ndquadrant.com>, Dmitry Dolgov <9erthalion6@gmail.com>, Alexander Korotkov <a.korotkov@postgrespro.ru>, Tom Lane <tgl@sss.pgh.pa.us>, PostgreSQL Developers <pgsql-hackers@lists.postgresql.org>, Andreas Karlsson <andreas@proxel.se>
Date: 2020-03-30T01:44:42Z
Lists: pgsql-hackers

Attachments

Hi,

Attached is a slightly reorganized patch series. I've merged the fixes
into the appropriate matches, and I've also combined the two patches
adding incremental sort paths to additional places in planner.

A couple more comments:


1) I think the GUC documentation in src/sgml/config.sgml is a bit too
detailed, compared to the other enable_* GUCs. I wonder if there's a
better place where to move the details. What about adding some examples
and explanation to perform.sgml?


2) Looking at the explain output, the verbose mode looks like this:

test=# explain (verbose, analyze) select a from t order by a, b, c;
                                                                           QUERY PLAN
--------------------------------------------------------------------------------------------------------------------------------------------------------------
  Gather Merge  (cost=66.31..816072.71 rows=8333226 width=24) (actual time=4.787..20092.555 rows=10000000 loops=1)
    Output: a, b, c
    Workers Planned: 2
    Workers Launched: 2
    ->  Incremental Sort  (cost=66.28..729200.36 rows=4166613 width=24) (actual time=1.308..14021.575 rows=3333333 loops=3)
          Output: a, b, c
          Sort Key: t.a, t.b, t.c
          Presorted Key: t.a, t.b
          Full-sort Groups: 4169 Sort Method: quicksort Memory: avg=30kB peak=30kB
          Presorted Groups: 4144 Sort Method: quicksort Memory: avg=128kB peak=138kB
          Worker 0:  actual time=0.766..16122.368 rows=3841573 loops=1
  Full-sort Groups: 6871 Sort Method: quicksort Memory: avg=30kB peak=30kB
            Presorted Groups: 6823 Sort Method: quicksort Memory: avg=132kB peak=141kB
          Worker 1:  actual time=1.986..16189.831 rows=3845490 loops=1
  Full-sort Groups: 6874 Sort Method: quicksort Memory: avg=30kB peak=30kB
            Presorted Groups: 6847 Sort Method: quicksort Memory: avg=130kB peak=139kB
          ->  Parallel Index Scan using t_a_b_idx on public.t  (cost=0.43..382365.92 rows=4166613 width=24) (actual time=0.040..9808.449 rows=3333333 loops=3)
                Output: a, b, c
                Worker 0:  actual time=0.048..11275.178 rows=3841573 loops=1
                Worker 1:  actual time=0.041..11314.133 rows=3845490 loops=1
  Planning Time: 0.166 ms
  Execution Time: 25135.029 ms
(22 rows)

There seems to be missing indentation for the first line of worker info.

I'm still not quite convinced we should be printing two lines - I know
you mentioned the lines might be too long, but see how long the other
lines may get ...


3) I see the new nodes (plan state, ...) have "presortedCols" which does
not indicate it's a "number of". I think we usually prefix names of such
fields "n" or "num". What about "nPresortedCols"? (Nitpicking, I know.)


My TODO for this patch is this:

- review the costing (I think the estimates are OK, but I recall I
   haven't been entirely happy with how it's broken into functions.)

- review the tuplesort changes (the memory contexts etc.)

- do more testing of performance impact on planning


regards

-- 
Tomas Vondra                  http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Commits

  1. Further adjustments to Hashagg EXPLAIN ANALYZE output

  2. Rework EXPLAIN format for incremental sort

  3. Fix typos and improve incremental sort comments

  4. Stabilize incremental_sort tests

  5. Minor improvements in Incremental Sort explain

  6. Consider Incremental Sort paths at additional places

  7. Fix representation of SORT_TYPE_STILL_IN_PROGRESS.

  8. Fix failures in incremental_sort due to number of workers

  9. Fix show_incremental_sort_info with force_parallel_mode

  10. Implement Incremental Sort

  11. Fix handling of "Subplans Removed" field in EXPLAIN output.

  12. Fix EXPLAIN (SETTINGS) to follow policy about when to print empty fields.

  13. Ensure plpgsql result tuples have the right composite type marking.

  14. Propagate sort instrumentation from workers back to leader.

  15. Make new regression test case parallel-safe, and improve its output.

  16. Push limit through subqueries to underlying sort, where possible.

  17. Fix inappropriate printing of never-measured times in EXPLAIN.

  18. Fix some infelicities in EXPLAIN output for parallel query plans.