Re: checkpointer continuous flushing

On 2015-08-10 19:07:12 +0200, Fabien COELHO wrote:
> I think that there is no issue with the current shared_buffers limit. I
> could allocate and use 4 GB on my laptop without problem. I added a cast to
> ensure that unsigned int are used for the size computation.

You can't allocate 4GB with palloc(), it has a builtin limit against
allocating more than 1GB.

> >>+ /* + * Lazy allocation: this function is called through the
> >>checkpointer, + * but also by initdb. Maybe the allocation could be
> >>moved to the callers. + */ + if (CheckpointBufferIds == NULL) +
> >>AllocateCheckpointBufferIds(); +
> >>
> >
> >I don't think it's a good idea to allocate this on every round.
> >That just means a lot of page table entries have to be built and torn down
> >regularly. It's not like checkpoints only run for 1% of the time or such.
>
> Sure. It is not allocated on every round, it is allocated once on the first
> checkpoint, the variable tested is static. There is no free. Maybe
> the allocation could be moved to the callers, though.

Well, then everytime the checkpointer is restarted.

> >FWIW, I still think it's a much better idea to allocate the memory once
> >in shared buffers.
>
> Hmmm. The memory does not need to be shared with other processes?

The point is that it's done at postmaster startup, and we're pretty much
guaranteed that the memory will availabl.e.

> >It's not like that makes us need more memory overall, and it'll be huge
> >page allocations if configured. I also think that sooner rather than later
> >we're going to need more than one process flushing buffers, and then it'll
> >need to be moved there.
>
> That is an argument. I think that it could wait for the need to actually
> arise.

Huge pages are used today.

> >>+ /*
> >>+ * Sort buffer ids to help find sequential writes.
> >>+ *
> >>+ * Note: buffers are not locked in anyway, but that does not matter,
> >>+ * this sorting is really advisory, if some buffer changes status during
> >>+ * this pass it will be filtered out later. The only necessary property
> >>+ * is that marked buffers do not move elsewhere.
> >>+ */
> >
> >That reasoning makes it impossible to move the fsyncing of files into the
> >loop (whenever we move to a new file). That's not nice.
>
> I do not see why.

Because it means that the sorting isn't necessarily correct. I.e. we
can't rely on it to determine whether a file has already been fsynced.

> >> Also, qsort implementation
> >>+ * should be resilient to occasional contradictions (cmp(a,b) != -cmp(b,a))
> >>+ * because of these possible concurrent changes.
> >
> >Hm. Is that actually the case for our qsort implementation?
>
> I think that it is hard to write a qsort which would fail that. That would
> mean that it would compare the same items twice, which would be inefficient.

What? The same two elements aren't frequently compared pairwise with
each other, but of course an individual element is frequently compared
with other elements. Consider what happens when the chosen pivot element
changes its identity after already dividing half. The two partitions
will not be divided in any meaning full way anymore. I don't see how
this will results in a meaningful sort.

> >If the pivot element changes its identity won't the result be pretty much
> >random?
>
> That would be a very unlikely event, given the short time spent in
> qsort.

Meh, we don't want to rely on "likeliness" on such things.

Greetings,

Andres Freund