From 4697923f41a6f492c816fa9b6ad2708cc90c0c41 Mon Sep 17 00:00:00 2001
From: John Sully <john@csquare.ca>
Date: Mon, 7 Mar 2022 16:00:48 -0500
Subject: [PATCH] Optimize async command snapshot creation, and make slip
 configurable

---
 src/config.cpp     | 1 +
 src/networking.cpp | 4 ++--
 src/server.cpp     | 2 +-
 src/server.h       | 4 ++--
 src/snapshot.cpp   | 2 +-
 5 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/src/config.cpp b/src/config.cpp
index 5d416fcde..08da538be 100644
--- a/src/config.cpp
+++ b/src/config.cpp
@@ -2826,6 +2826,7 @@ standardConfig configs[] = {
     createLongLongConfig("stream-node-max-entries", NULL, MODIFIABLE_CONFIG, 0, LLONG_MAX, g_pserver->stream_node_max_entries, 100, INTEGER_CONFIG, NULL, NULL),
     createLongLongConfig("repl-backlog-size", NULL, MODIFIABLE_CONFIG, 1, LLONG_MAX, g_pserver->repl_backlog_size, 1024*1024, MEMORY_CONFIG, NULL, updateReplBacklogSize), /* Default: 1mb */
     createLongLongConfig("repl-backlog-disk-reserve", NULL, IMMUTABLE_CONFIG, 0, LLONG_MAX, cserver.repl_backlog_disk_size, 0, MEMORY_CONFIG, NULL, NULL),
+    createLongLongConfig("max-snapshot-slip", NULL, MODIFIABLE_CONFIG, 0, 5000, g_pserver->snapshot_slip, 400, 0, NULL, NULL),
 
     /* Unsigned Long Long configs */
     createULongLongConfig("maxmemory", NULL, MODIFIABLE_CONFIG, 0, LLONG_MAX, g_pserver->maxmemory, 0, MEMORY_CONFIG, NULL, updateMaxmemory),
diff --git a/src/networking.cpp b/src/networking.cpp
index 2b8ac08c9..80eded73d 100644
--- a/src/networking.cpp
+++ b/src/networking.cpp
@@ -208,7 +208,7 @@ client *createClient(connection *conn, int iel) {
     c->paused_list_node = NULL;
     c->client_tracking_redirection = 0;
     c->casyncOpsPending = 0;
-    c->mvccCheckpoint = getMvccTstamp();
+    c->mvccCheckpoint = 0;
     c->master_error = 0;
     memset(c->uuid, 0, UUID_BINARY_LEN);
 
@@ -2754,7 +2754,7 @@ void readQueryFromClient(connection *conn) {
             // Frequent writers aren't good candidates for this optimization, they cause us to renew the snapshot too often
             //  so we exclude them unless the snapshot we need already exists
             bool fSnapshotExists = c->db->mvccLastSnapshot >= c->mvccCheckpoint;
-            bool fWriteTooRecent = (((getMvccTstamp() - c->mvccCheckpoint) >> MVCC_MS_SHIFT) < redisDbPersistentDataSnapshot::msStaleThreshold/2);
+            bool fWriteTooRecent = (((getMvccTstamp() - c->mvccCheckpoint) >> MVCC_MS_SHIFT) < static_cast<uint64_t>(g_pserver->snapshot_slip)/2);
 
             // The check below avoids running async commands if this is a frequent writer unless a snapshot is already there to service it
             if (!fWriteTooRecent || fSnapshotExists) {
diff --git a/src/server.cpp b/src/server.cpp
index 977191e7c..b357507e7 100644
--- a/src/server.cpp
+++ b/src/server.cpp
@@ -2776,7 +2776,7 @@ void beforeSleep(struct aeEventLoop *eventLoop) {
 
     /* end any snapshots created by fast async commands */
     for (int idb = 0; idb < cserver.dbnum; ++idb) {
-        if (serverTL->rgdbSnapshot[idb] != nullptr) {
+        if (serverTL->rgdbSnapshot[idb] != nullptr && serverTL->rgdbSnapshot[idb]->FStale()) {
             g_pserver->db[idb]->endSnapshot(serverTL->rgdbSnapshot[idb]);
             serverTL->rgdbSnapshot[idb] = nullptr;
         }
diff --git a/src/server.h b/src/server.h
index 743011f0d..d6959a131 100644
--- a/src/server.h
+++ b/src/server.h
@@ -1286,8 +1286,6 @@ public:
     // These need to be fixed
     using redisDbPersistentData::size;
     using redisDbPersistentData::expireSize;
-
-    static const uint64_t msStaleThreshold = 500;
 };
 
 /* Redis database representation. There are multiple databases identified
@@ -2605,6 +2603,8 @@ struct redisServer {
     IStorageFactory *m_pstorageFactory = nullptr;
     int storage_flush_period;   // The time between flushes in the CRON job
 
+    long long snapshot_slip = 500;   // The amount of time in milliseconds we let a snapshot be behind the current database
+
     /* TLS Configuration */
     int tls_cluster;
     int tls_replication;
diff --git a/src/snapshot.cpp b/src/snapshot.cpp
index a13f5d9f5..408b78aad 100644
--- a/src/snapshot.cpp
+++ b/src/snapshot.cpp
@@ -654,7 +654,7 @@ int redisDbPersistentDataSnapshot::snapshot_depth() const
 
 bool redisDbPersistentDataSnapshot::FStale() const
 {
-    return ((getMvccTstamp() - m_mvccCheckpoint) >> MVCC_MS_SHIFT) >= redisDbPersistentDataSnapshot::msStaleThreshold;
+    return ((getMvccTstamp() - m_mvccCheckpoint) >> MVCC_MS_SHIFT) >= static_cast<uint64_t>(g_pserver->snapshot_slip);
 }
 
 void dictGCAsyncFree(dictAsyncRehashCtl *async) {