On Sun, Mar 4, 2018 at 10:25 PM, Masahiko Sawada <sawada.mshk@gmail.com> wrote:
> On Mon, Mar 5, 2018 at 10:21 AM, Masahiko Sawada <sawada.mshk@gmail.com> wrote:
>> On Fri, Mar 2, 2018 at 10:50 PM, Claudio Freire <klaussfreire@gmail.com> wrote:
>>> On Fri, Mar 2, 2018 at 10:47 AM, Claudio Freire <klaussfreire@gmail.com> wrote:
>>>> On Fri, Mar 2, 2018 at 7:38 AM, Masahiko Sawada <sawada.mshk@gmail.com> wrote:
>>>>> Thank you for updating the patches!
>>>>>
>>>>> +/*
>>>>> + * When a table has no indexes, vacuum the FSM at most every this
>>>>> + * many dirty pages. With a default page size of 8kb, this value
>>>>> + * basically means 8GB of dirtied pages.
>>>>> + */
>>>>> +#define VACUUM_FSM_EVERY_PAGES 1048576
>>>>>
>>>>> Is it better if we vacuum fsm every 8GB regardless of the block size?
>>>>> Because an installation that uses >8GB block size is likely to have
>>>>> the pages less than what an 8GB block size installation has, the
>>>>> current patch might lead to delay fsm vacuum. What do you think? If
>>>>> it's better, we can define it as follows.
>>>>>
>>>>> #define VACUUM_FSM_EVERY_PAGES ((8 * 1024 * 1024) / BLCKSZ)
>>>>
>>>> I honestly don't know. I've never tweaked page size, and know nobody
>>>> that did, so I have no experience with those installations.
>>>>
>>>> Lets go with your proposal.
>>>>
>>>>>
>>>>> --
>>>>> @@ -470,7 +484,9 @@ lazy_scan_heap(Relation onerel, int options,
>>>>> LVRelStats *vacrelstats,
>>>>> TransactionId relfrozenxid = onerel->rd_rel->relfrozenxid;
>>>>> TransactionId relminmxid = onerel->rd_rel->relminmxid;
>>>>> BlockNumber empty_pages,
>>>>> - vacuumed_pages;
>>>>> + vacuumed_pages,
>>>>> + fsm_updated_pages,
>>>>> + vacuum_fsm_every_pages;
>>>>> double num_tuples,
>>>>> tups_vacuumed,
>>>>> nkeep,
>>>>>
>>>>> Regarding fsm_updated_pages variable name, I think it's better to be
>>>>> freespace_updated_pages because we actually counts the page updated
>>>>> its freespace, not fsm.
>>>>
>>>> Good point.
>>>>
>>>> New version attached.
>>>
>>> Sorry, forgot to actually squash. Now the real new version is attached.
>>
>> Thank you for updating. I think the patches has enough discussion and
>> quality, so can go to the committer's review. I've marked them as
>> Ready for Committer.
>>
>
> Oops, the following comment needs to be updated. Since it would be
> better to defer decision of this behavior to committers I'll keep the
> status of this patch as Ready for Committer behavior.
>
> +/*
> + * When a table has no indexes, vacuum the FSM at most every this
> + * many dirty pages. With a default page size of 8kb, this value
> + * basically means 8GB of dirtied pages.
> + */
> +#define VACUUM_FSM_EVERY_PAGES ((8 * 1024 * 1024) / BLCKSZ)
> +
I just noticed that it actually computes 8MB (not GB) of pages.
Attached a fix for that and the comment update.