Avoid unnecessary ReplicationSlotControl lwlock acquistion
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
Attachments:
avoid_replicationslotcontrol.patchapplication/octet-stream; name=avoid_replicationslotcontrol.patchDownload
diff --git a/src/backend/replication/slot.c b/src/backend/replication/slot.c
index 42c78eabd4..45122b9ec9 100644
--- a/src/backend/replication/slot.c
+++ b/src/backend/replication/slot.c
@@ -535,6 +535,9 @@ ReplicationSlotCleanup(void)
Assert(MyReplicationSlot == NULL);
+ if (max_replication_slots <= 0)
+ return;
+
restart:
LWLockAcquire(ReplicationSlotControlLock, LW_SHARED);
for (i = 0; i < max_replication_slots; i++)
@@ -1135,6 +1138,9 @@ InvalidateObsoleteReplicationSlots(XLogSegNo oldestSegno)
{
XLogRecPtr oldestLSN;
+ if (max_replication_slots <= 0)
+ return;
+
XLogSegNoOffsetToRecPtr(oldestSegno, 0, wal_segment_size, oldestLSN);
restart:
@@ -1251,6 +1257,9 @@ CheckPointReplicationSlots(void)
{
int i;
+ if (max_replication_slots <= 0)
+ return;
+
elog(DEBUG1, "performing replication slot checkpoint");
/*
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