On Fri, Oct 25, 2019 at 1:13 PM Kyotaro Horiguchi <horikyota....@gmail.com> wrote: > Hello. Thanks for the comment. > > # Sorry in advance for possilbe breaking the thread. > > > MarkBufferDirtyHint() writes WAL even when rd_firstRelfilenodeSubid or > > rd_createSubid is set; see attached test case. It needs to skip WAL > > whenever > > RelationNeedsWAL() returns false. > > Thanks for pointing out that. And the test patch helped me very much. > > Most of callers can tell that to the function, but SetHintBits() > cannot easily. Rather I think we shouldn't even try to do > that. Instead, In the attached, MarkBufferDirtyHint() asks storage.c > for sync-pending state of the relfilenode for the buffer. In the > attached patch (0003) RelFileNodeSkippingWAL loops over pendingSyncs > but it is called only at the time FPW is added so I believe it doesn't > affect performance so much. However, we can use hash for pendingSyncs > instead of liked list. Anyway the change is in its own file > v21-0003-Fix-MarkBufferDirtyHint.patch, which will be merged into > 0002. > > AFAICS all XLogInsert is guarded by RelationNeedsWAL() or in the > non-wal_minimal code paths. > > > Cylinder and track sizes are obsolete as user-visible concepts. (They're > > not > > onstant for a given drive, and I think modern disks provide no way to read > > the relevant parameters.) I like the name "wal_skip_threshold", and my > > second > > I strongly agree. Thanks for the draft. I used it as-is. I don't come > up with an appropriate second description of the GUC so I just removed > it. > > # it was "For rotating magnetic disks, it is around the size of a > # track or sylinder." > > > the relevant parameters.) I like the name "wal_skip_threshold", and > > my second choice would be "wal_skip_min_size". Possibly documented > > as follows: > .. > > Any other opinions on the GUC name? > > I prefer the first candidate. I already used the terminology in > storage.c and the name fits more to the context. > > > * We emit newpage WAL records for smaller size of relations. > > * > > * Small WAL records have a chance to be emitted at once along with > > * other backends' WAL records. We emit WAL records instead of syncing > > * for files that are smaller than a certain threshold expecting faster > - * commit. The threshold is defined by the GUC effective_io_block_size. > + * commit. The threshold is defined by the GUC wal_skip_threshold.
> It's wrong that it also skips changing flags. > I"ll fix it soon This is the fixed verison v22. The attached are: - v22-0001-TAP-test-for-copy-truncation-optimization.patch Same as v20, 21 - v22-0002-Fix-WAL-skipping-feature.patch GUC name changed. Same as v21. - v22-0003-Fix-MarkBufferDirtyHint.patch PoC of fixing the function. will be merged into 0002. (New in v21, fixed in v22) - v21-0004-Documentation-for-wal_skip_threshold.patch GUC name and description changed. (Previous 0003, same as v21) - v21-0005-Additional-test-for-new-GUC-setting.patch including adjusted version of wal-optimize-noah-tests-v3.patch Maybe test names need further adjustment. (Previous 0004, same as v21) regards. -- Kyotaro Horiguchi NTT Open Source Software Center
v22-0004-Documentation-for-wal_skip_threshold.patch
Description: Binary data
v22-0001-TAP-test-for-copy-truncation-optimization.patch
Description: Binary data
v22-0002-Fix-WAL-skipping-feature.patch
Description: Binary data
v22-0005-Additional-test-for-new-GUC-setting.patch
Description: Binary data
v22-0003-Fix-MarkBufferDirtyHint.patch
Description: Binary data
