Re: checkpointer continuous flushing

On 2015-09-09 21:29:12 +0200, Fabien COELHO wrote:
> >Imagine a binaryheap.h style heap over a structure like (tablespaceid,
> >progress, progress_inc, nextbuf) where the comparator compares the progress.
>
> It would replace what is currently an array.

It'd still be one afterwards.

> The balancing code needs to enumerate all tablespaces in a round-robin
> way so as to ensure that all tablespaces are given some attention,
> otherwise you could have a balance on two tablespaces but others could
> be left out. The array makes this property straightforward.

Why would a heap as I've described it require that?

> >>Anyway, I think it would complicate the code significantly (compared to the
> >>straightforward array)
> >
> >I doubt it. I mean instead of your GetNext you'd just do:
> > next_tblspc = DatumGetPointer(binaryheap_first(heap));
> > if (next_tblspc == 0)
> > return 0;
> > next_tblspc.progress += next_tblspc.progress_slice;
> > binaryheap_replace_first(PointerGetDatum(next_tblspc));
> >
> > return next_tblspc.nextbuf++;
>
> Compare to the array, this tree approach would required ln(Ntablespace) work
> to extract and reinsert the tablespace under progress, so there is no
> complexity advantage.

extract/reinsert is actually O(1).

> >progress_slice is the number of buffers in the tablespace divided by the
> >number of total buffers, to avoid doing any sort of expensive math in
> >the more frequently executed path.
>
> If there are many buffers, I'm not too sure about rounding issues and the
> like, so the current approach with a rational seems more secure.

Meh. The amount of imbalance introduced by rounding won't matter.

> >>ISTM that using a tablespace in the sorting would reduce the complexity to
> >>ln(NBuffers) * Ntablespace for finding the boundaries, and then Nbuffers *
> >>(Ntablespace/Ntablespace) = NBuffers for scanning, at the expense of more
> >>memory and code complexity.
> >
> >Afaics finding the boundaries can be done as part of the enumeration of
> >tablespaces in BufferSync(). That code needs to be moved, but that's not
> >too bad. I don't see the code be that much more complicated?
>
> Hmmm. you are proposing to replace prooved and heavilly tested code by a
> more complex tree data structures distributed quite differently around the
> source, and no very clear benefit.

There's no "proved and heavily tested code" touched here.

> So I would prefer to keep the code as is, that is pretty straightforward,
> and wait for a strong incentive before doing anything fancier.

I find the proposed code not particularly pretty, so I don't really buy
the straightforwardness argument.

Greetings,

Andres Freund