From: Jamison, Kirk/ジャミソン カーク <k.jami...@fujitsu.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
