Re: [HACKERS] WAL logging problem in 9.4.3? - Mailing list pgsql-hackers

Wow.. This is embarrassing.. *^^*.

At Thu, 21 Nov 2019 16:01:07 +0900 (JST), Kyotaro Horiguchi <horikyota.ntt@gmail.com> wrote in 
> I should have replied this first.
> 
> At Sun, 17 Nov 2019 20:54:34 -0800, Noah Misch <noah@leadboat.com> wrote in 
> > On Tue, Nov 05, 2019 at 02:53:35PM -0800, Noah Misch wrote:
> > > I started pre-commit editing on 2019-10-28, and comment+README updates have
> > > been the largest part of that.  I'll check my edits against the things you
> > > list here, and I'll share on-list before committing.  I've now marked the CF
> > > entry Ready for Committer.
> > 
> > Having dedicated many days to that, I am attaching v24nm.  I know of two
> > remaining defects:
> > 
> > === Defect 1: gistGetFakeLSN()
> > 
> > When I modified pg_regress.c to use wal_level=minimal for all suites,
> > src/test/isolation/specs/predicate-gist.spec failed the assertion in
> > gistGetFakeLSN().  One could reproduce the problem just by running this
> > sequence in psql:
> > 
> >           begin;
> >           create table gist_point_tbl(id int4, p point);
> >           create index gist_pointidx on gist_point_tbl using gist(p);
> >           insert into gist_point_tbl (id, p)
> >           select g, point(g*10, g*10) from generate_series(1, 1000) g;
> > 
> > I've included a wrong-in-general hack to make the test pass.  I see two main
> > options for fixing this:
> > 
> > (a) Introduce an empty WAL record that reserves an LSN and has no other
> > effect.  Make GiST use that for permanent relations that are skipping WAL.
> > Further optimizations are possible.  For example, we could use a backend-local
> > counter (like the one gistGetFakeLSN() uses for temp relations) until the
> > counter is greater a recent real LSN.  That optimization is probably too
> > clever, though it would make the new WAL record almost never appear.
> > 
> > (b) Exempt GiST from most WAL skipping.  GiST index build could still skip
> > WAL, but it would do its own smgrimmedsync() in addition to the one done at
> > commit.  Regular GiST mutations would test RELPERSISTENCE_PERMANENT instead of
> > RelationNeedsWal(), and we'd need some hack for index_copy_data() and possibly
> > other AM-independent code that skips WAL.
> > 
> > Overall, I like the cleanliness of (a).  The main argument for (b) is that it
> > ensures we have all the features to opt-out of WAL skipping, which could be
> > useful for out-of-tree index access methods.  (I think we currently have the
> > features for a tableam to do so, but not for an indexam to do so.)  Overall, I
> > lean toward (a).  Any other ideas or preferences?
> 
> I don't like (b), too.
> 
> What we need there is any sequential numbers for page LSN but really
> compatible with real LSN. Couldn't we use GetXLogInsertRecPtr() in the

> case?  Or, I'm not sure but I suppose that nothing happenes when
> UNLOGGED GiST index gets turned into LOGGED one.

Yes, I just forgot to remove these lines when writing the following.

> Rewriting table like SET LOGGED will work but not realistic.
> 
> > === Defect 2: repetitive work when syncing many relations
> > 
> > For deleting relfilenodes, smgrDoPendingDeletes() collects a list for
> > smgrdounlinkall() to pass to DropRelFileNodesAllBuffers(), which is
> > sophisticated about optimizing the shared buffers scan.  Commit 279628a
> > introduced that, in 2013.  I think smgrDoPendingSyncs() should do likewise, to
> > further reduce the chance of causing performance regressions.  (One could,
> > however, work around the problem by raising wal_skip_threshold.)  Kyotaro, if
> > you agree, could you modify v24nm to implement that?
> 
> Seems reasonable. Please wait a minite.

regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center