Hi,
On 2021-05-18 20:43:41 +0200, Tomas Vondra wrote:
> Yeah, emitting WAL is not exactly cheap, although it's just a little bit
> more (0.44%). I haven't looked into the details, but I wonder why it has
> such disproportionate impact (although, the 32 vs. 40 sec may be off).
I couldn't reproduce this large a performance difference - I saw more
like 10% instead of 25%.
> > I dimly remember that we explicitly discussed that we do *not* want to
> > emit WAL records here?
> Ummm, in which thread?
https://postgr.es/m/20190408010427.4l63qr7h2fjcyp77%40alap3.anarazel.de
On 2019-04-07 18:04:27 -0700, Andres Freund wrote:
> This avoids an extra WAL record for setting empty pages to all visible,
> by adding XLH_INSERT_ALL_VISIBLE_SET & XLH_INSERT_ALL_FROZEN_SET, and
> setting those when appropriate in heap_multi_insert. Unfortunately
> currently visibilitymap_set() doesn't really properly allow to do this,
> as it has embedded WAL logging for heap.
>
> I think we should remove the WAL logging from visibilitymap_set(), and
> move it to a separate, heap specific, function.
It'd probably be sufficient for the current purpose to change
visibilitymap_set()'s documentation to say that recptr can also be
passed in if the action is already covered by a WAL record, and that
it's the callers responsibility to think through the correctness
issues. Here it's easy, because any error will just throw the relation
away.
We do need to to include all-visible / FSM change in the WAL, so
crash-recovery / standbys end up with the same result as a primary
running normally. We already have the information, via
XLH_INSERT_ALL_FROZEN_SET. I think all we need to do is to add a
visibilitymap_set() in the redo routines if XLH_INSERT_ALL_FROZEN_SET.
Greetings,
Andres Freund