On Monday, August 19, 2019 10:39 AM (GMT+9), Masahiko Sawada wrote:
> Fixed.
>
> Attached the updated version patch.
Hi Sawada-san,
I haven't tested it with heavily updated large tables, but I think the patch
is reasonable as it helps to shorten the execution time of vacuum by removing
the redundant vacuuming and prioritizing reclaiming the garbage instead.
I'm not sure if it's commonly reported to have problems even when we repeat
vacuuming the already-vacuumed blocks, but I think it's a reasonable improvement.
I skimmed the patch and have few comments. If they deem fit, feel free to
follow, but it's also ok if you don't.
1.
>+ <entry>Block number to resume vacuuming from</entry>
Perhaps you could drop "from".
2.
>+ <xref linkend="pg-stat-all-tables-view"/>. This behavior is helpful
>+ when to resume vacuuming from interruption and cancellation.The default
when resuming vacuum run from interruption and cancellation.
There should also be space between period and "The".
3.
>+ set to true. This option is ignored if either the <literal>FULL</literal>,
>+ the <literal>FREEZE</literal> or <literal>DISABLE_PAGE_SKIPPING</literal>
>+ option is used.
..if either of the <literal>FULL</literal>, <literal>FREEZE</literal>, or <literal>DISABLE_PAGE_SKIPPING</literal>
optionsis used.
4.
>+ next_fsm_block_to_vacuum,
>+ next_block_to_resume;
Clearer one would be "next_block_to_resume_vacuum"?
You may disregard if that's too long.
5.
>+ Assert(start_blkno <= nblocks); /* both are the same iif it's empty */
iif -> if there are no blocks / if nblocks is 0
6.
>+ * If not found a valid saved block number, resume from the
>+ * first block.
>+ */
>+ if (!found ||
>+ tabentry->vacuum_resume_block >= RelationGetNumberOfBlocks(onerel))
This describes when vacuum_resume_block > RelationGetNumberOfBlocks.., isn't it?
Perhaps a better framing would be
"If the saved block number is found invalid,...",
7.
>+ bool vacuum_resume; /* enables vacuum to resuming from last
>+ * vacuumed block. */
resuming --> resume
Regards,
Kirk Jamison
