Set access strategy for parallel vacuum workers

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
11 messages Options
Reply | Threaded
Open this post in threaded view
|

Set access strategy for parallel vacuum workers

akapila
During recent developments in the vacuum, it has been noticed [1] that
parallel vacuum workers don't use any buffer access strategy. I think
we can fix it either by propagating the required information from the
leader or just get the access strategy in each worker separately. The
patches for both approaches for PG-13 are attached.

Thoughts?

[1] - https://www.postgresql.org/message-id/CAH2-Wz%3Dgf6FXW-jPVRdeCZk0QjhduCqH_XD3QbES9wPmhircuA%40mail.gmail.com

--
With Regards,
Amit Kapila.

fix_access_strategy_workers_1.patch (4K) Download Attachment
fix_access_strategy_workers_11.patch (1K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Set access strategy for parallel vacuum workers

Masahiko Sawada
On Wed, Apr 7, 2021 at 7:00 PM Amit Kapila <[hidden email]> wrote:
>
> During recent developments in the vacuum, it has been noticed [1] that
> parallel vacuum workers don't use any buffer access strategy. I think
> we can fix it either by propagating the required information from the
> leader or just get the access strategy in each worker separately. The
> patches for both approaches for PG-13 are attached.

Thank you for starting the new thread.

I'd prefer to just have parallel vacuum workers get BAS_VACUUM buffer
access strategy. If we want to have set different buffer access
strategies or ring buffer sizes for the leader and worker processes,
the former approach would be better. But I think we're unlikely to
want to do that, especially in back branches.

Regards,

--
Masahiko Sawada
EDB:  https://www.enterprisedb.com/


Reply | Threaded
Open this post in threaded view
|

Re: Set access strategy for parallel vacuum workers

Bharath Rupireddy
In reply to this post by akapila
On Wed, Apr 7, 2021 at 3:30 PM Amit Kapila <[hidden email]> wrote:

>
> During recent developments in the vacuum, it has been noticed [1] that
> parallel vacuum workers don't use any buffer access strategy. I think
> we can fix it either by propagating the required information from the
> leader or just get the access strategy in each worker separately. The
> patches for both approaches for PG-13 are attached.
>
> Thoughts?
>
> [1] - https://www.postgresql.org/message-id/CAH2-Wz%3Dgf6FXW-jPVRdeCZk0QjhduCqH_XD3QbES9wPmhircuA%40mail.gmail.com

Note: I have not followed the original discussion in [1].

My understanding of the approach #1 i.e. propagating the vacuum
strategy down to the parallel vacuum workers from the leader is that
the same ring buffer (of 256KB for vacuum) will be used by both leader
and all the workers. In that case, I think we see more page flushes
(thus more IO) because 256KB is now shared by all of them. Whereas
with approach #2 each worker gets its own ring buffer (of 256KB) thus
less IO occurs compared to approach #1.

And in case of parallel inserts (although they are not yet committed
and in various levels discussions) we let each worker get its own ring
buffer (of size 16MB). Whatever the approach is chosen here, I think
it should be consistent.

With Regards,
Bharath Rupireddy.
EnterpriseDB: http://www.enterprisedb.com


Reply | Threaded
Open this post in threaded view
|

Re: Set access strategy for parallel vacuum workers

akapila
On Wed, Apr 7, 2021 at 7:12 PM Bharath Rupireddy
<[hidden email]> wrote:

>
> On Wed, Apr 7, 2021 at 3:30 PM Amit Kapila <[hidden email]> wrote:
> >
> > During recent developments in the vacuum, it has been noticed [1] that
> > parallel vacuum workers don't use any buffer access strategy. I think
> > we can fix it either by propagating the required information from the
> > leader or just get the access strategy in each worker separately. The
> > patches for both approaches for PG-13 are attached.
> >
> > Thoughts?
> >
> > [1] - https://www.postgresql.org/message-id/CAH2-Wz%3Dgf6FXW-jPVRdeCZk0QjhduCqH_XD3QbES9wPmhircuA%40mail.gmail.com
>
> Note: I have not followed the original discussion in [1].
>
> My understanding of the approach #1 i.e. propagating the vacuum
> strategy down to the parallel vacuum workers from the leader is that
> the same ring buffer (of 256KB for vacuum) will be used by both leader
> and all the workers.
>

No that is not the intention, each worker will use its ring buffer.
The first approach just passes the relevant information to workers so
that they can use the same strategy as used by the leader but both
will use separate ring buffer.


--
With Regards,
Amit Kapila.


Reply | Threaded
Open this post in threaded view
|

Re: Set access strategy for parallel vacuum workers

akapila
In reply to this post by Masahiko Sawada
On Wed, Apr 7, 2021 at 5:11 PM Masahiko Sawada <[hidden email]> wrote:

>
> On Wed, Apr 7, 2021 at 7:00 PM Amit Kapila <[hidden email]> wrote:
> >
> > During recent developments in the vacuum, it has been noticed [1] that
> > parallel vacuum workers don't use any buffer access strategy. I think
> > we can fix it either by propagating the required information from the
> > leader or just get the access strategy in each worker separately. The
> > patches for both approaches for PG-13 are attached.
>
> Thank you for starting the new thread.
>
> I'd prefer to just have parallel vacuum workers get BAS_VACUUM buffer
> access strategy. If we want to have set different buffer access
> strategies or ring buffer sizes for the leader and worker processes,
> the former approach would be better. But I think we're unlikely to
> want to do that, especially in back branches.
>

Fair enough. Just to be clear, you prefer to go with
fix_access_strategy_workers_11.patch, right?

--
With Regards,
Amit Kapila.


Reply | Threaded
Open this post in threaded view
|

Re: Set access strategy for parallel vacuum workers

Masahiko Sawada
On Thu, Apr 8, 2021 at 12:17 PM Amit Kapila <[hidden email]> wrote:

>
> On Wed, Apr 7, 2021 at 5:11 PM Masahiko Sawada <[hidden email]> wrote:
> >
> > On Wed, Apr 7, 2021 at 7:00 PM Amit Kapila <[hidden email]> wrote:
> > >
> > > During recent developments in the vacuum, it has been noticed [1] that
> > > parallel vacuum workers don't use any buffer access strategy. I think
> > > we can fix it either by propagating the required information from the
> > > leader or just get the access strategy in each worker separately. The
> > > patches for both approaches for PG-13 are attached.
> >
> > Thank you for starting the new thread.
> >
> > I'd prefer to just have parallel vacuum workers get BAS_VACUUM buffer
> > access strategy. If we want to have set different buffer access
> > strategies or ring buffer sizes for the leader and worker processes,
> > the former approach would be better. But I think we're unlikely to
> > want to do that, especially in back branches.
> >
>
> Fair enough. Just to be clear, you prefer to go with
> fix_access_strategy_workers_11.patch, right?

That's right.

In HEAD, we fixed it in that way in commit f6b8f19.

Regards,

--
Masahiko Sawada
EDB:  https://www.enterprisedb.com/


Reply | Threaded
Open this post in threaded view
|

Re: Set access strategy for parallel vacuum workers

akapila
On Thu, Apr 8, 2021 at 8:52 AM Masahiko Sawada <[hidden email]> wrote:

>
> On Thu, Apr 8, 2021 at 12:17 PM Amit Kapila <[hidden email]> wrote:
> >
> > On Wed, Apr 7, 2021 at 5:11 PM Masahiko Sawada <[hidden email]> wrote:
> > >
> > > On Wed, Apr 7, 2021 at 7:00 PM Amit Kapila <[hidden email]> wrote:
> > > >
> > > > During recent developments in the vacuum, it has been noticed [1] that
> > > > parallel vacuum workers don't use any buffer access strategy. I think
> > > > we can fix it either by propagating the required information from the
> > > > leader or just get the access strategy in each worker separately. The
> > > > patches for both approaches for PG-13 are attached.
> > >
> > > Thank you for starting the new thread.
> > >
> > > I'd prefer to just have parallel vacuum workers get BAS_VACUUM buffer
> > > access strategy. If we want to have set different buffer access
> > > strategies or ring buffer sizes for the leader and worker processes,
> > > the former approach would be better. But I think we're unlikely to
> > > want to do that, especially in back branches.
> > >
> >
> > Fair enough. Just to be clear, you prefer to go with
> > fix_access_strategy_workers_11.patch, right?
>
> That's right.
>

Okay, I'll wait for a day or two to see if anyone else has comments or
suggestions.

--
With Regards,
Amit Kapila.


Reply | Threaded
Open this post in threaded view
|

Re: Set access strategy for parallel vacuum workers

Bharath Rupireddy
In reply to this post by akapila
On Thu, Apr 8, 2021 at 8:44 AM Amit Kapila <[hidden email]> wrote:

>
> On Wed, Apr 7, 2021 at 7:12 PM Bharath Rupireddy
> <[hidden email]> wrote:
> >
> > On Wed, Apr 7, 2021 at 3:30 PM Amit Kapila <[hidden email]> wrote:
> > >
> > > During recent developments in the vacuum, it has been noticed [1] that
> > > parallel vacuum workers don't use any buffer access strategy. I think
> > > we can fix it either by propagating the required information from the
> > > leader or just get the access strategy in each worker separately. The
> > > patches for both approaches for PG-13 are attached.
> > >
> > > Thoughts?
> > >
> > > [1] - https://www.postgresql.org/message-id/CAH2-Wz%3Dgf6FXW-jPVRdeCZk0QjhduCqH_XD3QbES9wPmhircuA%40mail.gmail.com
> >
> > Note: I have not followed the original discussion in [1].
> >
> > My understanding of the approach #1 i.e. propagating the vacuum
> > strategy down to the parallel vacuum workers from the leader is that
> > the same ring buffer (of 256KB for vacuum) will be used by both leader
> > and all the workers.
> >
>
> No that is not the intention, each worker will use its ring buffer.
> The first approach just passes the relevant information to workers so
> that they can use the same strategy as used by the leader but both
> will use separate ring buffer.

Thanks for the clarification. I understood now.

On the patch fix_access_strategy_workers_11.patch: can we have the
more descriptive comment like "/* Each parallel VACUUM worker gets its
own access strategy */" that's introduced by commit f6b8f19 instead of
just saying "/* Set up vacuum access strategy */" which is quite
obvious from the function name GetAccessStrategy?

With Regards,
Bharath Rupireddy.
EnterpriseDB: http://www.enterprisedb.com


Reply | Threaded
Open this post in threaded view
|

Re: Set access strategy for parallel vacuum workers

akapila
On Thu, Apr 8, 2021 at 9:42 AM Bharath Rupireddy
<[hidden email]> wrote:

>
> On Thu, Apr 8, 2021 at 8:44 AM Amit Kapila <[hidden email]> wrote:
> >
> > On Wed, Apr 7, 2021 at 7:12 PM Bharath Rupireddy
> > <[hidden email]> wrote:
> > >
> > > On Wed, Apr 7, 2021 at 3:30 PM Amit Kapila <[hidden email]> wrote:
> > > >
> > > > During recent developments in the vacuum, it has been noticed [1] that
> > > > parallel vacuum workers don't use any buffer access strategy. I think
> > > > we can fix it either by propagating the required information from the
> > > > leader or just get the access strategy in each worker separately. The
> > > > patches for both approaches for PG-13 are attached.
> > > >
> > > > Thoughts?
> > > >
> > > > [1] - https://www.postgresql.org/message-id/CAH2-Wz%3Dgf6FXW-jPVRdeCZk0QjhduCqH_XD3QbES9wPmhircuA%40mail.gmail.com
> > >
> > > Note: I have not followed the original discussion in [1].
> > >
> > > My understanding of the approach #1 i.e. propagating the vacuum
> > > strategy down to the parallel vacuum workers from the leader is that
> > > the same ring buffer (of 256KB for vacuum) will be used by both leader
> > > and all the workers.
> > >
> >
> > No that is not the intention, each worker will use its ring buffer.
> > The first approach just passes the relevant information to workers so
> > that they can use the same strategy as used by the leader but both
> > will use separate ring buffer.
>
> Thanks for the clarification. I understood now.
>
> On the patch fix_access_strategy_workers_11.patch: can we have the
> more descriptive comment like "/* Each parallel VACUUM worker gets its
> own access strategy */" that's introduced by commit f6b8f19 instead of
> just saying "/* Set up vacuum access strategy */" which is quite
> obvious from the function name GetAccessStrategy?
>

Yeah, I will change that before commit unless there are more suggestions.


--
With Regards,
Amit Kapila.


Reply | Threaded
Open this post in threaded view
|

Re: Set access strategy for parallel vacuum workers

Bharath Rupireddy
On Thu, Apr 8, 2021 at 11:22 AM Amit Kapila <[hidden email]> wrote:
> Yeah, I will change that before commit unless there are more suggestions.

I have no further comments on the patch
fix_access_strategy_workers_11.patch, it LGTM.

With Regards,
Bharath Rupireddy.
EnterpriseDB: http://www.enterprisedb.com


Reply | Threaded
Open this post in threaded view
|

Re: Set access strategy for parallel vacuum workers

akapila
On Thu, Apr 8, 2021 at 12:37 PM Bharath Rupireddy
<[hidden email]> wrote:
>
> On Thu, Apr 8, 2021 at 11:22 AM Amit Kapila <[hidden email]> wrote:
> > Yeah, I will change that before commit unless there are more suggestions.
>
> I have no further comments on the patch
> fix_access_strategy_workers_11.patch, it LGTM.
>

Thanks, seeing no further comments, I have pushed
fix_access_strategy_workers_11.patch.

--
With Regards,
Amit Kapila.