Dear Sawada-san, Thanks for updating the patch. I have been reviewing and below are comments for now.
01. Sorry if I forgot something, but is there a reason that parallel_vacuum_compute_workers() is mandatory? My fresh eye felt that the function can check and regards zero if the API is NULL. 02. table_parallel_vacuum_estimate We can assert that state is not NULL. 03. table_parallel_vacuum_initialize Same as 02.. Also, I think we should ensure here that table_parallel_vacuum_estimate() has already been called before, and current assertion might not enough because others can set random value here. Other functions like table_rescan_tidrange() checks the internal flag of the data structure, which is good for me. How do you feel to check pcxt->estimator has non-zero value? 04. table_parallel_vacuum_initialize_worker Same as 02. Also, can we esure that table_parallel_vacuum_initialize() has been done? 05. table_parallel_vacuum_collect_dead_items Same as 02. Also, can we esure that table_parallel_vacuum_initialize_worker() has been done? 06. table_parallel_vacuum_initialize_worker Comments atop function needs to be updated. 07. While playing, I found that the parallel vacuum worker can be launched more than pages: ``` postgres=# CREATE TABLE var (id int); CREATE TABLE postgres=# INSERT INTO var VALUES (generate_series(1, 10000)); INSERT 0 10000 postgres=# VACUUM (parallel 80, verbose) var ; INFO: vacuuming "postgres.public.var" INFO: launched 80 parallel vacuum workers for collecting dead tuples (planned: 80) INFO: finished vacuuming "postgres.public.var": index scans: 0 pages: 0 removed, 45 remain, 45 scanned (100.00% of total), 0 eagerly scanned ... ``` I hope the minimum chunk size is a page so that this situation can reduce the performance. How do you feel to cap the value with rel::rd_rel::relpages in heap_parallel_vacuum_compute_workers()? This value is not always up-to-date but seems good candidate. Best regards, Hayato Kuroda FUJITSU LIMITED
