On Tue, Dec 22, 2020 at 8:30 AM Kyotaro Horiguchi <horikyota....@gmail.com> wrote: > > At Tue, 22 Dec 2020 02:48:22 +0000, "tsunakawa.ta...@fujitsu.com" > <tsunakawa.ta...@fujitsu.com> wrote in > > From: Amit Kapila <amit.kapil...@gmail.com> > > > Why would all client backends wait for AccessExclusive lock on this > > > relation? Say, a client needs a buffer for some other relation and > > > that might evict this buffer after we release the lock on the > > > partition. In StrategyGetBuffer, it is important to either have a pin > > > on the buffer or the buffer header itself must be locked to avoid > > > getting picked as victim buffer. Am I missing something? > > > > Ouch, right. (The year-end business must be making me crazy...) > > > > So, there are two choices here: > > > > 1) The current patch. > > 2) Acquire the buffer header spinlock before releasing the buffer mapping > > lwlock, and eliminate the buffer tag comparison as follows: > > > > BufTableLookup(); > > LockBufHdr(); > > LWLockRelease(); > > InvalidateBuffer(); > > > > I think both are okay. If I must choose either, I kind of prefer 1), > > because LWLockRelease() could take longer time to wake up other processes > > waiting on the lwlock, which is not very good to do while holding a > > spinlock. > > I like, as said before, the current patch. >
Attached, please find the updated patch with the following modifications, (a) updated comments at various places especially to tell why this is a safe optimization, (b) merged the patch for extending the smgrnblocks and vacuum optimization patch, (c) made minor cosmetic changes and ran pgindent, and (d) updated commit message. BTW, this optimization will help not only vacuum but also truncate when it is done in the same transaction in which the relation is created. I would like to see certain tests to ensure that the value we choose for BUF_DROP_FULL_SCAN_THRESHOLD is correct. I see that some testing has been done earlier [1] for this threshold but I am not still able to conclude. The criteria to find the right threshold should be what is the maximum size of relation to be truncated above which we don't get benefit with this optimization. One idea could be to remove "nBlocksToInvalidate < BUF_DROP_FULL_SCAN_THRESHOLD" part of check "if (cached && nBlocksToInvalidate < BUF_DROP_FULL_SCAN_THRESHOLD)" so that it always use optimized path for the tests. Then use the relation size as NBuffers/128, NBuffers/256, NBuffers/512 for different values of shared buffers as 128MB, 1GB, 20GB, 100GB. Apart from tests, do let me know if you are happy with the changes in the patch? Next, I'll look into DropRelFileNodesAllBuffers() optimization patch. [1] - https://www.postgresql.org/message-id/OSBPR01MB234176B1829AECFE9FDDFCC2EFE90%40OSBPR01MB2341.jpnprd01.prod.outlook.com -- With Regards, Amit Kapila.
v36-0001-Optimize-DropRelFileNodeBuffers-for-recovery.patch
Description: Binary data
