Hi Chao-san,
Thank you very much for your testing and review!
> 1. Major concern: In this patch, you only increase vacrel->total_dead_items_bytes in dead_items_reset(), however, dead_items_reset() is not always called, that way I often see usage is 0. We should also increate the counter in heap_vacuum_rel() where you now get dead_items_max_bytes. My dirty fix is like below:
I believe the key point is how to display memory usage when index vacuum is skipped.
In the v6 implementation, memory usage was always shown as 0 when there were no indexes.
However, more accurately, memory is allocated during initialization even when index vacuum is not executed, so it seems appropriate to display memory consumption as you suggested.
To avoid double-counting memory usage, I've modified the location and conditions where this processing is added.
> 2
> + * Save dead items max_bytes before cleanup for reporting the memory usage
> + * as the dead_items_info is freed in parallel vacuum cases during
> + * cleanup.
> + */
> + dead_items_max_bytes = vacrel->dead_items_info->max_bytes;
> The comment says "the dead_items_info is freed in parallel vacuum", should we check if vacrel->dead_items_info != NULL before deferencing max_bytes?
In the case of parallel vacuum, the reference to dead_items_info is released in the subsequent dead_items_cleanup() call, so it cannot be NULL at this point.
I've updated the comment to make it clearer which function call causes the memory to become inaccessible.
> 3
> + appendStringInfo(&buf,
> + ngettext("memory usage: %.2f MB in total, with dead-item storage reset %d time (limit was %.2f MB)\n",
> + "memory usage: %.2f MB in total, with dead-item storage reset %d times (limit was %.2f MB)\n",
>
> I just feel "limit was xxx MB" is still not clear enough. How about be explicit, like: Memory usage: 0.2 MB in total, memory allocated across 0 dead-item storage resets in total: 64.00 MB Or Memory usage: 0.2 MB in total, with dead-item storage reset %d time, total allocated: 64.00 MB
I've revised it as follows. What do you think?
memory usage: 0.02 MB in total, with dead-item storage reset 0 times (memory allocated: 64.00 MB)
I applied the changes above.
The updated v7 is attached.
I look forward to your feedback.
Best regards,
Tatsuya Kawata
