From ea4941a3e7f7e0691ca6f13a56ced56b8407341c Mon Sep 17 00:00:00 2001 From: John Sully Date: Fri, 20 May 2022 01:47:42 +0000 Subject: [PATCH] Fix crash in expire when a snapshot is in flight. Caused by a perf optimization getting the expire map out of sync with the val --- src/db.cpp | 5 +++++ src/networking.cpp | 5 +++-- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/src/db.cpp b/src/db.cpp index 0ad3df99f..d9e37816d 100644 --- a/src/db.cpp +++ b/src/db.cpp @@ -2669,6 +2669,11 @@ void redisDbPersistentData::prepOverwriteForSnapshot(char *key) auto itr = m_pdbSnapshot->find_cached_threadsafe(key); if (itr.key() != nullptr) { + if (itr.val()->FExpires()) { + // Note: I'm sure we could handle this, but its too risky at the moment. + // There are known bugs doing this with expires + return; + } sds keyNew = sdsdupshared(itr.key()); if (dictAdd(m_pdictTombstone, keyNew, (void*)dictHashKey(m_pdict, key)) != DICT_OK) sdsfree(keyNew); diff --git a/src/networking.cpp b/src/networking.cpp index 5b1fe5894..e22a2d8a1 100644 --- a/src/networking.cpp +++ b/src/networking.cpp @@ -2743,9 +2743,10 @@ void readQueryFromClient(connection *conn) { parseClientCommandBuffer(c); if (g_pserver->enable_async_commands && !serverTL->disable_async_commands && listLength(g_pserver->monitors) == 0 && (aeLockContention() || serverTL->rgdbSnapshot[c->db->id] || g_fTestMode)) { // 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 + // so we exclude them unless the snapshot we need already exists. + // Note: In test mode we want to create snapshots as often as possibl to excercise them - we don't care about perf bool fSnapshotExists = c->db->mvccLastSnapshot >= c->mvccCheckpoint; - bool fWriteTooRecent = (((getMvccTstamp() - c->mvccCheckpoint) >> MVCC_MS_SHIFT) < static_cast(g_pserver->snapshot_slip)/2); + bool fWriteTooRecent = !g_fTestMode && (((getMvccTstamp() - c->mvccCheckpoint) >> MVCC_MS_SHIFT) < static_cast(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) {