Thread: parallel vacuum - few questions on docs, comments and code
Hi,
I was going through the parallel vacuum docs and code. I found below
things, please someone clarify:
1) I see that a term "parallel degree" is used in the docs, code
comments, error messages "parallel vacuum degree must be a
non-negative integer", "parallel vacuum degree must be between 0 and
%d". Is there any specific reason to use the term "parallel degree"?
In the docs and code comments we generally use "parallel workers".
2) The error messages "parallel vacuum degree must be between 0 and
%d" and "parallel option requires a value between 0 and %d" look
inconsistent.
3) Should the Assert(nindexes > 0); in begin_parallel_vacuum just be
Assert(nindexes > 1); as this function is entered only when indexes
are > 1?
4) IIUC, below comment says that even if PARALLEL 0 is specified with
VACUUM command, there are chances that the indexes are vacuumed in
parallel. Isn't it a bit unusual that a user specified 0 workers but
still the system is picking up parallelism? I'm sure this would have
been discussed, but I'm curious to know the reason.
* nrequested is the number of parallel workers that user requested. If
* nrequested is 0, we compute the parallel degree based on nindexes, that is
* the number of indexes that support parallel vacuum.
5) Can the parallel_workers in below condition ever be negative in
begin_parallel_vacuum? I think we can just have if (parallel_workers
== 0).
/* Can't perform vacuum in parallel */
if (parallel_workers <= 0)
6) I think, instead of saying "using integer background workers", we
can just say "using specified or lesser number of background workers".
From the docs: Perform index vacuum and index cleanup phases of VACUUM
in parallel using integer background workers
We can say "workers specified will be used during execution"
From the docs: workers specified in integer will be used during execution
7) I think we need a comma after "if any" .
From the docs: which is limited by the number of workers specified
with PARALLEL option if any which is further limited by
8) Is it still true that if parallel workers are specified as 0 the
parallelism will not be picked up?
From the docs: This feature is known as parallel vacuum. To disable
this feature, one can use PARALLEL option and specify parallel workers
as zero.
With Regards,
Bharath Rupireddy.
EnterpriseDB: http://www.enterprisedb.com
On Tue, May 11, 2021 at 05:37:50PM +0530, Bharath Rupireddy wrote:
> 3) Should the Assert(nindexes > 0); in begin_parallel_vacuum just be
> Assert(nindexes > 1); as this function is entered only when indexes
> are > 1?
I think you're right, at least with the current implementation that
parallelization is done across indexes. Same in parallel_vacuum_main.
> 4) IIUC, below comment says that even if PARALLEL 0 is specified with
> VACUUM command, there are chances that the indexes are vacuumed in
> parallel. Isn't it a bit unusual that a user specified 0 workers but
> still the system is picking up parallelism? I'm sure this would have
> been discussed, but I'm curious to know the reason.
> * nrequested is the number of parallel workers that user requested. If
> * nrequested is 0, we compute the parallel degree based on nindexes, that is
> * the number of indexes that support parallel vacuum.
No - nrequested is not actually the number of workers requested - it seems like
a poor choice of name.
This is the key part:
src/include/commands/vacuum.h
* The number of parallel vacuum workers. 0 by default which means choose
* based on the number of indexes. -1 indicates parallel vacuum is
* disabled.
*/
int nworkers;
} VacuumParams;
The parsing code is in src/backend/commands/vacuum.c.
> 8) Is it still true that if parallel workers are specified as 0 the
> parallelism will not be picked up?
> From the docs: This feature is known as parallel vacuum. To disable
> this feature, one can use PARALLEL option and specify parallel workers
> as zero.
I think it's the same answer as above.
--
Justin
On Tue, May 11, 2021 at 6:31 PM Justin Pryzby <pryzby@telsasoft.com> wrote: > > On Tue, May 11, 2021 at 05:37:50PM +0530, Bharath Rupireddy wrote: > > 3) Should the Assert(nindexes > 0); in begin_parallel_vacuum just be > > Assert(nindexes > 1); as this function is entered only when indexes > > are > 1? > > I think you're right, at least with the current implementation that > parallelization is done across indexes. Same in parallel_vacuum_main. > Yeah, as code stands both of you are right. However, it can be helpful to test parallelism even with one index say if we implement something like force_parallel_mode = regress or parallel_leader_participation = off. -- With Regards, Amit Kapila.
On Tue, May 11, 2021 at 5:38 PM Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com> wrote: > > Hi, > > I was going through the parallel vacuum docs and code. I found below > things, please someone clarify: > > 1) I see that a term "parallel degree" is used in the docs, code > comments, error messages "parallel vacuum degree must be a > non-negative integer", "parallel vacuum degree must be between 0 and > %d". Is there any specific reason to use the term "parallel degree"? > In the docs and code comments we generally use "parallel workers". I think using "parallel workers" will be more consistent. > 2) The error messages "parallel vacuum degree must be between 0 and > %d" and "parallel option requires a value between 0 and %d" look > inconsistent. +1 > 3) Should the Assert(nindexes > 0); in begin_parallel_vacuum just be > Assert(nindexes > 1); as this function is entered only when indexes > are > 1? > 4) IIUC, below comment says that even if PARALLEL 0 is specified with > VACUUM command, there are chances that the indexes are vacuumed in > parallel. Isn't it a bit unusual that a user specified 0 workers but > still the system is picking up parallelism? I'm sure this would have > been discussed, but I'm curious to know the reason. > * nrequested is the number of parallel workers that user requested. If > * nrequested is 0, we compute the parallel degree based on nindexes, that is > * the number of indexes that support parallel vacuum. > 5) Can the parallel_workers in below condition ever be negative in > begin_parallel_vacuum? I think we can just have if (parallel_workers > == 0). > /* Can't perform vacuum in parallel */ > if (parallel_workers <= 0) Yes it should if (parallel_workers == 0) > 8) Is it still true that if parallel workers are specified as 0 the > parallelism will not be picked up? > From the docs: This feature is known as parallel vacuum. To disable > this feature, one can use PARALLEL option and specify parallel workers > as zero. Yes, by default this is enabled so for disabling user need to give PARALLEL as 0. -- Regards, Dilip Kumar EnterpriseDB: http://www.enterprisedb.com
On Tue, May 11, 2021 at 5:38 PM Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com> wrote: > > I was going through the parallel vacuum docs and code. I found below > things, please someone clarify: > > 1) I see that a term "parallel degree" is used in the docs, code > comments, error messages "parallel vacuum degree must be a > non-negative integer", "parallel vacuum degree must be between 0 and > %d". Is there any specific reason to use the term "parallel degree"? > In the docs and code comments we generally use "parallel workers". > The parallel degree term is used here to indicate that we compute how much parallelism we can achieve based on the indexes. > 2) The error messages "parallel vacuum degree must be between 0 and > %d" and "parallel option requires a value between 0 and %d" look > inconsistent. > I think we can make them consistent. > 5) Can the parallel_workers in below condition ever be negative in > begin_parallel_vacuum? I think we can just have if (parallel_workers > == 0). > /* Can't perform vacuum in parallel */ > if (parallel_workers <= 0) Even if it can't go negative in the current code, I don't see a problem with the current code. It seems safe like this. > 6) I think, instead of saying "using integer background workers", we > can just say "using specified or lesser number of background workers". > From the docs: Perform index vacuum and index cleanup phases of VACUUM > in parallel using integer background workers > We can say "workers specified will be used during execution" > From the docs: workers specified in integer will be used during execution > The docs here refer to "PARALLEL integer" specified in specs, so not sure if the proposed text is better. -- With Regards, Amit Kapila.
On Tue, May 11, 2021 at 6:31 PM Justin Pryzby <pryzby@telsasoft.com> wrote:
> > 4) IIUC, below comment says that even if PARALLEL 0 is specified with
> > VACUUM command, there are chances that the indexes are vacuumed in
> > parallel. Isn't it a bit unusual that a user specified 0 workers but
> > still the system is picking up parallelism? I'm sure this would have
> > been discussed, but I'm curious to know the reason.
> > * nrequested is the number of parallel workers that user requested. If
> > * nrequested is 0, we compute the parallel degree based on nindexes, that is
> > * the number of indexes that support parallel vacuum.
>
> No - nrequested is not actually the number of workers requested - it seems like
> a poor choice of name.
>
> This is the key part:
>
> src/include/commands/vacuum.h
> * The number of parallel vacuum workers. 0 by default which means choose
> * based on the number of indexes. -1 indicates parallel vacuum is
> * disabled.
> */
> int nworkers;
> } VacuumParams;
Thanks. The name "nworkers" looks fine to me after reading the comment
above it. And the parallelism will be chosen by default.
/* By default parallel vacuum is enabled */
params.nworkers = 0;
With Regards,
Bharath Rupireddy.
EnterpriseDB: http://www.enterprisedb.com
On Wed, May 12, 2021 at 9:48 AM Amit Kapila <amit.kapila16@gmail.com> wrote: > > On Tue, May 11, 2021 at 6:31 PM Justin Pryzby <pryzby@telsasoft.com> wrote: > > > > On Tue, May 11, 2021 at 05:37:50PM +0530, Bharath Rupireddy wrote: > > > 3) Should the Assert(nindexes > 0); in begin_parallel_vacuum just be > > > Assert(nindexes > 1); as this function is entered only when indexes > > > are > 1? > > > > I think you're right, at least with the current implementation that > > parallelization is done across indexes. Same in parallel_vacuum_main. > > > > Yeah, as code stands both of you are right. However, it can be helpful > to test parallelism even with one index say if we implement something > like force_parallel_mode = regress or parallel_leader_participation = > off. I see that currently we don't have it yet. Is it worth implementing them? Something like 1) when force_parallel_mode = regress, spawn one parallel worker, send the relation information to it, so that it performs vacuuming both the relation and it's indexes. 2) parallel_leader_participation = off, spawn workers as specified, but don't let the leader to vacuum index, so that any worker can pick it up. I'm not sure of the complexity though. With Regards, Bharath Rupireddy. EnterpriseDB: http://www.enterprisedb.com
On Wed, May 12, 2021 at 11:10 AM Amit Kapila <amit.kapila16@gmail.com> wrote: > > On Tue, May 11, 2021 at 5:38 PM Bharath Rupireddy > <bharath.rupireddyforpostgres@gmail.com> wrote: > > > > I was going through the parallel vacuum docs and code. I found below > > things, please someone clarify: > > > > 1) I see that a term "parallel degree" is used in the docs, code > > comments, error messages "parallel vacuum degree must be a > > non-negative integer", "parallel vacuum degree must be between 0 and > > %d". Is there any specific reason to use the term "parallel degree"? > > In the docs and code comments we generally use "parallel workers". > > > > The parallel degree term is used here to indicate that we compute how > much parallelism we can achieve based on the indexes. Yeah, I get it. Even if users don't specify a parallel option there are chances that parallelism is picked. So the parallel degree is the final number of workers that are chosen by the server for vacuuming indexes. And, I think that parallel degree is something internal to the server, and it's better we replace it in the vacuumdb.sgml, change PARALLEL_DEGREE to PARALLEL_WORKERS in vacuumdb.c and change the error message "parallel vacuum degree must be a non-negative integer" to "parallel workers for vacuum must be greater than or equal to zero". Thoughts? > > 2) The error messages "parallel vacuum degree must be between 0 and > > %d" and "parallel option requires a value between 0 and %d" look > > inconsistent. > > > > I think we can make them consistent. How about only one message "parallel option requires a value between 0 and %d" for both cases below? IMO they essentially mean the same thing. postgres=# vacuum (parallel ) t1; ERROR: parallel option requires a value between 0 and 1024 postgres=# vacuum (parallel -4) t1; ERROR: parallel vacuum degree must be between 0 and 1024 > > 5) Can the parallel_workers in below condition ever be negative in > > begin_parallel_vacuum? I think we can just have if (parallel_workers > > == 0). > > /* Can't perform vacuum in parallel */ > > if (parallel_workers <= 0) > > Even if it can't go negative in the current code, I don't see a > problem with the current code. It seems safe like this. Okay. > > 6) I think, instead of saying "using integer background workers", we > > can just say "using specified or lesser number of background workers". > > From the docs: Perform index vacuum and index cleanup phases of VACUUM > > in parallel using integer background workers > > We can say "workers specified will be used during execution" > > From the docs: workers specified in integer will be used during execution > > > The docs here refer to "PARALLEL integer" specified in specs, so not > sure if the proposed text is better. IMO, "using the number of background workers specified with the option" looks better than "using integer background workers". Thoughts? With Regards, Bharath Rupireddy. EnterpriseDB: http://www.enterprisedb.com
On Wed, May 12, 2021 at 6:37 PM Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com> wrote: > > On Wed, May 12, 2021 at 11:10 AM Amit Kapila <amit.kapila16@gmail.com> wrote: > > > > On Tue, May 11, 2021 at 5:38 PM Bharath Rupireddy > > <bharath.rupireddyforpostgres@gmail.com> wrote: > > > > > > I was going through the parallel vacuum docs and code. I found below > > > things, please someone clarify: > > > > > > 1) I see that a term "parallel degree" is used in the docs, code > > > comments, error messages "parallel vacuum degree must be a > > > non-negative integer", "parallel vacuum degree must be between 0 and > > > %d". Is there any specific reason to use the term "parallel degree"? > > > In the docs and code comments we generally use "parallel workers". > > > > > > > The parallel degree term is used here to indicate that we compute how > > much parallelism we can achieve based on the indexes. > > Yeah, I get it. Even if users don't specify a parallel option there > are chances that parallelism is picked. So the parallel degree is the > final number of workers that are chosen by the server for vacuuming > indexes. And, I think that parallel degree is something internal to > the server, and it's better we replace it in the vacuumdb.sgml, change > PARALLEL_DEGREE to PARALLEL_WORKERS in vacuumdb.c and change the error > message "parallel vacuum degree must be a non-negative integer" to > "parallel workers for vacuum must be greater than or equal to zero". > > Thoughts? > > > > 2) The error messages "parallel vacuum degree must be between 0 and > > > %d" and "parallel option requires a value between 0 and %d" look > > > inconsistent. > > > > > > > I think we can make them consistent. > > How about only one message "parallel option requires a value between 0 > and %d" for both cases below? IMO they essentially mean the same > thing. > I am fine with changing what you are proposing in the above two points. Sawada-San, any thoughts? > > > > 6) I think, instead of saying "using integer background workers", we > > > can just say "using specified or lesser number of background workers". > > > From the docs: Perform index vacuum and index cleanup phases of VACUUM > > > in parallel using integer background workers > > > We can say "workers specified will be used during execution" > > > From the docs: workers specified in integer will be used during execution > > > > > The docs here refer to "PARALLEL integer" specified in specs, so not > > sure if the proposed text is better. > > IMO, "using the number of background workers specified with the > option" looks better than "using integer background workers". > Thoughts? > I am not too sure about this point. I guess we can leave it for now. -- With Regards, Amit Kapila.
On Wed, May 12, 2021 at 6:30 PM Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com> wrote: > > On Wed, May 12, 2021 at 9:48 AM Amit Kapila <amit.kapila16@gmail.com> wrote: > > > > On Tue, May 11, 2021 at 6:31 PM Justin Pryzby <pryzby@telsasoft.com> wrote: > > > > > > On Tue, May 11, 2021 at 05:37:50PM +0530, Bharath Rupireddy wrote: > > > > 3) Should the Assert(nindexes > 0); in begin_parallel_vacuum just be > > > > Assert(nindexes > 1); as this function is entered only when indexes > > > > are > 1? > > > > > > I think you're right, at least with the current implementation that > > > parallelization is done across indexes. Same in parallel_vacuum_main. > > > > > > > Yeah, as code stands both of you are right. However, it can be helpful > > to test parallelism even with one index say if we implement something > > like force_parallel_mode = regress or parallel_leader_participation = > > off. > > I see that currently we don't have it yet. Is it worth implementing > them? Something like 1) when force_parallel_mode = regress, spawn one > parallel worker, send the relation information to it, so that it > performs vacuuming both the relation and it's indexes. 2) > parallel_leader_participation = off, spawn workers as specified, but > don't let the leader to vacuum index, so that any worker can pick it > up. I'm not sure of the complexity though. > We had some patch on the above lines in the original development thread which has been used for testing during development but not sure how much useful it is now. However, I am fine if others think something like that is useful. -- With Regards, Amit Kapila.
On Thu, May 13, 2021 at 3:25 PM Amit Kapila <amit.kapila16@gmail.com> wrote: > > On Wed, May 12, 2021 at 6:37 PM Bharath Rupireddy > <bharath.rupireddyforpostgres@gmail.com> wrote: > > > > On Wed, May 12, 2021 at 11:10 AM Amit Kapila <amit.kapila16@gmail.com> wrote: > > > > > > On Tue, May 11, 2021 at 5:38 PM Bharath Rupireddy > > > <bharath.rupireddyforpostgres@gmail.com> wrote: > > > > > > > > I was going through the parallel vacuum docs and code. I found below > > > > things, please someone clarify: > > > > > > > > 1) I see that a term "parallel degree" is used in the docs, code > > > > comments, error messages "parallel vacuum degree must be a > > > > non-negative integer", "parallel vacuum degree must be between 0 and > > > > %d". Is there any specific reason to use the term "parallel degree"? > > > > In the docs and code comments we generally use "parallel workers". > > > > > > > > > > The parallel degree term is used here to indicate that we compute how > > > much parallelism we can achieve based on the indexes. > > > > Yeah, I get it. Even if users don't specify a parallel option there > > are chances that parallelism is picked. So the parallel degree is the > > final number of workers that are chosen by the server for vacuuming > > indexes. And, I think that parallel degree is something internal to > > the server, and it's better we replace it in the vacuumdb.sgml, change > > PARALLEL_DEGREE to PARALLEL_WORKERS in vacuumdb.c and change the error > > message "parallel vacuum degree must be a non-negative integer" to > > "parallel workers for vacuum must be greater than or equal to zero". > > > > Thoughts? I'm fine with this change. > > > > > > 2) The error messages "parallel vacuum degree must be between 0 and > > > > %d" and "parallel option requires a value between 0 and %d" look > > > > inconsistent. > > > > > > > > > > I think we can make them consistent. > > > > How about only one message "parallel option requires a value between 0 > > and %d" for both cases below? IMO they essentially mean the same > > thing. The change looks good to me in terms of consistency but even the current messages also make sense and are slightly clearer to me aside from using the term "degree". If the user lacks an integer after PARALLEL option, we say "parallel option requires a value between 0 and %d" and if the user specifies an invalid number to the option we say "parallel vacuum degree must be between 0 and %d”. We use the message something like “AAA must be between X and Y” also in other places if users input an invalid value. I'm not sure the consistency is important here but another idea to improve the error message would be to change "parallel vacuum degree must be between 0 and %d” to "the number of parallel workers must be between 0 and %d” (or using “parallel workers for vacuum” instead of “the number of parallel workers”) while leaving another message as it is. Regards, -- Masahiko Sawada EDB: https://www.enterprisedb.com/
On Thu, May 13, 2021 at 7:00 PM Masahiko Sawada <sawada.mshk@gmail.com> wrote: > > > Yeah, I get it. Even if users don't specify a parallel option there > > > are chances that parallelism is picked. So the parallel degree is the > > > final number of workers that are chosen by the server for vacuuming > > > indexes. And, I think that parallel degree is something internal to > > > the server, and it's better we replace it in the vacuumdb.sgml, change > > > PARALLEL_DEGREE to PARALLEL_WORKERS in vacuumdb.c and change the error > > > message "parallel vacuum degree must be a non-negative integer" to > > > "parallel workers for vacuum must be greater than or equal to zero". > > > > > > Thoughts? > > I'm fine with this change. Thanks. > is important here but another idea to improve the error message would > be to change "parallel vacuum degree must be between 0 and %d” to "the > number of parallel workers must be between 0 and %d” (or using > “parallel workers for vacuum” instead of “the number of parallel > workers”) while leaving another message as it is. Done that way. PSA patch. With Regards, Bharath Rupireddy. EnterpriseDB: http://www.enterprisedb.com
Attachment
On Thu, May 13, 2021 at 9:00 PM Bharath Rupireddy
<bharath.rupireddyforpostgres@gmail.com> wrote:
>
> Done that way.
>
> PSA patch.
Your changes look good. About changing the "non-negative integer" to
"greater than or equal to zero", there is another thread [1], I am not
sure that have we concluded anything there yet.
- pg_log_error("parallel vacuum degree must be a non-negative integer");
+ pg_log_error("parallel workers for vacuum must be greater than or
equal to zero");
exit(1);
[1]
https://www.postgresql.org/message-id/OS0PR01MB5716415335A06B489F1B3A8194569@OS0PR01MB5716.jpnprd01.prod.outlook.com
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
On Fri, May 14, 2021 at 10:43 AM Dilip Kumar <dilipbalaut@gmail.com> wrote:
>
> On Thu, May 13, 2021 at 9:00 PM Bharath Rupireddy
> <bharath.rupireddyforpostgres@gmail.com> wrote:
> >
> > Done that way.
> >
> > PSA patch.
>
> Your changes look good. About changing the "non-negative integer" to
> "greater than or equal to zero", there is another thread [1], I am not
> sure that have we concluded anything there yet.
>
> - pg_log_error("parallel vacuum degree must be a non-negative integer");
> + pg_log_error("parallel workers for vacuum must be greater than or
> equal to zero");
> exit(1);
>
> [1]
https://www.postgresql.org/message-id/OS0PR01MB5716415335A06B489F1B3A8194569@OS0PR01MB5716.jpnprd01.prod.outlook.com
Yeah. Tom proposed if (foo <= 0) { error:"foo must be greater than
zero" } at [1]. In the subsequent messages both Michael and I agreed
with that. But we also have cases like if (foo < 0) for which I think
{ error:"foo must be greater than or equal to zero" } would be better,
similar to what's proposed. Please feel free to provide your thoughts
there in that thread.
[1] - https://www.postgresql.org/message-id/621822.1620655780%40sss.pgh.pa.us
With Regards,
Bharath Rupireddy.
EnterpriseDB: http://www.enterprisedb.com
On Fri, May 14, 2021 at 6:00 PM Bharath Rupireddy
<bharath.rupireddyforpostgres@gmail.com> wrote:
>
> On Fri, May 14, 2021 at 10:43 AM Dilip Kumar <dilipbalaut@gmail.com> wrote:
> >
> >
> > Your changes look good. About changing the "non-negative integer" to
> > "greater than or equal to zero", there is another thread [1], I am not
> > sure that have we concluded anything there yet.
> >
> > - pg_log_error("parallel vacuum degree must be a non-negative integer");
> > + pg_log_error("parallel workers for vacuum must be greater than or
> > equal to zero");
> > exit(1);
> >
> > [1]
https://www.postgresql.org/message-id/OS0PR01MB5716415335A06B489F1B3A8194569@OS0PR01MB5716.jpnprd01.prod.outlook.com
>
> Yeah. Tom proposed if (foo <= 0) { error:"foo must be greater than
> zero" } at [1]. In the subsequent messages both Michael and I agreed
> with that. But we also have cases like if (foo < 0) for which I think
> { error:"foo must be greater than or equal to zero" } would be better,
> similar to what's proposed. Please feel free to provide your thoughts
> there in that thread.
>
I responded on that thread and it seems there is no object to the new
message. I have a minor comment on your patch:
- printf(_(" -P, --parallel=PARALLEL_DEGREE use this many background
workers for vacuum, if available\n"));
+ printf(_(" -P, --parallel=PARALLEL_WORKERS use this many background
workers for vacuum, if available\n"));
If the patch changes the vacuumdb code as above then isn't it better
to change the vacuumdb docs to reflect the same. See below part of
vacuumdb docs:
-P parallel_degree
--parallel=parallel_degree
Also, can you please check if your patch works for PG-13 as well
because I think it is better to backpatch it?
--
With Regards,
Amit Kapila.
On Fri, May 21, 2021 at 11:10 AM Amit Kapila <amit.kapila16@gmail.com> wrote:
> I responded on that thread and it seems there is no object to the new
> message. I have a minor comment on your patch:
Thanks Amit!
> - printf(_(" -P, --parallel=PARALLEL_DEGREE use this many background
> workers for vacuum, if available\n"));
> + printf(_(" -P, --parallel=PARALLEL_WORKERS use this many background
> workers for vacuum, if available\n"));
>
> If the patch changes the vacuumdb code as above then isn't it better
> to change the vacuumdb docs to reflect the same. See below part of
> vacuumdb docs:
> -P parallel_degree
> --parallel=parallel_degree
Changed.
> Also, can you please check if your patch works for PG-13 as well
> because I think it is better to backpatch it?
I'm not sure about backpatching as it is not a critical bug fix. Since
the changes are user visible, I think that it's okay to backpatch.
Anyways, attaching patches for both master and v13 branch. Please
review it further.
With Regards,
Bharath Rupireddy.
EnterpriseDB: http://www.enterprisedb.com
Attachment
On Fri, May 21, 2021 at 1:30 PM Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com> wrote: > > On Fri, May 21, 2021 at 11:10 AM Amit Kapila <amit.kapila16@gmail.com> wrote: > > > > If the patch changes the vacuumdb code as above then isn't it better > > to change the vacuumdb docs to reflect the same. See below part of > > vacuumdb docs: > > -P parallel_degree > > --parallel=parallel_degree > > Changed. > > > Also, can you please check if your patch works for PG-13 as well > > because I think it is better to backpatch it? > > I'm not sure about backpatching as it is not a critical bug fix. Since > the changes are user visible, I think that it's okay to backpatch. > Yes, as it is a user-visible change (though minor) so I thought it would be good to backpatch this. Does anyone else have any opinion on this? -- With Regards, Amit Kapila.
On Fri, May 21, 2021 at 3:33 PM Amit Kapila <amit.kapila16@gmail.com> wrote: > > On Fri, May 21, 2021 at 1:30 PM Bharath Rupireddy > <bharath.rupireddyforpostgres@gmail.com> wrote: > > > > On Fri, May 21, 2021 at 11:10 AM Amit Kapila <amit.kapila16@gmail.com> wrote: > > > > > > If the patch changes the vacuumdb code as above then isn't it better > > > to change the vacuumdb docs to reflect the same. See below part of > > > vacuumdb docs: > > > -P parallel_degree > > > --parallel=parallel_degree > > > > Changed. > > > > > Also, can you please check if your patch works for PG-13 as well > > > because I think it is better to backpatch it? > > > > I'm not sure about backpatching as it is not a critical bug fix. Since > > the changes are user visible, I think that it's okay to backpatch. > > > > Yes, as it is a user-visible change (though minor) so I thought it > would be good to backpatch this. Does anyone else have any opinion on > this? > Pushed! -- With Regards, Amit Kapila.