From: Nathan Bossart <nathandbossart@gmail.com>
To: Imseih (AWS), Sami <simseih@amazon.com>
Cc: Bossart, Nathan <bossartn@amazon.com>
Cc: Masahiko Sawada <sawada.mshk@gmail.com>
Cc: Justin Pryzby <pryzby@telsasoft.com>
Cc: Peter Geoghegan <pg@bowt.ie>
Cc: pgsql-hackers@postgresql.org <pgsql-hackers@postgresql.org>
Subject: Re: Add index scan progress to pg_stat_progress_vacuum
Date: Mon, 21 Feb 2022 11:09:10 -0800
Message-ID: <20220221190910.GA3740885@nathanxps13> (raw)
In-Reply-To: <1E7CB510-FB91-4DDB-93E1-A19C05F299F3@amazon.com>
References: <6D2C60C2-C09E-4117-A6B8-DCA7A0F5B182@amazon.com>
<22C9CB0C-9576-47AF-8A6E-F9573F73CD4E@amazon.com>
<CAD21AoCWFyvtf_EHHLWqejDjW0=apRugn_=UkpGzY5OgKhkH2g@mail.gmail.com>
<6753CC4A-D1D8-477D-9B82-CFE12169DFF5@amazon.com>
<7A4B3BA8-0768-4463-9B25-25DBECF56B25@amazon.com>
<96387867-CBAF-4CCF-861E-2F4ACBCBBD68@amazon.com>
<DC471B48-A815-4326-AC62-3672CDB9B4AB@amazon.com>
<CC7DB450-A142-43B0-A68F-C6B45DC435F2@amazon.com>
<441BA1FF-23AA-4CEE-9976-7FB693C30D00@amazon.com>
<1E7CB510-FB91-4DDB-93E1-A19C05F299F3@amazon.com>
On Mon, Feb 21, 2022 at 07:03:39PM +0000, Imseih (AWS), Sami wrote:
> Sending again with patch files renamed to ensure correct apply order.
I haven't had a chance to test this too much, but I did look through the
patch set and have a couple of small comments.
+ <row>
+ <entry role="catalog_table_entry"><para role="column_definition">
+ <structfield>indexes_total</structfield> <type>bigint</type>
+ </para>
+ <para>
+ The number of indexes to be processed in the
+ <literal>vacuuming indexes</literal> or <literal>cleaning up indexes</literal> phase
+ of the vacuum.
+ </para></entry>
+ </row>
+
+ <row>
+ <entry role="catalog_table_entry"><para role="column_definition">
+ <structfield>indexes_processed</structfield> <type>bigint</type>
+ </para>
+ <para>
+ The number of indexes processed in the
+ <literal>vacuuming indexes</literal> or <literal>cleaning up indexes</literal> phase.
+ At the start of an index vacuum cycle, this value is set to <literal>0</literal>.
+ </para></entry>
+ </row>
Will these be set to 0 for failsafe vacuums and vacuums with INDEX_CLEANUP
turned off?
+typedef struct VacWorkerProgressInfo
+{
+ int num_vacuums; /* number of active VACUUMS with parallel workers */
+ int max_vacuums; /* max number of VACUUMS with parallel workers */
+ slock_t mutex;
+ VacOneWorkerProgressInfo vacuums[FLEXIBLE_ARRAY_MEMBER];
+} VacWorkerProgressInfo;
max_vacuums appears to just be a local copy of MaxBackends. Does this
information really need to be stored here? Also, is there a strong reason
for using a spinlock instead of an LWLock?
+void
+vacuum_worker_end(int leader_pid)
+{
+ SpinLockAcquire(&vacworkerprogress->mutex);
+ for (int i = 0; i < vacworkerprogress->num_vacuums; i++)
+ {
+ VacOneWorkerProgressInfo *vac = &vacworkerprogress->vacuums[i];
+
+ if (vac->leader_pid == leader_pid)
+ {
+ *vac = vacworkerprogress->vacuums[vacworkerprogress->num_vacuums - 1];
+ vacworkerprogress->num_vacuums--;
+ SpinLockRelease(&vacworkerprogress->mutex);
+ break;
+ }
+ }
+ SpinLockRelease(&vacworkerprogress->mutex);
+}
I see this loop pattern in a couple of places, and it makes me wonder if
this information would fit more naturally in a hash table.
+ if (callback)
+ callback(values, 3);
Why does this need to be set up as a callback function? Could we just call
the function if cmdtype == PROGRESS_COMMAND_VACUUM? ISTM that is pretty
much all this is doing.
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: nathandbossart@gmail.com, simseih@amazon.com, bossartn@amazon.com, sawada.mshk@gmail.com, pryzby@telsasoft.com, pg@bowt.ie
Subject: Re: Add index scan progress to pg_stat_progress_vacuum
In-Reply-To: <20220221190910.GA3740885@nathanxps13>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox