Re: Polyphase merge is obsolete

On Fri, Sep 10, 2021 at 11:35 PM Jaime Casanova <
jcasanov(at)systemguards(dot)com(dot)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(at)iki(dot)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