At Fri, 13 Nov 2020 06:43:13 +0000, "tsunakawa.ta...@fujitsu.com" <tsunakawa.ta...@fujitsu.com> wrote in > Hi Horiguchi-san, > > > Thank you for making a patch so quickly. I've started looking at it. > > What makes you think this is a PoC? Documentation and test cases? If > there's something you think that doesn't work or are concerned about, can you > share it?
The latest version is heavily revised and is given much comment so it might have exited from PoC state. The necessity of documentation is doubtful since this patch doesn't user-facing behavior other than speed. Some tests are required especialy about recovery and replication perspective but I haven't been able to make it. (One of the tests needs to cause crash while a transaction is running.) > Do you know the reason why data copy was done before? And, it may be odd for > me to ask this, but I think I saw someone referred to the past discussion > that eliminating data copy is difficult due to some processing at commit. I > can't find it. To imagine that, just because it is simpler considering rollback and code sharing, and maybe no one have been complained that SET LOGGED/UNLOGGED looks taking a long time than required/expected. The current implement is simple. It's enough to just discard old or new relfilenode according to the current transaction ends with commit or abort. Tweaking of relfilenode under use leads-in some skews in some places. I used pendingDelete mechanism a bit complexified way and a violated an abstraction (I think, calling AM-routines from storage.c is not good.) and even introduce a new fork kind only to mark a init fork as "not committed yet". There might be better way, but I haven't find it. (The patch scans all shared buffer blocks for each relation). > (1) > @@ -168,6 +168,8 @@ extern PGDLLIMPORT int32 *LocalRefCount; > */ > #define BufferGetPage(buffer) ((Page)BufferGetBlock(buffer)) > > +struct SmgrRelationData; > > This declaration is already in the file: > > /* forward declared, to avoid having to expose buf_internals.h here */ > struct WritebackContext; > > /* forward declared, to avoid including smgr.h here */ > struct SMgrRelationData; Hmmm. Nice chatch. And will fix in the next version. regards. -- Kyotaro Horiguchi NTT Open Source Software Center