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> options is 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
