On Tue, May 28, 2019 at 4:33 AM Noah Misch <n...@leadboat.com> wrote: > > On Mon, May 27, 2019 at 02:08:26PM +0900, Kyotaro HORIGUCHI wrote: > > At Fri, 24 May 2019 19:33:32 -0700, Noah Misch <n...@leadboat.com> wrote in > > <20190525023332.ge1624...@rfd.leadboat.com> > > > On Mon, May 20, 2019 at 03:54:30PM +0900, Kyotaro HORIGUCHI wrote: > > > > Following this direction, the attached PoC works *at least for* > > > > the wal_optimization TAP tests, but doing pending flush not in > > > > smgr but in relcache. > > > > > > This task, syncing files created in the current transaction, is not the > > > kind > > > of task normally assigned to a cache. We already have a module, > > > storage.c, > > > that maintains state about files created in the current transaction. Why > > > did > > > you use relcache instead of storage.c? > > > > The reason was at-commit sync needs buffer flush beforehand. But > > FlushRelationBufferWithoutRelCache() in v11 can do > > that. storage.c is reasonable as the place. > > Okay. I do want this to work in 9.5 and later, but I'm not aware of a reason > relcache.c would be a better code location in older branches. Unless you > think of a reason to prefer relcache.c, please use storage.c. > > > > On Tue, May 21, 2019 at 09:29:48PM +0900, Kyotaro HORIGUCHI wrote: > > > > This is a tidier version of the patch. > > > > > > > - Move the substantial work to table/index AMs. > > > > > > > > Each AM can decide whether to support WAL skip or not. > > > > Currently heap and nbtree support it. > > > > > > Why would an AM find it important to disable WAL skip? > > > > The reason is currently it's AM's responsibility to decide > > whether to skip WAL or not. > > I see. Skipping the sync would be a mere optimization; no AM would require it > for correctness. An AM might want RelationNeedsWAL() to keep returning true > despite the sync happening, perhaps because it persists data somewhere other > than the forks of pg_class.relfilenode. Since the index and table APIs > already assume one relfilenode captures all persistent data, I'm not seeing a > use case for an AM overriding this behavior. Let's take away the AM's > responsibility for this decision, making the system simpler. A future patch > could let AM code decide, if someone find a real-world use case for > AM-specific logic around when to skip WAL. >
It seems there is some feedback for this patch and the CF is going to start in 2 days. Are you planning to work on this patch for next CF, if not then it is better to bump this? It is not a good idea to see the patch in "waiting on author" in the beginning of CF unless the author is actively working on the patch and is going to produce a version in next few days. -- With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com
