Re: Polyphase merge is obsolete
Zhihong Yu <zyu@yugabyte.com>
From: Zhihong Yu <zyu@yugabyte.com>
To: Jaime Casanova <jcasanov@systemguards.com.ec>
Cc: Heikki Linnakangas <hlinnaka@iki.fi>, vignesh C <vignesh21@gmail.com>, pgsql-hackers <pgsql-hackers@postgresql.org>, Peter Geoghegan <pg@bowt.ie>, Tomas Vondra <tomas.vondra@2ndquadrant.com>
Date: 2021-09-11T08:28:42Z
Lists: pgsql-hackers
On Fri, Sep 10, 2021 at 11:35 PM Jaime Casanova < jcasanov@systemguards.com.ec> wrote: > On Wed, Jul 14, 2021 at 06:04:14PM +0300, Heikki Linnakangas wrote: > > On 14/07/2021 15:12, vignesh C wrote: > > > On Sat, Jan 23, 2021 at 3:49 AM Heikki Linnakangas <hlinnaka@iki.fi> > wrote: > > > > Here's an updated version that fixes one bug: > > > > > > > > The CFBot was reporting a failure on the FreeBSD system [1]. It > turned > > > > out to be an out-of-memory issue caused by an underflow bug in the > > > > calculation of the size of the tape read buffer size. With a small > > > > work_mem size, the memory left for tape buffers was negative, and > that > > > > wrapped around to a very large number. I believe that was not caught > by > > > > the other systems, because the other ones had enough memory for the > > > > incorrectly-sized buffers anyway. That was the case on my laptop at > > > > least. It did cause a big slowdown in the 'tuplesort' regression test > > > > though, which I hadn't noticed. > > > > > > > > The fix for that bug is here as a separate patch for easier review, > but > > > > I'll squash it before committing. > > > > > > The patch does not apply on Head anymore, could you rebase and post a > > > patch. I'm changing the status to "Waiting for Author". > > > > Here's a rebased version. I also squashed that little bug fix from > previous > > patch set. > > > > Hi, > > This patch does not apply, can you submit a rebased version? > > -- > Jaime Casanova > Director de Servicios Profesionales > SystemGuards - Consultores de PostgreSQL > > > Hi, + * Before PostgreSQL 14, we used the polyphase merge algorithm (Knuth's + * Algorithm 5.4.2D), I think the above 'Before PostgreSQL 14' should be 'Before PostgreSQL 15' now that PostgreSQL 14 has been released. +static int64 +merge_read_buffer_size(int64 avail_mem, int nInputTapes, int nInputRuns, + int maxOutputTapes) For memory to allocate, I think uint64 can be used (instead of int64). Cheers
Commits
-
Fix and clarify function comment on LogicalTapeSetCreate.
- 95f62b16a334 15.2 landed
- 236f1ea84c4c 16.0 landed
-
Refactor LogicalTapeSet/LogicalTape interface.
- c4649cce39a4 15.0 landed
-
Replace polyphase merge algorithm with a simple balanced k-way merge.
- 65014000b351 15.0 landed
-
logtape.c: do not preallocate for tapes when sorting
- 075896496394 14.0 cited