Avoid call MaintainOldSnapshotTimeMapping, if old_snapshot_threshold is disabled.
Hi,
While another long thread discusses the situation of old_snapshot_threshold,
I believe we can improve procarray.c by avoiding calling
MaintainOldSnapshotTimeMapping (src/backend/utils/time/snapmgr.c).
There's a very explicit comment there, which says (line 1866):
"Never call this function when old snapshot checking is disabled."
Well, assert should never be used to validate a condition that certainly
occurs at runtime.
Since old_snapshot_threshold is -1, it is disabled, so
MaintainOldSnapshotTimeMapping doesn't need to be run, right?
regards,
Ranier Vilela
Attachments:
avoid_call_if_old_snapshot_threshold_is_disabled.patchapplication/octet-stream; name=avoid_call_if_old_snapshot_threshold_is_disabled.patchDownload
diff --git a/src/backend/utils/time/snapmgr.c b/src/backend/utils/time/snapmgr.c
index 2968c7f7b7..46bc1a161c 100644
--- a/src/backend/utils/time/snapmgr.c
+++ b/src/backend/utils/time/snapmgr.c
@@ -1690,7 +1690,8 @@ SnapshotTooOldMagicForTest(void)
{
TimestampTz ts = GetSnapshotCurrentTimestamp();
- Assert(old_snapshot_threshold == 0);
+ if (old_snapshot_threshold != 0)
+ return;
ts -= 5 * USECS_PER_SEC;
@@ -1863,7 +1864,8 @@ MaintainOldSnapshotTimeMapping(TimestampTz whenTaken, TransactionId xmin)
bool map_update_required = false;
/* Never call this function when old snapshot checking is disabled. */
- Assert(old_snapshot_threshold >= 0);
+ if (old_snapshot_threshold < 0)
+ return;
ts = AlignTimestampToMinuteBoundary(whenTaken);
Hi,
On 2021-06-17 21:27:15 -0300, Ranier Vilela wrote:
While another long thread discusses the situation of old_snapshot_threshold,
I believe we can improve procarray.c by avoiding calling
MaintainOldSnapshotTimeMapping (src/backend/utils/time/snapmgr.c).There's a very explicit comment there, which says (line 1866):
"Never call this function when old snapshot checking is disabled."Well, assert should never be used to validate a condition that certainly
occurs at runtime.
I don't see how it can happen at runtime currently?
Since old_snapshot_threshold is -1, it is disabled, so
MaintainOldSnapshotTimeMapping doesn't need to be run, right?
It *isn't* run, the caller checks OldSnapshotThresholdActive() first.
Greetings,
Andres Freund
Em qui., 17 de jun. de 2021 às 22:08, Andres Freund <andres@anarazel.de>
escreveu:
Hi,
On 2021-06-17 21:27:15 -0300, Ranier Vilela wrote:
While another long thread discusses the situation of
old_snapshot_threshold,
I believe we can improve procarray.c by avoiding calling
MaintainOldSnapshotTimeMapping (src/backend/utils/time/snapmgr.c).There's a very explicit comment there, which says (line 1866):
"Never call this function when old snapshot checking is disabled."Well, assert should never be used to validate a condition that certainly
occurs at runtime.I don't see how it can happen at runtime currently?
Since old_snapshot_threshold is -1, it is disabled, so
MaintainOldSnapshotTimeMapping doesn't need to be run, right?It *isn't* run, the caller checks OldSnapshotThresholdActive() first.
True. My mistake.
I didn't check GetSnapshotDataInitOldSnapshot correctly.
regards,
Ranier Vilela