Some minor review comments for patch v10-0001.
======
src/include/access/tableam.h
1.
struct IndexInfo;
+struct ParallelVacuumState;
+struct ParallelContext;
+struct ParallelWorkerContext;
struct SampleScanState;
Use alphabetical order for consistency with existing code.
~~~
2.
+ /*
+ * Estimate the size of shared memory that the parallel table vacuum needs
+ * for AM
+ *
2a.
Missing period (.)
2b.
Change the wording to be like below, for consistency with the other
'estimate' function comments, and for consistency with the comment
where this function is implemented.
Estimate the size of shared memory needed for a parallel table vacuum
of this relation.
~~~
3.
+ * The state_out is the output parameter so that an arbitrary data can be
+ * passed to the subsequent callback, parallel_vacuum_remove_dead_items.
Typo? "an arbitrary data"
~~~
4. General/Asserts
All the below functions have a comment saying "Not called if parallel
table vacuum is disabled."
- parallel_vacuum_estimate
- parallel_vacuum_initialize
- parallel_vacuum_initialize_worker
- parallel_vacuum_collect_dead_items
But, it's only a comment. I wondered if they should all have some
Assert as an integrity check on that.
~~~
5.
+/*
+ * Return the number of parallel workers for a parallel vacuum scan of this
+ * relation.
+ */
"Return the number" or "Compute the number"?
The comment should match the comment in the fwd declaration of this function.
~~~
6.
+/*
+ * Perform a parallel vacuums scan to collect dead items.
+ */
6a.
"Perform" or "Execute"?
The comment should match the one in the fwd declaration of this function.
6b.
Typo "vacuums"
======
Kind Regards,
Peter Smith.
Fujitsu Australia
