Avoid unnecessary ReplicationSlotControl lwlock acquistion
Hackorum builds and tests every patch posted to the lists, not only commitfest submissions. This is Hackorum's own CI rather than the PostgreSQL project's, and it is still under testing - please report anything that looks wrong.
Every patchset is also pushed to a branch of our PostgreSQL fork, so you can check out the same tree CI built. Without a PostgreSQL checkout:
git clone --branch t42838_1 https://github.com/hackorum-dev/postgres.gitIn a checkout you already have, add the fork once:
git remote add hackorum https://github.com/hackorum-dev/postgres.gitthen, for this patchset and every later one:
git fetch hackorum t42838_1 && git checkout t42838_1Patchset v1 (message #1) is on t42838_1
Hi all,
While testing with DTrace, I realized we acquire
ReplicationSlotControl lwlock at some places even when
max_replication_slots is set to 0. For instance, we call
ReplicationSlotCleanup() within PostgresMian() when an error happens
and acquire ReplicationSlotControl lwlock.
The attached patch fixes some functions so that we quickly return if
max_replication_slots is set to 0.
Regards,
--
Masahiko Sawada http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Hi,
On August 24, 2020 7:38:39 PM PDT, Masahiko Sawada <masahiko.sawada@2ndquadrant.com> wrote:
Hi all,
While testing with DTrace, I realized we acquire
ReplicationSlotControl lwlock at some places even when
max_replication_slots is set to 0. For instance, we call
ReplicationSlotCleanup() within PostgresMian() when an error happens
and acquire ReplicationSlotControl lwlock.The attached patch fixes some functions so that we quickly return if
max_replication_slots is set to 0.
Why is it worth doing so?
Regards,
Andres
--
Sent from my Android device with K-9 Mail. Please excuse my brevity.
On Tue, 25 Aug 2020 at 11:42, Andres Freund <andres@anarazel.de> wrote:
Hi,
On August 24, 2020 7:38:39 PM PDT, Masahiko Sawada <masahiko.sawada@2ndquadrant.com> wrote:
Hi all,
While testing with DTrace, I realized we acquire
ReplicationSlotControl lwlock at some places even when
max_replication_slots is set to 0. For instance, we call
ReplicationSlotCleanup() within PostgresMian() when an error happens
and acquire ReplicationSlotControl lwlock.The attached patch fixes some functions so that we quickly return if
max_replication_slots is set to 0.Why is it worth doing so?
I think we can avoid unnecessary overhead caused by acquiring and
releasing that lwlock itself. The functions modified by this patch are
called during error cleanup or checkpoints. For the former case,
since it’s not a commit path the benefit might not be large on common
workload but it might help to reduce the latency on a workload whose
abort rate is relatively high. Also looking at other functions in
slot.c, other functions also do so. I think these are also for
preventing unnecessary overhead.
Regards,
--
Masahiko Sawada http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Hi,
On 2020-08-25 12:00:47 +0900, Masahiko Sawada wrote:
On Tue, 25 Aug 2020 at 11:42, Andres Freund <andres@anarazel.de> wrote:
Hi,
On August 24, 2020 7:38:39 PM PDT, Masahiko Sawada <masahiko.sawada@2ndquadrant.com> wrote:
Hi all,
While testing with DTrace, I realized we acquire
ReplicationSlotControl lwlock at some places even when
max_replication_slots is set to 0. For instance, we call
ReplicationSlotCleanup() within PostgresMian() when an error happens
and acquire ReplicationSlotControl lwlock.The attached patch fixes some functions so that we quickly return if
max_replication_slots is set to 0.Why is it worth doing so?
I think we can avoid unnecessary overhead caused by acquiring and
releasing that lwlock itself. The functions modified by this patch are
called during error cleanup or checkpoints. For the former case,
since it’s not a commit path the benefit might not be large on common
workload but it might help to reduce the latency on a workload whose
abort rate is relatively high. Also looking at other functions in
slot.c, other functions also do so. I think these are also for
preventing unnecessary overhead.
I don't see how these could matter. The error checking path isn't
reached if no slot is acquired. One uncontended lwlock acquisition isn't
measurable compared to the cost of a checkpoint.
Greetings,
Andres Freund
On Tue, 25 Aug 2020 at 13:13, Andres Freund <andres@anarazel.de> wrote:
Hi,
On 2020-08-25 12:00:47 +0900, Masahiko Sawada wrote:
On Tue, 25 Aug 2020 at 11:42, Andres Freund <andres@anarazel.de> wrote:
Hi,
On August 24, 2020 7:38:39 PM PDT, Masahiko Sawada <masahiko.sawada@2ndquadrant.com> wrote:
Hi all,
While testing with DTrace, I realized we acquire
ReplicationSlotControl lwlock at some places even when
max_replication_slots is set to 0. For instance, we call
ReplicationSlotCleanup() within PostgresMian() when an error happens
and acquire ReplicationSlotControl lwlock.The attached patch fixes some functions so that we quickly return if
max_replication_slots is set to 0.Why is it worth doing so?
I think we can avoid unnecessary overhead caused by acquiring and
releasing that lwlock itself. The functions modified by this patch are
called during error cleanup or checkpoints. For the former case,
since it’s not a commit path the benefit might not be large on common
workload but it might help to reduce the latency on a workload whose
abort rate is relatively high. Also looking at other functions in
slot.c, other functions also do so. I think these are also for
preventing unnecessary overhead.I don't see how these could matter. The error checking path isn't
reached if no slot is acquired.
I think we always call ReplicationSlotCleanup() which acquires the
lwlock during error cleanup even if no slot is acquired.
One uncontended lwlock acquisition isn't
measurable compared to the cost of a checkpoint.
That's true.
Regards,
--
Masahiko Sawada http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services