On Mon, Jul 17, 2017 at 6:02 PM, Robert Haas <robertmhaas@gmail.com> wrote:
> It seems to me that this area might benefit from a broader rethink.
> It's not very nice to impose a restriction that init forks can only be
> constructed using log_newpage(); on the other hand, it is also not
> very nice that Amit's patch needs to recapitulate the mysterious
> incantation used by XLogReadBufferForRedoExtended in several more
> places. The basic problem here is that it would be really easy for
> the next person who adds an index AM to make the exact same mistake
> that Amit made here and that I failed to spot during code review. It
> would be nice to come up with some more generic solution to the
> problem rather than relying on each AM to do insert this
> FlushOneBuffer() incantation in each place where it's needed. I think
> there are probably several ways to do that; one idea might be to flush
> all init-fork buffers in bulk at the end of recovery.
Things are centralized in XLogReadBufferForRedoExtended() for init
fork flushes, which is not something bad in itself as it is the unique
place doing this work normally for other AMs. And I have to admit that
the current coding for hash index WAL has the advantage to create less
WAL quantity as the bitmap and metadata pages get initialized using
the data of the records themselves. One idea I can think of would be
to create a README in src/backend/access for index AMs that summarizes
a basic set of recommendations for each routine used.
> However, it doesn't really seem urgent to tinker with that; while I
> think the fact that existing AMs use log_newpage() to populate the
> init fork is mostly just luck, it might well be 10 or 20 years before
> somebody adds another AM that happens to use any other technique.
> Moreover, at the moment, we're trying to get a release out the door,
> and to me that argues for keeping structural changes to a minimum.
> Amit's patch seems like a pretty surgical fix to this problem with
> minimal chance of breaking anything new, and that seems like the right
> kind of fix for July. So I plan to commit what Amit proposed (with a
> few grammar corrections).
No objections to that.
--
Michael