Set access strategy for parallel vacuum workers

On Wed, Apr 7, 2021 at 7:00 PM Amit Kapila <amit.kapila16@gmail.com> 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/

On Wed, Apr 7, 2021 at 3:30 PM Amit Kapila <amit.kapila16@gmail.com> 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] - /messages/by-id/CAH2-Wz=gf6FXW-jPVRdeCZk0QjhduCqH_XD3QbES9wPmhircuA@mail.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

On Wed, Apr 7, 2021 at 7:12 PM Bharath Rupireddy
<bharath.rupireddyforpostgres@gmail.com> wrote:

On Wed, Apr 7, 2021 at 3:30 PM Amit Kapila <amit.kapila16@gmail.com> 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] - /messages/by-id/CAH2-Wz=gf6FXW-jPVRdeCZk0QjhduCqH_XD3QbES9wPmhircuA@mail.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.

On Wed, Apr 7, 2021 at 5:11 PM Masahiko Sawada <sawada.mshk@gmail.com> wrote:

On Wed, Apr 7, 2021 at 7:00 PM Amit Kapila <amit.kapila16@gmail.com> 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.

On Thu, Apr 8, 2021 at 12:17 PM Amit Kapila <amit.kapila16@gmail.com> wrote:

On Wed, Apr 7, 2021 at 5:11 PM Masahiko Sawada <sawada.mshk@gmail.com> wrote:

On Wed, Apr 7, 2021 at 7:00 PM Amit Kapila <amit.kapila16@gmail.com> 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/

On Thu, Apr 8, 2021 at 8:52 AM Masahiko Sawada <sawada.mshk@gmail.com> wrote:

On Thu, Apr 8, 2021 at 12:17 PM Amit Kapila <amit.kapila16@gmail.com> wrote:

On Wed, Apr 7, 2021 at 5:11 PM Masahiko Sawada <sawada.mshk@gmail.com> wrote:

On Wed, Apr 7, 2021 at 7:00 PM Amit Kapila <amit.kapila16@gmail.com> 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.

On Thu, Apr 8, 2021 at 8:44 AM Amit Kapila <amit.kapila16@gmail.com> wrote:

On Wed, Apr 7, 2021 at 7:12 PM Bharath Rupireddy
<bharath.rupireddyforpostgres@gmail.com> wrote:

On Wed, Apr 7, 2021 at 3:30 PM Amit Kapila <amit.kapila16@gmail.com> 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] - /messages/by-id/CAH2-Wz=gf6FXW-jPVRdeCZk0QjhduCqH_XD3QbES9wPmhircuA@mail.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

On Thu, Apr 8, 2021 at 9:42 AM Bharath Rupireddy
<bharath.rupireddyforpostgres@gmail.com> wrote:

On Thu, Apr 8, 2021 at 8:44 AM Amit Kapila <amit.kapila16@gmail.com> wrote:

On Wed, Apr 7, 2021 at 7:12 PM Bharath Rupireddy
<bharath.rupireddyforpostgres@gmail.com> wrote:

On Wed, Apr 7, 2021 at 3:30 PM Amit Kapila <amit.kapila16@gmail.com> 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] - /messages/by-id/CAH2-Wz=gf6FXW-jPVRdeCZk0QjhduCqH_XD3QbES9wPmhircuA@mail.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.

On Thu, Apr 8, 2021 at 11:22 AM Amit Kapila <amit.kapila16@gmail.com> 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

On Thu, Apr 8, 2021 at 12:37 PM Bharath Rupireddy
<bharath.rupireddyforpostgres@gmail.com> wrote:

On Thu, Apr 8, 2021 at 11:22 AM Amit Kapila <amit.kapila16@gmail.com> 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.