RE: [Patch] Optimize dropping of relation buffers using dlist

From: Jamison, Kirk/ジャミソン カーク <k(dot)jamison(at)fujitsu(dot)com>

(1)
> Alright. I also removed nTotalBlocks in v24-0003 patch.
>
> for (i = 0; i < nforks; i++)
> {
> if (nForkBlocks[i] != InvalidBlockNumber &&
> nBlocksToInvalidate < BUF_DROP_FULL_SCAN_THRESHOLD)
> {
> Optimization loop
> }
> else
> break;
> }
> if (i >= nforks)
> return;
> { usual buffer invalidation process }

Why do you do this way? I think the previous patch was more correct (while agreeing with Horiguchi-san in that nTotalBlocks may be unnecessary. What you want to do is "if the size of any fork could be inaccurate, do the traditional full buffer scan without performing any optimization for any fork," right? But the above code performs optimization for forks until it finds a fork with inaccurate size.

(2)
+ * Get the total number of cached blocks and to-be-invalidated blocks
+ * of the relation. The cached value returned by smgrnblocks could be
+ * smaller than the actual number of existing buffers of the file.

As you changed the meaning of the smgrnblocks() argument from cached to accurate, and you nolonger calculate the total blocks, the comment should reflect them.

(3)
In smgrnblocks(), accurate is not set to false when mdnblocks() is called. The caller doesn't initialize the value either, so it can see garbage value.

(4)
+ if (nForkBlocks[i] != InvalidBlockNumber &&
+ nBlocksToInvalidate < BUF_DROP_FULL_SCAN_THRESHOLD)
+ {
...
+ }
+ else
+ break;
+ }

In cases like this, it's better to reverse the if and else. Thus, you can reduce the nest depth.

Regards
Takayuki Tsunakawa