Re: [HACKERS] WAL logging problem in 9.4.3?

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

At Thu, 21 Nov 2019 16:01:07 +0900 (JST), Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com> wrote in
> I should have replied this first.
>
> At Sun, 17 Nov 2019 20:54:34 -0800, Noah Misch <noah(at)leadboat(dot)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