Re: Parallel heap vacuum

From: Peter Smith <smithpb2250(at)gmail(dot)com>
To: Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>
Cc: Melanie Plageman <melanieplageman(at)gmail(dot)com>, John Naylor <johncnaylorls(at)gmail(dot)com>, Tomas Vondra <tomas(at)vondra(dot)me>, "Hayato Kuroda (Fujitsu)" <kuroda(dot)hayato(at)fujitsu(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Parallel heap vacuum
Date: 2025-03-07 01:33:21
Message-ID: CAHut+PuaBtXpJOtY7yi=nV6s2ed3+Q9J5cg6h=b5HqspPtKRyg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Erik Wienhold 2025-03-07 01:42:00 Re: Add column name to error description
Previous Message Jeff Davis 2025-03-07 00:58:37 Re: Statistics Import and Export