public inbox for [email protected]
help / color / mirror / Atom feedFrom: Maxim Orlov <[email protected]>
To: Postgres hackers <[email protected]>
Subject: old_snapshot_threshold bottleneck on replica
Date: Mon, 23 Jan 2023 17:40:15 +0300
Message-ID: <CACG=ezYV+EvO135fLRdVn-ZusfVsTY6cH1OZqWtezuEYH6ciQA@mail.gmail.com> (raw)
Hi!
One of our customers stumble onto a significant performance degradation
while running multiple OLAP-like queries on a replica.
After some investigation, it became clear that the problem is in accessing
old_snapshot_threshold parameter.
Accessing old_snapshot_threshold parameter is guarded by mutex_threshold.
This is not a problem on primary
server, since we rarely call GetOldSnapshotThresholdTimestamp:
5028 void
5029 TestForOldSnapshot_impl(Snapshot snapshot, Relation relation)
5030 {
5031 ····if (RelationAllowsEarlyPruning(relation)
5032 ········&& (snapshot)->whenTaken < GetOldSnapshotThresholdTimestamp())
5033 ········ereport(ERROR,
5034 ················(errcode(ERRCODE_SNAPSHOT_TOO_OLD),
5035 ················ errmsg("snapshot too old")));
But in case of a replica, we have to call GetOldSnapshotThresholdTimestamp
much often. So, this become a
bottleneck. The customer solve this issue by setting old_snapshot_threshold
to 0. But, I think, we can
do something about it.
Some more investigation:
-- On primary --
$ ./bin/psql postgres -c "create database benchmark"
CREATE DATABASE
$ ./bin/pgbench -i -Uorlov -s300 benchmark
dropping old tables...
NOTICE: table "pgbench_accounts" does not exist, skipping
...
creating tables...
generating data (client-side)...
30000000 of 30000000 tuples (100%) done (elapsed 142.37 s, remaining 0.00 s)
vacuuming...
creating primary keys...
done in 177.67 s (drop tables 0.00 s, create tables 0.01 s, client-side
generate 144.45 s, vacuum 0.59 s, primary keys 32.61 s).
-- On secondary --
$ touch 1.sql
$ vim 1.sql
$ cat 1.sql
\set bid random(1, 300)
BEGIN;
SELECT sum(aid) FROM pgbench_accounts where bid = :bid GROUP BY bid;
END;
$ ./bin/pgbench -f 1.sql -p5433 -Uorlov -j10 -c100 -T720 -P1 -n benchmark
pgbench (16devel)
progress: 1.0 s, 0.0 tps, lat 0.000 ms stddev 0.000, 0 failed
...
progress: 20.0 s, 0.0 tps, lat 0.000 ms stddev 0.000, 0 failed
$ perf record -F 99 -a -g --call-graph=dwarf sleep 5
$ perf script --header --fields comm,pid,tid,time,event,ip,sym,dso > file
$ grep s_lock file | wc -l
3486
My proposal is to use atomic for threshold_timestamp and threshold_xid. PFA
0001 patch.
With patch 0001 we got:
$ grep s_lock file2 | wc -l
8
Maybe, we shall go farther and remove mutex_threshold here? This will lead
to inconsistency of
threshold_timestamp and threshold_xid, but is this really a problem?
Thoughts?
--
Best regards,
Maxim Orlov.
Attachments:
[application/octet-stream] 0001-PGPRO-7624-use-atomic-old_snapshot_threshold.patch (4.4K, ../CACG=ezYV+EvO135fLRdVn-ZusfVsTY6cH1OZqWtezuEYH6ciQA@mail.gmail.com/3-0001-PGPRO-7624-use-atomic-old_snapshot_threshold.patch)
download | inline diff:
From 98fa70754a8c68cdfb20f27bd7f6e6e3e5f8ed92 Mon Sep 17 00:00:00 2001
From: Maxim Orlov <[email protected]>
Date: Wed, 11 Jan 2023 18:07:10 +0300
Subject: [PATCH] [PGPRO-7624] use atomic old_snapshot_threshold
Using spinlock to access old_snapshot_threshold lead to the bottleneck on
replica, since GetOldSnapshotThresholdTimestamp is called too often. So, switch
to an atomic values.
tags: commitfest_hotfix
---
src/backend/utils/time/snapmgr.c | 31 +++++++++++++++++--------------
src/include/utils/old_snapshot.h | 5 +++--
2 files changed, 20 insertions(+), 16 deletions(-)
diff --git a/src/backend/utils/time/snapmgr.c b/src/backend/utils/time/snapmgr.c
index 7d11ae34781..0902e29f224 100644
--- a/src/backend/utils/time/snapmgr.c
+++ b/src/backend/utils/time/snapmgr.c
@@ -230,8 +230,8 @@ SnapMgrInit(void)
oldSnapshotControl->latest_xmin = InvalidTransactionId;
oldSnapshotControl->next_map_update = 0;
SpinLockInit(&oldSnapshotControl->mutex_threshold);
- oldSnapshotControl->threshold_timestamp = 0;
- oldSnapshotControl->threshold_xid = InvalidTransactionId;
+ pg_atomic_init_u64(&oldSnapshotControl->threshold_timestamp, 0);
+ pg_atomic_init_u32(&oldSnapshotControl->threshold_xid, InvalidTransactionId);
oldSnapshotControl->head_offset = 0;
oldSnapshotControl->head_timestamp = 0;
oldSnapshotControl->count_used = 0;
@@ -1706,9 +1706,7 @@ GetOldSnapshotThresholdTimestamp(void)
{
TimestampTz threshold_timestamp;
- SpinLockAcquire(&oldSnapshotControl->mutex_threshold);
- threshold_timestamp = oldSnapshotControl->threshold_timestamp;
- SpinLockRelease(&oldSnapshotControl->mutex_threshold);
+ threshold_timestamp = pg_atomic_read_u64(&oldSnapshotControl->threshold_timestamp);
return threshold_timestamp;
}
@@ -1716,11 +1714,18 @@ GetOldSnapshotThresholdTimestamp(void)
void
SetOldSnapshotThresholdTimestamp(TimestampTz ts, TransactionId xlimit)
{
+ TimestampTz threshold_timestamp;
+ TransactionId threshold_xid;
+
SpinLockAcquire(&oldSnapshotControl->mutex_threshold);
- Assert(oldSnapshotControl->threshold_timestamp <= ts);
- Assert(TransactionIdPrecedesOrEquals(oldSnapshotControl->threshold_xid, xlimit));
- oldSnapshotControl->threshold_timestamp = ts;
- oldSnapshotControl->threshold_xid = xlimit;
+ threshold_timestamp = pg_atomic_read_u64(&oldSnapshotControl->threshold_timestamp);
+ threshold_xid = pg_atomic_read_u32(&oldSnapshotControl->threshold_xid);
+
+ Assert(threshold_timestamp <= ts);
+ Assert(TransactionIdPrecedesOrEquals(threshold_xid, xlimit));
+
+ pg_atomic_write_u64(&oldSnapshotControl->threshold_timestamp, ts);
+ pg_atomic_write_u32(&oldSnapshotControl->threshold_xid, xlimit);
SpinLockRelease(&oldSnapshotControl->mutex_threshold);
}
@@ -1739,9 +1744,7 @@ SnapshotTooOldMagicForTest(void)
ts -= 5 * USECS_PER_SEC;
- SpinLockAcquire(&oldSnapshotControl->mutex_threshold);
- oldSnapshotControl->threshold_timestamp = ts;
- SpinLockRelease(&oldSnapshotControl->mutex_threshold);
+ pg_atomic_write_u64(&oldSnapshotControl->threshold_timestamp, ts);
}
/*
@@ -1846,8 +1849,8 @@ TransactionIdLimitedForOldSnapshots(TransactionId recentXmin,
/* Check for fast exit without LW locking. */
SpinLockAcquire(&oldSnapshotControl->mutex_threshold);
- threshold_timestamp = oldSnapshotControl->threshold_timestamp;
- threshold_xid = oldSnapshotControl->threshold_xid;
+ threshold_timestamp = pg_atomic_read_u64(&oldSnapshotControl->threshold_timestamp);
+ threshold_xid = pg_atomic_read_u32(&oldSnapshotControl->threshold_xid);
SpinLockRelease(&oldSnapshotControl->mutex_threshold);
if (ts == threshold_timestamp)
diff --git a/src/include/utils/old_snapshot.h b/src/include/utils/old_snapshot.h
index f1978a28e1c..3dd31d721c7 100644
--- a/src/include/utils/old_snapshot.h
+++ b/src/include/utils/old_snapshot.h
@@ -16,6 +16,7 @@
#define OLD_SNAPSHOT_H
#include "datatype/timestamp.h"
+#include "port/atomics.h"
#include "storage/s_lock.h"
/*
@@ -33,8 +34,8 @@ typedef struct OldSnapshotControlData
TransactionId latest_xmin; /* latest snapshot xmin */
TimestampTz next_map_update; /* latest snapshot valid up to */
slock_t mutex_threshold; /* protect threshold fields */
- TimestampTz threshold_timestamp; /* earlier snapshot is old */
- TransactionId threshold_xid; /* earlier xid may be gone */
+ pg_atomic_uint64 threshold_timestamp; /* earlier snapshot is old */
+ pg_atomic_uint32 threshold_xid; /* earlier xid may be gone */
/*
* Keep one xid per minute for old snapshot error handling.
--
2.34.1
view thread (2+ messages)
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected]
Subject: Re: old_snapshot_threshold bottleneck on replica
In-Reply-To: <CACG=ezYV+EvO135fLRdVn-ZusfVsTY6cH1OZqWtezuEYH6ciQA@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox