Re: [HACKERS] Block level parallel vacuum
| От | Haribabu Kommi |
|---|---|
| Тема | Re: [HACKERS] Block level parallel vacuum |
| Дата | |
| Msg-id | CAJrrPGcka5BokW1-oeOCK6ir8yTqeNzS2XJZ5r+3ghseHix5+Q@mail.gmail.com обсуждение исходный текст |
| Ответ на | Re: [HACKERS] Block level parallel vacuum (Masahiko Sawada <sawada.mshk@gmail.com>) |
| Список | pgsql-hackers |
On Fri, Mar 22, 2019 at 4:06 PM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
Attached the updated version patch. 0001 patch allows all existing
vacuum options an boolean argument. 0002 patch introduces parallel
lazy vacuum. 0003 patch adds -P (--parallel) option to vacuumdb
command.
0001 patch:
+ PARALLEL [ <replaceable class="parameter">N</replaceable> ]
But this patch contains syntax of PARALLEL but no explanation, I saw that
it is explained in 0002. It is not a problem, but just mentioning.
+ Specifies parallel degree for <literal>PARALLEL</literal> option. The
+ value must be at least 1. If the parallel degree
+ <replaceable class="parameter">integer</replaceable> is omitted, then
+ <command>VACUUM</command> decides the number of workers based on number of
+ indexes on the relation which further limited by
+ <xref linkend="guc-max-parallel-workers-maintenance"/>.
Can we add some more details about backend participation also, parallel workers will
come into picture only when there are 2 indexes in the table.
+ /*
+ * Do post-vacuum cleanup and statistics update for each index if
+ * we're not in parallel lazy vacuum. If in parallel lazy vacuum, do
+ * only post-vacum cleanup and then update statistics after exited
+ * from parallel mode.
+ */
+ lazy_vacuum_all_indexes(vacrelstats, Irel, nindexes, indstats,
+ lps, true);
How about renaming the above function, as it does the cleanup also?
lazy_vacuum_or_cleanup_all_indexes?
+ if (!IsInParallelVacuum(lps))
+ {
+ /*
+ * Update index statistics. If in parallel lazy vacuum, we will
+ * update them after exited from parallel mode.
+ */
+ lazy_update_index_statistics(Irel[idx], stats[idx]);
+
+ if (stats[idx])
+ pfree(stats[idx]);
+ }
The above check in lazy_vacuum_all_indexes can be combined it with the outer
if check where the memcpy is happening. I still feel that the logic around the stats
makes it little bit complex.
+ if (IsParallelWorker())
+ msg = "scanned index \"%s\" to remove %d row versions by parallel vacuum worker";
+ else
+ msg = "scanned index \"%s\" to remove %d row versions";
I feel, this way of error message may not be picked for the translations.
Is there any problem if we duplicate the entire ereport message with changed message?
+ for (i = 0; i < nindexes; i++)
+ {
+ LVIndStats *s = &(copied_indstats[i]);
+
+ if (s->updated)
+ lazy_update_index_statistics(Irel[i], &(s->stats));
+ }
+
+ pfree(copied_indstats);
why can't we use the shared memory directly to update the stats once all the workers
are finished, instead of copying them to a local memory?
+ tab->at_params.nworkers = 0; /* parallel lazy autovacuum is not supported */
User is not required to provide workers number compulsory even that parallel vacuum can
work, so just setting the above parameters doesn't stop the parallel workers, user must
pass the PARALLEL option also. So mentioning that also will be helpful later when we
start supporting it or some one who is reading the code can understand.
Regards,
Haribabu Kommi
Fujitsu Australia
В списке pgsql-hackers по дате отправления: