From e69bfd5da64cefee0eb60155e99f0db3a0ed6b77 Mon Sep 17 00:00:00 2001 From: Malavan Sotheeswaran Date: Fri, 5 May 2023 12:22:29 -0700 Subject: [PATCH 01/65] add hash prefixing to rocksdb --- src/storage/rocksdb.cpp | 40 +++++++++++++++++++++++++++------------- src/storage/rocksdb.h | 2 +- 2 files changed, 28 insertions(+), 14 deletions(-) diff --git a/src/storage/rocksdb.cpp b/src/storage/rocksdb.cpp index e592c74c5..9e7f82686 100644 --- a/src/storage/rocksdb.cpp +++ b/src/storage/rocksdb.cpp @@ -3,6 +3,8 @@ #include #include #include +#include "../server.h" +#include "../cluster.h" #include "rocksdbfactor_internal.h" static const char keyprefix[] = INTERNAL_KEY_PREFIX; @@ -10,6 +12,13 @@ static const char keyprefix[] = INTERNAL_KEY_PREFIX; rocksdb::Options DefaultRocksDBOptions(); extern "C" pid_t gettid(); +std::string prefixKey(const char *key, size_t cchKey) +{ + unsigned int hash = keyHashSlot(key, cchKey); + char *hash_char = (char *)&hash; + return std::string(hash_char + (sizeof(unsigned int) - 2), 2) + std::string(key, cchKey); +} + bool FInternalKey(const char *key, size_t cch) { if (cch >= sizeof(INTERNAL_KEY_PREFIX)) @@ -32,10 +41,11 @@ void RocksDBStorageProvider::insert(const char *key, size_t cchKey, void *data, { rocksdb::Status status; std::unique_lock l(m_lock); + std::string prefixed_key = prefixKey(key, cchKey); if (m_spbatch != nullptr) - status = m_spbatch->Put(m_spcolfamily.get(), rocksdb::Slice(key, cchKey), rocksdb::Slice((const char*)data, cb)); + status = m_spbatch->Put(m_spcolfamily.get(), rocksdb::Slice(prefixed_key), rocksdb::Slice((const char*)data, cb)); else - status = m_spdb->Put(WriteOptions(), m_spcolfamily.get(), rocksdb::Slice(key, cchKey), rocksdb::Slice((const char*)data, cb)); + status = m_spdb->Put(WriteOptions(), m_spcolfamily.get(), rocksdb::Slice(prefixed_key), rocksdb::Slice((const char*)data, cb)); if (!status.ok()) throw status.ToString(); @@ -59,7 +69,8 @@ void RocksDBStorageProvider::bulkInsert(char **rgkeys, size_t *rgcbkeys, char ** // Insert rows into the SST file, note that inserted keys must be // strictly increasing (based on options.comparator) for (size_t ielem = 0; ielem < celem; ++ielem) { - s = sst_file_writer.Put(rocksdb::Slice(rgkeys[ielem], rgcbkeys[ielem]), rocksdb::Slice(rgvals[ielem], rgcbvals[ielem])); + std::string prefixed_key = prefixKey(rgkeys[ielem], rgcbkeys[ielem]); + s = sst_file_writer.Put(rocksdb::Slice(prefixed_key), rocksdb::Slice(rgvals[ielem], rgcbvals[ielem])); if (!s.ok()) { unlink(file_path.c_str()); goto LFallback; @@ -87,7 +98,8 @@ void RocksDBStorageProvider::bulkInsert(char **rgkeys, size_t *rgcbkeys, char ** LFallback: auto spbatch = std::make_unique(); for (size_t ielem = 0; ielem < celem; ++ielem) { - spbatch->Put(m_spcolfamily.get(), rocksdb::Slice(rgkeys[ielem], rgcbkeys[ielem]), rocksdb::Slice(rgvals[ielem], rgcbvals[ielem])); + std::string prefixed_key = prefixKey(rgkeys[ielem], rgcbkeys[ielem]); + spbatch->Put(m_spcolfamily.get(), rocksdb::Slice(prefixed_key), rocksdb::Slice(rgvals[ielem], rgcbvals[ielem])); } m_spdb->Write(WriteOptions(), spbatch.get()); } @@ -100,15 +112,16 @@ bool RocksDBStorageProvider::erase(const char *key, size_t cchKey) { rocksdb::Status status; std::unique_lock l(m_lock); - if (!FKeyExists(key, cchKey)) + std::string prefixed_key = prefixKey(key, cchKey); + if (!FKeyExists(prefixed_key)) return false; if (m_spbatch != nullptr) { - status = m_spbatch->Delete(m_spcolfamily.get(), rocksdb::Slice(key, cchKey)); + status = m_spbatch->Delete(m_spcolfamily.get(), rocksdb::Slice(prefixed_key)); } else { - status = m_spdb->Delete(WriteOptions(), m_spcolfamily.get(), rocksdb::Slice(key, cchKey)); + status = m_spdb->Delete(WriteOptions(), m_spcolfamily.get(), rocksdb::Slice(prefixed_key)); } if (status.ok()) --m_count; @@ -118,7 +131,8 @@ bool RocksDBStorageProvider::erase(const char *key, size_t cchKey) void RocksDBStorageProvider::retrieve(const char *key, size_t cchKey, callbackSingle fn) const { rocksdb::PinnableSlice slice; - auto status = m_spdb->Get(ReadOptions(), m_spcolfamily.get(), rocksdb::Slice(key, cchKey), &slice); + std::string prefixed_key = prefixKey(key, cchKey); + auto status = m_spdb->Get(ReadOptions(), m_spcolfamily.get(), rocksdb::Slice(prefixed_key), &slice); if (status.ok()) fn(key, cchKey, slice.data(), slice.size()); } @@ -151,10 +165,10 @@ bool RocksDBStorageProvider::enumerate(callback fn) const std::unique_ptr it = std::unique_ptr(m_spdb->NewIterator(ReadOptions(), m_spcolfamily.get())); size_t count = 0; for (it->SeekToFirst(); it->Valid(); it->Next()) { - if (FInternalKey(it->key().data(), it->key().size())) + if (FInternalKey(it->key().data()+2, it->key().size()-2)) continue; ++count; - bool fContinue = fn(it->key().data(), it->key().size(), it->value().data(), it->value().size()); + bool fContinue = fn(it->key().data()+2, it->key().size()-2, it->value().data(), it->value().size()); if (!fContinue) break; } @@ -227,10 +241,10 @@ void RocksDBStorageProvider::flush() m_spdb->SyncWAL(); } -bool RocksDBStorageProvider::FKeyExists(const char *key, size_t cch) const +bool RocksDBStorageProvider::FKeyExists(std::string& key) const { rocksdb::PinnableSlice slice; if (m_spbatch) - return m_spbatch->GetFromBatchAndDB(m_spdb.get(), ReadOptions(), m_spcolfamily.get(), rocksdb::Slice(key, cch), &slice).ok(); - return m_spdb->Get(ReadOptions(), m_spcolfamily.get(), rocksdb::Slice(key, cch), &slice).ok(); + return m_spbatch->GetFromBatchAndDB(m_spdb.get(), ReadOptions(), m_spcolfamily.get(), rocksdb::Slice(key), &slice).ok(); + return m_spdb->Get(ReadOptions(), m_spcolfamily.get(), rocksdb::Slice(key), &slice).ok(); } \ No newline at end of file diff --git a/src/storage/rocksdb.h b/src/storage/rocksdb.h index 10c54606c..7cb1df837 100644 --- a/src/storage/rocksdb.h +++ b/src/storage/rocksdb.h @@ -48,7 +48,7 @@ class RocksDBStorageProvider : public IStorage size_t count() const override; protected: - bool FKeyExists(const char *key, size_t cchKey) const; + bool FKeyExists(std::string&) const; const rocksdb::ReadOptions &ReadOptions() const { return m_readOptionsTemplate; } rocksdb::WriteOptions WriteOptions() const; From b9f049a25d97c4777a017f46aefe9ab82367577e Mon Sep 17 00:00:00 2001 From: Malavan Sotheeswaran Date: Fri, 5 May 2023 15:45:09 -0700 Subject: [PATCH 02/65] don't need to offset internal keys --- src/storage/rocksdb.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/storage/rocksdb.cpp b/src/storage/rocksdb.cpp index 9e7f82686..f1b8d2aab 100644 --- a/src/storage/rocksdb.cpp +++ b/src/storage/rocksdb.cpp @@ -165,7 +165,7 @@ bool RocksDBStorageProvider::enumerate(callback fn) const std::unique_ptr it = std::unique_ptr(m_spdb->NewIterator(ReadOptions(), m_spcolfamily.get())); size_t count = 0; for (it->SeekToFirst(); it->Valid(); it->Next()) { - if (FInternalKey(it->key().data()+2, it->key().size()-2)) + if (FInternalKey(it->key().data(), it->key().size())) continue; ++count; bool fContinue = fn(it->key().data()+2, it->key().size()-2, it->value().data(), it->value().size()); From 4bdd2ed5f2f734f1321f0b74fbfe8732c6e0b16a Mon Sep 17 00:00:00 2001 From: Malavan Sotheeswaran Date: Mon, 8 May 2023 21:53:56 -0700 Subject: [PATCH 03/65] remove hashslot prefix when loading from rocksdb --- src/storage/rocksdbfactory.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/storage/rocksdbfactory.cpp b/src/storage/rocksdbfactory.cpp index 398d6f68d..6baa8d050 100644 --- a/src/storage/rocksdbfactory.cpp +++ b/src/storage/rocksdbfactory.cpp @@ -186,7 +186,7 @@ IStorage *RocksDBStorageFactory::create(int db, key_load_iterator iter, void *pr printf("\tDatabase %d was not shutdown cleanly, recomputing metrics\n", db); fFirstRealKey = false; if (iter != nullptr) - iter(it->key().data(), it->key().size(), privdata); + iter(it->key().data()+2, it->key().size()-2, privdata); ++count; } } From ce248d7b666a1e0fecccb14bab407e541c33b959 Mon Sep 17 00:00:00 2001 From: Malavan Sotheeswaran Date: Tue, 9 May 2023 11:30:08 -0700 Subject: [PATCH 04/65] don't prefix internal keys --- src/storage/rocksdb.cpp | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/storage/rocksdb.cpp b/src/storage/rocksdb.cpp index f1b8d2aab..0f9eb7bf0 100644 --- a/src/storage/rocksdb.cpp +++ b/src/storage/rocksdb.cpp @@ -12,13 +12,6 @@ static const char keyprefix[] = INTERNAL_KEY_PREFIX; rocksdb::Options DefaultRocksDBOptions(); extern "C" pid_t gettid(); -std::string prefixKey(const char *key, size_t cchKey) -{ - unsigned int hash = keyHashSlot(key, cchKey); - char *hash_char = (char *)&hash; - return std::string(hash_char + (sizeof(unsigned int) - 2), 2) + std::string(key, cchKey); -} - bool FInternalKey(const char *key, size_t cch) { if (cch >= sizeof(INTERNAL_KEY_PREFIX)) @@ -29,6 +22,13 @@ bool FInternalKey(const char *key, size_t cch) return false; } +std::string prefixKey(const char *key, size_t cchKey) +{ + unsigned int hash = keyHashSlot(key, cchKey); + char *hash_char = (char *)&hash; + return FInternalKey(key, cchKey) ? std::string(key, cchKey) : std::string(hash_char + (sizeof(unsigned int) - 2), 2) + std::string(key, cchKey); +} + RocksDBStorageProvider::RocksDBStorageProvider(RocksDBStorageFactory *pfactory, std::shared_ptr &spdb, std::shared_ptr &spcolfam, const rocksdb::Snapshot *psnapshot, size_t count) : m_pfactory(pfactory), m_spdb(spdb), m_psnapshot(psnapshot), m_spcolfamily(spcolfam), m_count(count) { From 6e1d6146e08972728afb9b6a7bbc40848ee53853 Mon Sep 17 00:00:00 2001 From: Malavan Sotheeswaran Date: Tue, 9 May 2023 12:36:48 -0700 Subject: [PATCH 05/65] implement slot to keys with rocksdb --- src/IStorage.h | 1 + src/StorageCache.h | 1 + src/db.cpp | 122 ++++++++++++++++++++++++---------------- src/server.h | 2 + src/storage/rocksdb.cpp | 35 +++++++++++- src/storage/rocksdb.h | 1 + 6 files changed, 109 insertions(+), 53 deletions(-) diff --git a/src/IStorage.h b/src/IStorage.h index ad956beb6..c13e3e547 100644 --- a/src/IStorage.h +++ b/src/IStorage.h @@ -31,6 +31,7 @@ class IStorage virtual void retrieve(const char *key, size_t cchKey, callbackSingle fn) const = 0; virtual size_t clear() = 0; virtual bool enumerate(callback fn) const = 0; + virtual bool enumerate_hashslot(callback fn, int hashslot) const = 0; virtual size_t count() const = 0; virtual void bulkInsert(char **rgkeys, size_t *rgcbkeys, char **rgvals, size_t *rgcbvals, size_t celem) { diff --git a/src/StorageCache.h b/src/StorageCache.h index 3c38450fb..add1f0dad 100644 --- a/src/StorageCache.h +++ b/src/StorageCache.h @@ -49,6 +49,7 @@ class StorageCache void expand(uint64_t slots); bool enumerate(IStorage::callback fn) const { return m_spstorage->enumerate(fn); } + bool enumerate_hashslot(IStorage::callback fn, int hashslot) const { return m_spstorage->enumerate_hashslot(fn, hashslot); } void beginWriteBatch(); void endWriteBatch() { m_spstorage->endWriteBatch(); } diff --git a/src/db.cpp b/src/db.cpp index a84fd143f..ea75e82ed 100644 --- a/src/db.cpp +++ b/src/db.cpp @@ -2489,26 +2489,29 @@ void slotToKeyUpdateKey(sds key, int add) { void slotToKeyUpdateKeyCore(const char *key, size_t keylen, int add) { serverAssert(GlobalLocksAcquired()); - unsigned int hashslot = keyHashSlot(key,keylen); - unsigned char buf[64]; - unsigned char *indexed = buf; - g_pserver->cluster->slots_keys_count[hashslot] += add ? 1 : -1; - if (keylen+2 > 64) indexed = (unsigned char*)zmalloc(keylen+2, MALLOC_SHARED); - indexed[0] = (hashslot >> 8) & 0xff; - indexed[1] = hashslot & 0xff; - memcpy(indexed+2,key,keylen); - int fModified = false; - if (add) { - fModified = raxInsert(g_pserver->cluster->slots_to_keys,indexed,keylen+2,NULL,NULL); - } else { - fModified = raxRemove(g_pserver->cluster->slots_to_keys,indexed,keylen+2,NULL); + + if (g_pserver->m_pstorageFactory == nullptr) { + unsigned int hashslot = keyHashSlot(key,keylen); + unsigned char buf[64]; + unsigned char *indexed = buf; + + if (keylen+2 > 64) indexed = (unsigned char*)zmalloc(keylen+2, MALLOC_SHARED); + indexed[0] = (hashslot >> 8) & 0xff; + indexed[1] = hashslot & 0xff; + memcpy(indexed+2,key,keylen); + int fModified = false; + if (add) { + fModified = raxInsert(g_pserver->cluster->slots_to_keys,indexed,keylen+2,NULL,NULL); + } else { + fModified = raxRemove(g_pserver->cluster->slots_to_keys,indexed,keylen+2,NULL); + } + // This assert is disabled when a snapshot depth is >0 because prepOverwriteForSnapshot will add in a tombstone, + // this prevents ensure from adding the key to the dictionary which means the caller isn't aware we're already tracking + // the key. + serverAssert(fModified || g_pserver->db[0]->snapshot_depth() > 0); + if (indexed != buf) zfree(indexed); } - // This assert is disabled when a snapshot depth is >0 because prepOverwriteForSnapshot will add in a tombstone, - // this prevents ensure from adding the key to the dictionary which means the caller isn't aware we're already tracking - // the key. - serverAssert(fModified || g_pserver->db[0]->snapshot_depth() > 0); - if (indexed != buf) zfree(indexed); } void slotToKeyAdd(sds key) { @@ -2544,47 +2547,66 @@ void slotToKeyFlush(int async) { * New objects are returned to represent keys, it's up to the caller to * decrement the reference count to release the keys names. */ unsigned int getKeysInSlot(unsigned int hashslot, robj **keys, unsigned int count) { - raxIterator iter; - int j = 0; - unsigned char indexed[2]; + if (g_pserver->m_pstorageFactory != nullptr) { + int j = 0; + g_pserver->db[0]->getStorageCache()->enumerate_hashslot([&](const char *key, size_t cchKey, const void *, size_t )->bool { + keys[j++] = createStringObject(key, cchKey); + return --count; + }, hashslot); + } else { + raxIterator iter; + int j = 0; + unsigned char indexed[2]; - indexed[0] = (hashslot >> 8) & 0xff; - indexed[1] = hashslot & 0xff; - raxStart(&iter,g_pserver->cluster->slots_to_keys); - raxSeek(&iter,">=",indexed,2); - while(count-- && raxNext(&iter)) { - if (iter.key[0] != indexed[0] || iter.key[1] != indexed[1]) break; - keys[j++] = createStringObject((char*)iter.key+2,iter.key_len-2); + indexed[0] = (hashslot >> 8) & 0xff; + indexed[1] = hashslot & 0xff; + raxStart(&iter,g_pserver->cluster->slots_to_keys); + raxSeek(&iter,">=",indexed,2); + while(count-- && raxNext(&iter)) { + if (iter.key[0] != indexed[0] || iter.key[1] != indexed[1]) break; + keys[j++] = createStringObject((char*)iter.key+2,iter.key_len-2); + } + raxStop(&iter); + return j; } - raxStop(&iter); - return j; } /* Remove all the keys in the specified hash slot. * The number of removed items is returned. */ unsigned int delKeysInSlot(unsigned int hashslot) { serverAssert(GlobalLocksAcquired()); - - raxIterator iter; - int j = 0; - unsigned char indexed[2]; - - indexed[0] = (hashslot >> 8) & 0xff; - indexed[1] = hashslot & 0xff; - raxStart(&iter,g_pserver->cluster->slots_to_keys); - while(g_pserver->cluster->slots_keys_count[hashslot]) { - raxSeek(&iter,">=",indexed,2); - raxNext(&iter); - - auto count = g_pserver->cluster->slots_keys_count[hashslot]; - robj *key = createStringObject((char*)iter.key+2,iter.key_len-2); - dbDelete(g_pserver->db[0],key); - serverAssert(count > g_pserver->cluster->slots_keys_count[hashslot]); // we should have deleted something or we will be in an infinite loop - decrRefCount(key); - j++; + if (g_pserver->m_pstorageFactory != nullptr) { + int j = 0; + g_pserver->db[0]->getStorageCache()->enumerate_hashslot([&](const char *key, size_t cchKey, const void *, size_t )->bool { + robj *keyobj = createStringObject(key, cchKey); + dbDelete(g_pserver->db[0], keyobj); + decrRefCount(keyobj); + j++; + return true; + }, hashslot); + return j; + } else { + raxIterator iter; + int j = 0; + unsigned char indexed[2]; + + indexed[0] = (hashslot >> 8) & 0xff; + indexed[1] = hashslot & 0xff; + raxStart(&iter,g_pserver->cluster->slots_to_keys); + while(g_pserver->cluster->slots_keys_count[hashslot]) { + raxSeek(&iter,">=",indexed,2); + raxNext(&iter); + + auto count = g_pserver->cluster->slots_keys_count[hashslot]; + robj *key = createStringObject((char*)iter.key+2,iter.key_len-2); + dbDelete(g_pserver->db[0],key); + serverAssert(count > g_pserver->cluster->slots_keys_count[hashslot]); // we should have deleted something or we will be in an infinite loop + decrRefCount(key); + j++; + } + raxStop(&iter); + return j; } - raxStop(&iter); - return j; } unsigned int countKeysInSlot(unsigned int hashslot) { diff --git a/src/server.h b/src/server.h index f7d7b038b..9f5e3ac6c 100644 --- a/src/server.h +++ b/src/server.h @@ -1209,6 +1209,7 @@ class redisDbPersistentData bool FSnapshot() const { return m_spdbSnapshotHOLDER != nullptr; } std::unique_ptr CloneStorageCache() { return std::unique_ptr(m_spstorage->clone()); } + std::shared_ptr getStorageCache() { return m_spstorage; } void bulkDirectStorageInsert(char **rgKeys, size_t *rgcbKeys, char **rgVals, size_t *rgcbVals, size_t celem); dict_iter find_cached_threadsafe(const char *key) const; @@ -1370,6 +1371,7 @@ struct redisDb : public redisDbPersistentDataSnapshot using redisDbPersistentData::FRehashing; using redisDbPersistentData::FTrackingChanges; using redisDbPersistentData::CloneStorageCache; + using redisDbPersistentData::getStorageCache(); using redisDbPersistentData::bulkDirectStorageInsert; public: diff --git a/src/storage/rocksdb.cpp b/src/storage/rocksdb.cpp index 0f9eb7bf0..8e13802d9 100644 --- a/src/storage/rocksdb.cpp +++ b/src/storage/rocksdb.cpp @@ -22,11 +22,15 @@ bool FInternalKey(const char *key, size_t cch) return false; } +std::string getPrefix(int hashslot) +{ + char *hash_char = (char *)&hashslot; + return std::string(hash_char + (sizeof(unsigned int) - 2), 2); +} + std::string prefixKey(const char *key, size_t cchKey) { - unsigned int hash = keyHashSlot(key, cchKey); - char *hash_char = (char *)&hash; - return FInternalKey(key, cchKey) ? std::string(key, cchKey) : std::string(hash_char + (sizeof(unsigned int) - 2), 2) + std::string(key, cchKey); + return FInternalKey(key, cchKey) ? std::string(key, cchKey) : getPrefix(keyHashSlot(key, cchKey)) + std::string(key, cchKey); } RocksDBStorageProvider::RocksDBStorageProvider(RocksDBStorageFactory *pfactory, std::shared_ptr &spdb, std::shared_ptr &spcolfam, const rocksdb::Snapshot *psnapshot, size_t count) @@ -182,6 +186,31 @@ bool RocksDBStorageProvider::enumerate(callback fn) const return !it->Valid(); } +bool RocksDBStorageProvider::enumerate_hashslot(callback fn, int hashslot) const +{ + std::string prefix = getPrefix(hashslot); + std::unique_ptr it = std::unique_ptr(m_spdb->NewIterator(ReadOptions(), m_spcolfamily.get())); + size_t count = 0; + for (it->Seek(prefix.cstr()); it->Valid(); it->Next()) { + if (FInternalKey(it->key().data(), it->key().size())) + continue; + if (strncmp(it->key().data(),prefix.cstr(),2) != 0) + break; + ++count; + bool fContinue = fn(it->key().data()+2, it->key().size()-2, it->value().data(), it->value().size()); + if (!fContinue) + break; + } + if (!it->Valid() && count != m_count) + { + if (const_cast(this)->m_count != count) + printf("WARNING: rocksdb count mismatch"); + const_cast(this)->m_count = count; + } + assert(it->status().ok()); // Check for any errors found during the scan + return !it->Valid(); +} + const IStorage *RocksDBStorageProvider::clone() const { std::unique_lock l(m_lock); diff --git a/src/storage/rocksdb.h b/src/storage/rocksdb.h index 7cb1df837..14a8df8fe 100644 --- a/src/storage/rocksdb.h +++ b/src/storage/rocksdb.h @@ -32,6 +32,7 @@ class RocksDBStorageProvider : public IStorage virtual void retrieve(const char *key, size_t cchKey, callbackSingle fn) const override; virtual size_t clear() override; virtual bool enumerate(callback fn) const override; + virtual bool enumerate_hashslot(callback fn, int hashslot) const override; virtual const IStorage *clone() const override; From 880ca46788af102e7aad3e3102bc8735d3eb4062 Mon Sep 17 00:00:00 2001 From: Malavan Sotheeswaran Date: Tue, 9 May 2023 13:49:19 -0700 Subject: [PATCH 06/65] fix compile --- src/db.cpp | 2 +- src/server.h | 2 +- src/storage/rocksdb.cpp | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/db.cpp b/src/db.cpp index ea75e82ed..0d64edadb 100644 --- a/src/db.cpp +++ b/src/db.cpp @@ -2489,10 +2489,10 @@ void slotToKeyUpdateKey(sds key, int add) { void slotToKeyUpdateKeyCore(const char *key, size_t keylen, int add) { serverAssert(GlobalLocksAcquired()); + unsigned int hashslot = keyHashSlot(key,keylen); g_pserver->cluster->slots_keys_count[hashslot] += add ? 1 : -1; if (g_pserver->m_pstorageFactory == nullptr) { - unsigned int hashslot = keyHashSlot(key,keylen); unsigned char buf[64]; unsigned char *indexed = buf; diff --git a/src/server.h b/src/server.h index 9f5e3ac6c..ac927d07a 100644 --- a/src/server.h +++ b/src/server.h @@ -1371,7 +1371,7 @@ struct redisDb : public redisDbPersistentDataSnapshot using redisDbPersistentData::FRehashing; using redisDbPersistentData::FTrackingChanges; using redisDbPersistentData::CloneStorageCache; - using redisDbPersistentData::getStorageCache(); + using redisDbPersistentData::getStorageCache; using redisDbPersistentData::bulkDirectStorageInsert; public: diff --git a/src/storage/rocksdb.cpp b/src/storage/rocksdb.cpp index 8e13802d9..286ffe9f4 100644 --- a/src/storage/rocksdb.cpp +++ b/src/storage/rocksdb.cpp @@ -191,10 +191,10 @@ bool RocksDBStorageProvider::enumerate_hashslot(callback fn, int hashslot) const std::string prefix = getPrefix(hashslot); std::unique_ptr it = std::unique_ptr(m_spdb->NewIterator(ReadOptions(), m_spcolfamily.get())); size_t count = 0; - for (it->Seek(prefix.cstr()); it->Valid(); it->Next()) { + for (it->Seek(prefix.c_str()); it->Valid(); it->Next()) { if (FInternalKey(it->key().data(), it->key().size())) continue; - if (strncmp(it->key().data(),prefix.cstr(),2) != 0) + if (strncmp(it->key().data(),prefix.c_str(),2) != 0) break; ++count; bool fContinue = fn(it->key().data()+2, it->key().size()-2, it->value().data(), it->value().size()); From e52792fe2a2e2a2d6c51c58fb454b8e93d228ad4 Mon Sep 17 00:00:00 2001 From: Malavan Sotheeswaran Date: Tue, 9 May 2023 13:52:37 -0700 Subject: [PATCH 07/65] add hashslot enum to test storage provider --- src/storage/teststorageprovider.cpp | 17 ++++++++++++++++- src/storage/teststorageprovider.h | 1 + 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/src/storage/teststorageprovider.cpp b/src/storage/teststorageprovider.cpp index a287397c7..ab902e3e6 100644 --- a/src/storage/teststorageprovider.cpp +++ b/src/storage/teststorageprovider.cpp @@ -73,7 +73,22 @@ bool TestStorageProvider::enumerate(callback fn) const } return fAll; } - + +bool TestStorageProvider::enumerate_hashslot(callback fn, int hashslot) const +{ + bool fAll = true; + for (auto &pair : m_map) + { + if (keyHashSlot(pair.first.data(), pair.first.size()) == hashslot) + if (!fn(pair.first.data(), pair.first.size(), pair.second.data(), pair.second.size())) + { + fAll = false; + break; + } + } + return fAll; +} + size_t TestStorageProvider::count() const { return m_map.size(); diff --git a/src/storage/teststorageprovider.h b/src/storage/teststorageprovider.h index f21732256..ffd08f2f5 100644 --- a/src/storage/teststorageprovider.h +++ b/src/storage/teststorageprovider.h @@ -24,6 +24,7 @@ class TestStorageProvider final : public IStorage virtual void retrieve(const char *key, size_t cchKey, callbackSingle fn) const override; virtual size_t clear() override; virtual bool enumerate(callback fn) const override; + virtual bool enumerate_hashslot(callback fn, int hashslot) const override; virtual size_t count() const override; virtual void flush() override; From e0007d9687834599fd759d5019b530af01a8c396 Mon Sep 17 00:00:00 2001 From: Malavan Sotheeswaran Date: Tue, 9 May 2023 21:07:54 -0700 Subject: [PATCH 08/65] add --flash option for tests --- tests/instances.tcl | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/tests/instances.tcl b/tests/instances.tcl index d4270d091..e8ccfaeaa 100644 --- a/tests/instances.tcl +++ b/tests/instances.tcl @@ -21,6 +21,7 @@ set ::tls 0 set ::pause_on_error 0 set ::dont_clean 0 set ::simulate_error 0 +set ::flash 0 set ::failed 0 set ::sentinel_instances {} set ::redis_instances {} @@ -81,6 +82,10 @@ proc spawn_instance {type base_port count {conf {}} {base_conf_file ""}} { set cfg [open $cfgfile w] } + if {$::flash} { + puts $cfg "storage-provider flash ./flash_$base_port" + } + if {$::tls} { puts $cfg "tls-port $port" puts $cfg "tls-replication yes" @@ -266,6 +271,8 @@ proc parse_options {} { set val2 [lindex $::argv [expr $j+2]] dict set ::global_config $val $val2 incr j 2 + } elseif {$opt eq {--flash}} { + set ::flash 1 } elseif {$opt eq "--help"} { puts "--single Only runs tests specified by pattern." puts "--dont-clean Keep log files on exit." @@ -275,6 +282,7 @@ proc parse_options {} { puts "--tls Run tests in TLS mode." puts "--host Use hostname instead of 127.0.0.1." puts "--config Extra config argument(s)." + puts "--flash Run the whole suite with flash enabled" puts "--help Shows this help." exit 0 } else { From bc37934f6fea7a7b0785922f01d2f6898bd25635 Mon Sep 17 00:00:00 2001 From: Malavan Sotheeswaran Date: Tue, 9 May 2023 21:10:35 -0700 Subject: [PATCH 09/65] forgot return statement in getKeysInSlot --- src/db.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/db.cpp b/src/db.cpp index 0d64edadb..dcf617c10 100644 --- a/src/db.cpp +++ b/src/db.cpp @@ -2553,6 +2553,7 @@ unsigned int getKeysInSlot(unsigned int hashslot, robj **keys, unsigned int coun keys[j++] = createStringObject(key, cchKey); return --count; }, hashslot); + return j; } else { raxIterator iter; int j = 0; From eb565ae8f7cbde3a0ec6ace98a9cb1a3ef729447 Mon Sep 17 00:00:00 2001 From: Malavan Sotheeswaran Date: Wed, 10 May 2023 14:12:19 -0700 Subject: [PATCH 10/65] check for count mismatch --- src/storage/rocksdb.cpp | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/storage/rocksdb.cpp b/src/storage/rocksdb.cpp index 286ffe9f4..1a30148e0 100644 --- a/src/storage/rocksdb.cpp +++ b/src/storage/rocksdb.cpp @@ -201,14 +201,13 @@ bool RocksDBStorageProvider::enumerate_hashslot(callback fn, int hashslot) const if (!fContinue) break; } - if (!it->Valid() && count != m_count) + bool full_iter = !it->Valid() || (strncmp(it->key().data(),prefix.c_str(),2) != 0) + if (full_iter && count != g_pserver->cluster->slots_keys_count[hashslot]) { - if (const_cast(this)->m_count != count) - printf("WARNING: rocksdb count mismatch"); - const_cast(this)->m_count = count; + printf("WARNING: rocksdb hashslot count mismatch"); } assert(it->status().ok()); // Check for any errors found during the scan - return !it->Valid(); + return full_iter; } const IStorage *RocksDBStorageProvider::clone() const From 9bfb89bfd8f7472934ca5d42dddb5cd4e4eaa6c6 Mon Sep 17 00:00:00 2001 From: Malavan Sotheeswaran Date: Wed, 10 May 2023 14:16:18 -0700 Subject: [PATCH 11/65] forgot ; --- src/storage/rocksdb.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/storage/rocksdb.cpp b/src/storage/rocksdb.cpp index 1a30148e0..1bc7b35f0 100644 --- a/src/storage/rocksdb.cpp +++ b/src/storage/rocksdb.cpp @@ -201,7 +201,7 @@ bool RocksDBStorageProvider::enumerate_hashslot(callback fn, int hashslot) const if (!fContinue) break; } - bool full_iter = !it->Valid() || (strncmp(it->key().data(),prefix.c_str(),2) != 0) + bool full_iter = !it->Valid() || (strncmp(it->key().data(),prefix.c_str(),2) != 0); if (full_iter && count != g_pserver->cluster->slots_keys_count[hashslot]) { printf("WARNING: rocksdb hashslot count mismatch"); From ac1bd8eec41d614b4cbd8e399cebab3efb6a89f1 Mon Sep 17 00:00:00 2001 From: Malavan Sotheeswaran Date: Wed, 10 May 2023 14:19:17 -0700 Subject: [PATCH 12/65] add assert to ensure correctness --- src/storage/rocksdb.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/storage/rocksdb.cpp b/src/storage/rocksdb.cpp index 1bc7b35f0..31ede0305 100644 --- a/src/storage/rocksdb.cpp +++ b/src/storage/rocksdb.cpp @@ -206,6 +206,7 @@ bool RocksDBStorageProvider::enumerate_hashslot(callback fn, int hashslot) const { printf("WARNING: rocksdb hashslot count mismatch"); } + assert(!full_iter || count == g_pserver->cluster->slots_keys_count[hashslot]); assert(it->status().ok()); // Check for any errors found during the scan return full_iter; } From 76e103b9bf9478b093468bf63729374bbf010920 Mon Sep 17 00:00:00 2001 From: Malavan Sotheeswaran Date: Wed, 10 May 2023 15:25:39 -0700 Subject: [PATCH 13/65] fix warnings --- src/IStorage.h | 2 +- src/StorageCache.h | 2 +- src/storage/rocksdb.cpp | 4 ++-- src/storage/rocksdb.h | 2 +- src/storage/teststorageprovider.cpp | 2 +- src/storage/teststorageprovider.h | 2 +- 6 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/IStorage.h b/src/IStorage.h index c13e3e547..3d474a648 100644 --- a/src/IStorage.h +++ b/src/IStorage.h @@ -31,7 +31,7 @@ class IStorage virtual void retrieve(const char *key, size_t cchKey, callbackSingle fn) const = 0; virtual size_t clear() = 0; virtual bool enumerate(callback fn) const = 0; - virtual bool enumerate_hashslot(callback fn, int hashslot) const = 0; + virtual bool enumerate_hashslot(callback fn, unsigned int hashslot) const = 0; virtual size_t count() const = 0; virtual void bulkInsert(char **rgkeys, size_t *rgcbkeys, char **rgvals, size_t *rgcbvals, size_t celem) { diff --git a/src/StorageCache.h b/src/StorageCache.h index add1f0dad..4f3c1a374 100644 --- a/src/StorageCache.h +++ b/src/StorageCache.h @@ -49,7 +49,7 @@ class StorageCache void expand(uint64_t slots); bool enumerate(IStorage::callback fn) const { return m_spstorage->enumerate(fn); } - bool enumerate_hashslot(IStorage::callback fn, int hashslot) const { return m_spstorage->enumerate_hashslot(fn, hashslot); } + bool enumerate_hashslot(IStorage::callback fn, unsigned int hashslot) const { return m_spstorage->enumerate_hashslot(fn, hashslot); } void beginWriteBatch(); void endWriteBatch() { m_spstorage->endWriteBatch(); } diff --git a/src/storage/rocksdb.cpp b/src/storage/rocksdb.cpp index 31ede0305..76eaa133a 100644 --- a/src/storage/rocksdb.cpp +++ b/src/storage/rocksdb.cpp @@ -22,7 +22,7 @@ bool FInternalKey(const char *key, size_t cch) return false; } -std::string getPrefix(int hashslot) +std::string getPrefix(unsigned int hashslot) { char *hash_char = (char *)&hashslot; return std::string(hash_char + (sizeof(unsigned int) - 2), 2); @@ -186,7 +186,7 @@ bool RocksDBStorageProvider::enumerate(callback fn) const return !it->Valid(); } -bool RocksDBStorageProvider::enumerate_hashslot(callback fn, int hashslot) const +bool RocksDBStorageProvider::enumerate_hashslot(callback fn, unsigned int hashslot) const { std::string prefix = getPrefix(hashslot); std::unique_ptr it = std::unique_ptr(m_spdb->NewIterator(ReadOptions(), m_spcolfamily.get())); diff --git a/src/storage/rocksdb.h b/src/storage/rocksdb.h index 14a8df8fe..b78788eb2 100644 --- a/src/storage/rocksdb.h +++ b/src/storage/rocksdb.h @@ -32,7 +32,7 @@ class RocksDBStorageProvider : public IStorage virtual void retrieve(const char *key, size_t cchKey, callbackSingle fn) const override; virtual size_t clear() override; virtual bool enumerate(callback fn) const override; - virtual bool enumerate_hashslot(callback fn, int hashslot) const override; + virtual bool enumerate_hashslot(callback fn, unsigned int hashslot) const override; virtual const IStorage *clone() const override; diff --git a/src/storage/teststorageprovider.cpp b/src/storage/teststorageprovider.cpp index ab902e3e6..b15924fc4 100644 --- a/src/storage/teststorageprovider.cpp +++ b/src/storage/teststorageprovider.cpp @@ -74,7 +74,7 @@ bool TestStorageProvider::enumerate(callback fn) const return fAll; } -bool TestStorageProvider::enumerate_hashslot(callback fn, int hashslot) const +bool TestStorageProvider::enumerate_hashslot(callback fn, unsigned int hashslot) const { bool fAll = true; for (auto &pair : m_map) diff --git a/src/storage/teststorageprovider.h b/src/storage/teststorageprovider.h index ffd08f2f5..601b20a64 100644 --- a/src/storage/teststorageprovider.h +++ b/src/storage/teststorageprovider.h @@ -24,7 +24,7 @@ class TestStorageProvider final : public IStorage virtual void retrieve(const char *key, size_t cchKey, callbackSingle fn) const override; virtual size_t clear() override; virtual bool enumerate(callback fn) const override; - virtual bool enumerate_hashslot(callback fn, int hashslot) const override; + virtual bool enumerate_hashslot(callback fn, unsigned int hashslot) const override; virtual size_t count() const override; virtual void flush() override; From e47624dc8ed8bc8743606cab7958d8b4d1abd004 Mon Sep 17 00:00:00 2001 From: Malavan Sotheeswaran Date: Wed, 10 May 2023 22:21:27 -0700 Subject: [PATCH 14/65] add option to show logs of 1 server at a time --- tests/instances.tcl | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/tests/instances.tcl b/tests/instances.tcl index e8ccfaeaa..db267f54d 100644 --- a/tests/instances.tcl +++ b/tests/instances.tcl @@ -309,11 +309,15 @@ proc pause_on_error {} { break } elseif {$cmd eq {show-keydb-logs}} { set count 10 + set instance {} if {[lindex $argv 1] ne {}} {set count [lindex $argv 1]} + if {[lindex $argv 2] ne {}} {set instance [lindex $argv 2]} foreach_redis_id id { - puts "=== KeyDB $id ====" - puts [exec tail -$count redis_$id/log.txt] - puts "---------------------\n" + if {$instance eq $id || $instance eq {}} { + puts "=== KeyDB $id ====" + puts [exec tail -$count redis_$id/log.txt] + puts "---------------------\n" + } } } elseif {$cmd eq {show-sentinel-logs}} { set count 10 @@ -358,7 +362,7 @@ proc pause_on_error {} { } elseif {$cmd eq {help}} { puts "ls List Sentinel and KeyDB instances." puts "show-sentinel-logs \[N\] Show latest N lines of logs." - puts "show-keydb-logs \[N\] Show latest N lines of logs." + puts "show-keydb-logs \[N\] \[id\] Show latest N lines of logs of server id." puts "S cmd ... arg Call command in Sentinel ." puts "R cmd ... arg Call command in KeyDB ." puts "SI Show Sentinel INFO ." From e6c0e14cdd7170352a0930eca0490a9eb9025b73 Mon Sep 17 00:00:00 2001 From: Malavan Sotheeswaran Date: Thu, 11 May 2023 12:46:47 -0700 Subject: [PATCH 15/65] add fixed length prefix extractor --- src/storage/rocksdbfactory.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/storage/rocksdbfactory.cpp b/src/storage/rocksdbfactory.cpp index 6baa8d050..a46d9c556 100644 --- a/src/storage/rocksdbfactory.cpp +++ b/src/storage/rocksdbfactory.cpp @@ -71,6 +71,7 @@ RocksDBStorageFactory::RocksDBStorageFactory(const char *dbfile, int dbnum, cons rocksdb::DB *db = nullptr; auto options = RocksDbOptions(); + options.prefix_extractor.reset(rocksdb::NewFixedPrefixTransform(2)); for (int idb = 0; idb < dbnum; ++idb) { From 8b5fced206ba5c667acbc401e4893d92760d23f8 Mon Sep 17 00:00:00 2001 From: Malavan Sotheeswaran Date: Thu, 11 May 2023 21:05:59 -0700 Subject: [PATCH 16/65] add machamp --- build.yaml | 53 +++++++++++++++++++++++++++++++++++++++++++++++++++++ ci.yaml | 16 ++++++++++++++++ 2 files changed, 69 insertions(+) create mode 100644 build.yaml create mode 100644 ci.yaml diff --git a/build.yaml b/build.yaml new file mode 100644 index 000000000..7e524b9f7 --- /dev/null +++ b/build.yaml @@ -0,0 +1,53 @@ +# Doc: https://wiki.sc-corp.net/pages/viewpage.action?pageId=121500284 +version: 1 +machamp: + keydb-build: + # Optional - build counter is linked to the build def + tag_template: 0.0.%build.counter% + # Optional - value in seconds before a build is terminated, default is 3600 seconds + timeout: 3600 + # Optional - update ghe or not, default to true + update_ghe: true + code_coverage: false + # Required + steps: + make-build: + type: cmd + # https://github.sc-corp.net/Snapchat/img/tree/master/keydb/ubuntu-20-04 + builder_image: us.gcr.io/snapchat-build-artifacts/prod/snapchat/img/keydb/keydb-ubuntu-20-04@sha256:cf869a3f5d1de1e1d976bb906689c37b7031938eb68661b844a38c532f27248c + command: ./machamp_scripts/build.sh + tls-test: + type: cmd + parent: make-build + # https://github.sc-corp.net/Snapchat/img/tree/master/keydb/ubuntu-20-04 + builder_image: us.gcr.io/snapchat-build-artifacts/prod/snapchat/img/keydb/keydb-ubuntu-20-04@sha256:cf869a3f5d1de1e1d976bb906689c37b7031938eb68661b844a38c532f27248c + command: ./runtest --clients 4 --verbose + cluster-test: + type: cmd + parent: make-build + # https://github.sc-corp.net/Snapchat/img/tree/master/keydb/ubuntu-20-04 + builder_image: us.gcr.io/snapchat-build-artifacts/prod/snapchat/img/keydb/keydb-ubuntu-20-04@sha256:cf869a3f5d1de1e1d976bb906689c37b7031938eb68661b844a38c532f27248c + command: ./runtest-cluster + sentinel-test: + type: cmd + parent: make-build + # https://github.sc-corp.net/Snapchat/img/tree/master/keydb/ubuntu-20-04 + builder_image: us.gcr.io/snapchat-build-artifacts/prod/snapchat/img/keydb/keydb-ubuntu-20-04@sha256:cf869a3f5d1de1e1d976bb906689c37b7031938eb68661b844a38c532f27248c + command: ./runtest-sentinel + module-test: + type: cmd + parent: make-build + # https://github.sc-corp.net/Snapchat/img/tree/master/keydb/ubuntu-20-04 + builder_image: us.gcr.io/snapchat-build-artifacts/prod/snapchat/img/keydb/keydb-ubuntu-20-04@sha256:cf869a3f5d1de1e1d976bb906689c37b7031938eb68661b844a38c532f27248c + command: ./runtest-moduleapi + rotation-test: + type: cmd + parent: make-build + # https://github.sc-corp.net/Snapchat/img/tree/master/keydb/ubuntu-20-04 + builder_image: us.gcr.io/snapchat-build-artifacts/prod/snapchat/img/keydb/keydb-ubuntu-20-04@sha256:cf869a3f5d1de1e1d976bb906689c37b7031938eb68661b844a38c532f27248c + command: ./runtest-rotation + keyproxy-test: + type: cmd + parent: make-build + command: ./runtest-keyproxy --tls + builder_image: us.gcr.io/snapchat-build-artifacts/prod/snapchat/img/keydb/keydb-ubuntu-20-04@sha256:cf869a3f5d1de1e1d976bb906689c37b7031938eb68661b844a38c532f27248c diff --git a/ci.yaml b/ci.yaml new file mode 100644 index 000000000..89c048d1c --- /dev/null +++ b/ci.yaml @@ -0,0 +1,16 @@ +# Doc: https://wiki.sc-corp.net/display/TOOL/ci.yaml+User+Guide +version: 1 +on: + pull_request: + - workflows: + # All builds that use machamp should use the defined `backend_workflow` + - workflow_type: backend_workflow + # references a build defined in build.yaml + build_name: keydb-build + arch_types: ["amd64", "arm64"] + push: + - branches: [keydbpro] + workflows: + - workflow_type: backend_workflow + build_name: keydb-build + arch_types: ["amd64", "arm64"] From bae5c0dad1b41d503b042422ab906012d161676b Mon Sep 17 00:00:00 2001 From: Malavan Sotheeswaran Date: Thu, 11 May 2023 21:06:31 -0700 Subject: [PATCH 17/65] switch machamp to main branch --- ci.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ci.yaml b/ci.yaml index 89c048d1c..ab346113f 100644 --- a/ci.yaml +++ b/ci.yaml @@ -9,7 +9,7 @@ on: build_name: keydb-build arch_types: ["amd64", "arm64"] push: - - branches: [keydbpro] + - branches: [main] workflows: - workflow_type: backend_workflow build_name: keydb-build From cdfca6a42bb271fee6ae2210879d8650eea0b093 Mon Sep 17 00:00:00 2001 From: Malavan Sotheeswaran Date: Thu, 11 May 2023 21:20:26 -0700 Subject: [PATCH 18/65] add machamp script --- machamp_scripts/build.sh | 8 ++++++++ 1 file changed, 8 insertions(+) create mode 100755 machamp_scripts/build.sh diff --git a/machamp_scripts/build.sh b/machamp_scripts/build.sh new file mode 100755 index 000000000..e05a9b3bd --- /dev/null +++ b/machamp_scripts/build.sh @@ -0,0 +1,8 @@ +#!/bin/bash + +# make the build +git submodule init && git submodule update +make BUILD_TLS=yes -j$(nproc) KEYDB_CFLAGS='-Werror' KEYDB_CXXFLAGS='-Werror' + +# gen-cert +./utils/gen-test-certs.sh \ No newline at end of file From d4857720f236d0bab0f21e31e788713ca4b65bc0 Mon Sep 17 00:00:00 2001 From: Malavan Sotheeswaran Date: Wed, 17 May 2023 13:46:43 -0700 Subject: [PATCH 19/65] adding KeyDB side of flash expiration/eviction --- src/IStorage.h | 3 ++ src/StorageCache.h | 3 ++ src/evict.cpp | 72 +++++++++++++++++++++++++++++++ src/expire.cpp | 52 ++++++++++++++-------- src/storage/rocksdb.h | 3 ++ src/storage/teststorageprovider.h | 3 ++ 6 files changed, 118 insertions(+), 18 deletions(-) diff --git a/src/IStorage.h b/src/IStorage.h index 3d474a648..963372b86 100644 --- a/src/IStorage.h +++ b/src/IStorage.h @@ -42,6 +42,9 @@ class IStorage endWriteBatch(); } + virtual std::vector getExpirationCandidates() = 0; + virtual std::vector getEvictionCandidates() = 0; + virtual void beginWriteBatch() {} // NOP virtual void endWriteBatch() {} // NOP diff --git a/src/StorageCache.h b/src/StorageCache.h index 4f3c1a374..d3623bd0b 100644 --- a/src/StorageCache.h +++ b/src/StorageCache.h @@ -51,6 +51,9 @@ class StorageCache bool enumerate(IStorage::callback fn) const { return m_spstorage->enumerate(fn); } bool enumerate_hashslot(IStorage::callback fn, unsigned int hashslot) const { return m_spstorage->enumerate_hashslot(fn, hashslot); } + std::vector getExpirationCandidates() { return m_spstorage->getExpirationCandidates(); } + std::vector getEvictionCandidates() { return m_spstorage->getEvictionCandidates(); } + void beginWriteBatch(); void endWriteBatch() { m_spstorage->endWriteBatch(); } void batch_lock() { return m_spstorage->batch_lock(); } diff --git a/src/evict.cpp b/src/evict.cpp index 899812f19..ec048c3d6 100644 --- a/src/evict.cpp +++ b/src/evict.cpp @@ -657,6 +657,78 @@ int performEvictions(bool fPreSnapshot) { monotime evictionTimer; elapsedStart(&evictionTimer); + if (g_pserver->maxstorage && g_pserver->m_pstorageFactory != nullptr) { + while (g_pserver->m_pstorageFactory->totalDiskspaceUsed() >= g_pserver->maxstorage && elapsedUs(evictionTimer) < eviction_time_limit_us) { + redisDb *db; + std::vector evictionPool; + robj *bestkey = nullptr; + redisDb *bestdb = nullptr; + unsigned long long bestidle = 0; + for (int i = 0; i < cserver.dbnum; i++) { + db = g_pserver->db[i]; + evictionPool = db->getStorageCache()->getEvictionCandidates(); + for (std::string key : evictionPool) { + robj *keyobj = createStringObject(key.c_str(), key.size()); + robj *obj = db->find(szFromObj(keyobj)); + if (obj != nullptr) { + unsigned long long idle; + expireEntry *e = db->getExpire(keyobj); + + /* Calculate the idle time according to the policy. This is called + * idle just because the code initially handled LRU, but is in fact + * just a score where an higher score means better candidate. */ + if (g_pserver->maxmemory_policy & MAXMEMORY_FLAG_LRU) { + idle = (obj != nullptr) ? estimateObjectIdleTime(obj) : 0; + } else if (g_pserver->maxmemory_policy & MAXMEMORY_FLAG_LFU) { + /* When we use an LRU policy, we sort the keys by idle time + * so that we expire keys starting from greater idle time. + * However when the policy is an LFU one, we have a frequency + * estimation, and we want to evict keys with lower frequency + * first. So inside the pool we put objects using the inverted + * frequency subtracting the actual frequency to the maximum + * frequency of 255. */ + idle = 255-LFUDecrAndReturn(obj); + } else if (g_pserver->maxmemory_policy == MAXMEMORY_VOLATILE_TTL && e != nullptr) { + /* In this case the sooner the expire the better. */ + idle = ULLONG_MAX - e->when(); + } else { + serverPanic("Unknown eviction policy in storage eviction"); + } + + if (bestkey == nullptr || bestidle < idle) { + decrRefCount(bestkey); + incrRefCount(keyobj); + bestkey = keyobj; + bestidle = idle; + bestdb = db; + } + } + decrRefCount(keyobj); + } + } + if (bestkey) { + propagateExpire(bestdb, bestkey, g_pserver->lazyfree_lazy_eviction); + + latencyStartMonitor(eviction_latency); + if (g_pserver->lazyfree_lazy_eviction) + dbAsyncDelete(bestdb, bestkey); + else + dbSyncDelete(bestdb, bestkey); + latencyEndMonitor(eviction_latency); + latencyAddSampleIfNeeded("eviction-del", eviction_latency); + + g_pserver->stat_evictedkeys++; + signalModifiedKey(NULL, bestdb, bestkey); + notifyKeyspaceEvent(NOTIFY_EVICTED, "evicted", + bestkey, bestdb->id); + + decrRefCount(bestkey); + } else { + break; //could not find a key to evict so stop now + } + } + } + if (g_pserver->maxstorage && g_pserver->m_pstorageFactory != nullptr && g_pserver->m_pstorageFactory->totalDiskspaceUsed() >= g_pserver->maxstorage) goto cant_free_storage; diff --git a/src/expire.cpp b/src/expire.cpp index b727183ad..dd18f55c8 100644 --- a/src/expire.cpp +++ b/src/expire.cpp @@ -394,27 +394,43 @@ void activeExpireCycleCore(int type) { size_t expired = 0; size_t tried = 0; long long check = ACTIVE_EXPIRE_CYCLE_FAST_DURATION; // assume a check is roughly 1us. It isn't but good enough - db->expireitr = db->setexpireUnsafe()->enumerate(db->expireitr, now, [&](expireEntry &e) __attribute__((always_inline)) { - if (e.when() < now) - { - expired += activeExpireCycleExpire(db, e, now, tried); - } + if (g_pserver->m_pstorageFactory == nullptr) { + db->expireitr = db->setexpireUnsafe()->enumerate(db->expireitr, now, [&](expireEntry &e) __attribute__((always_inline)) { + if (e.when() < now) + { + expired += activeExpireCycleExpire(db, e, now, tried); + } - if ((tried % ACTIVE_EXPIRE_CYCLE_LOOKUPS_PER_LOOP) == 0) - { - /* We can't block forever here even if there are many keys to - * expire. So after a given amount of milliseconds return to the - * caller waiting for the other active expire cycle. */ - elapsed = ustime()-start; - if (elapsed > timelimit) { - timelimit_exit = 1; - g_pserver->stat_expired_time_cap_reached_count++; - return false; + if ((tried % ACTIVE_EXPIRE_CYCLE_LOOKUPS_PER_LOOP) == 0) + { + /* We can't block forever here even if there are many keys to + * expire. So after a given amount of milliseconds return to the + * caller waiting for the other active expire cycle. */ + elapsed = ustime()-start; + if (elapsed > timelimit) { + timelimit_exit = 1; + g_pserver->stat_expired_time_cap_reached_count++; + return false; + } + check = ACTIVE_EXPIRE_CYCLE_FAST_DURATION; } - check = ACTIVE_EXPIRE_CYCLE_FAST_DURATION; + return true; + }, &check); + } else { + std::vector keys = db->getStorageCache()->getExpirationCandidates(); + for (std::string key : keys) { + g_pserver->asyncworkqueue->AddWorkFunction([db,key]() { + aeAcquireLock(); + robj* keyobj = createStringObject(key.c_str(), key.size()); + db->find(szFromObj(keyobj)); + expireEntry *e = db->getExpire(keyobj); + size_t tried; + activeExpireCycleExpire(db, *e, mstime(), tried); + decrRefCount(keyobj); + aeReleaseLock(); + }); } - return true; - }, &check); + } total_expired += expired; } diff --git a/src/storage/rocksdb.h b/src/storage/rocksdb.h index b78788eb2..63d7e080b 100644 --- a/src/storage/rocksdb.h +++ b/src/storage/rocksdb.h @@ -34,6 +34,9 @@ class RocksDBStorageProvider : public IStorage virtual bool enumerate(callback fn) const override; virtual bool enumerate_hashslot(callback fn, unsigned int hashslot) const override; + virtual std::vector getExpirationCandidates() override { return std::vector(); } + virtual std::vector getEvictionCandidates() override { return std::vector(); } + virtual const IStorage *clone() const override; virtual void beginWriteBatch() override; diff --git a/src/storage/teststorageprovider.h b/src/storage/teststorageprovider.h index 601b20a64..27c0d10e3 100644 --- a/src/storage/teststorageprovider.h +++ b/src/storage/teststorageprovider.h @@ -27,6 +27,9 @@ class TestStorageProvider final : public IStorage virtual bool enumerate_hashslot(callback fn, unsigned int hashslot) const override; virtual size_t count() const override; + virtual std::vector getExpirationCandidates() override { return std::vector(); } + virtual std::vector getEvictionCandidates() override { return std::vector(); } + virtual void flush() override; /* This is permitted to be a shallow clone */ From 90fc43b849a01b59de9f085bea72f2346d7a2099 Mon Sep 17 00:00:00 2001 From: Malavan Sotheeswaran Date: Thu, 18 May 2023 10:19:49 -0700 Subject: [PATCH 20/65] add get expire and get eviction candidate impls for rocksdb --- src/storage/rocksdb.cpp | 55 +++++++++++++++++++++++++++++++++++++++++ src/storage/rocksdb.h | 5 ++-- 2 files changed, 58 insertions(+), 2 deletions(-) diff --git a/src/storage/rocksdb.cpp b/src/storage/rocksdb.cpp index 76eaa133a..42c725e3d 100644 --- a/src/storage/rocksdb.cpp +++ b/src/storage/rocksdb.cpp @@ -211,6 +211,61 @@ bool RocksDBStorageProvider::enumerate_hashslot(callback fn, unsigned int hashsl return full_iter; } +std::vector RocksDBStorageProvider::getExpirationCandidates() override +{ + std::vector result; + std::string slice; + auto status = m_spdb->Get(ReadOptions(), m_spcolfamily.get(), rocksdb::Slice(last_expire_key, sizeof(last_expire_key)), &slice); + long long lastTime = std::stoll(slice); + long long curTime = ustime() << TIME_BUCKET_SHIFT; + if (status.ok()) { + std::string bucketString; + int i; + for (i = lastTime; i < curTime; i++) { + std::string bucketString = INTERNAL_KEY_PREFIX + std::to_string(i); + status = m_spdb->Get(ReadOptions(), m_spcolfamily.get(), rocksdb::Slice(bucketString), &slice); + if (status.ok()) { + result += deserialize(slice); + if (m_spbatch != nullptr) + { + status = m_spbatch->Delete(m_spcolfamily.get(), rocksdb::Slice(bucketString)); + } + else + { + status = m_spdb->Delete(WriteOptions(), m_spcolfamily.get(), rocksdb::Slice(bucketString)); + } + } + } + status = m_spdb->Put(WriteOptions(), m_spcolfamily.get(), rocksdb::Slice(last_expire_key, sizeof(last_expire_key)), rocksdb::Slice(std::to_string(i))); + if (!status.ok()) { + assert("failed to update flash expire last time"); + } + } + return result; +} + +std::vector RocksDBStorageProvider::getEvictionCandidates() override +{ + std::vector result; + std::string slice; + auto status = m_spdb->Get(ReadOptions(), m_spcolfamily.get(), rocksdb::Slice(last_expire_key, sizeof(last_expire_key)), &slice); + long long lastTime = std::stoll(slice); + long long curTime = ustime() << TIME_BUCKET_SHIFT; + if (status.ok()) { + std::string bucketString; + int i = lastTime; + while (result.empty()) { + std::string bucketString = INTERNAL_KEY_PREFIX + std::to_string(i); + status = m_spdb->Get(ReadOptions(), m_spcolfamily.get(), rocksdb::Slice(bucketString), &slice); + if (status.ok()) { + result += deserialize(slice); + } + i++; + } + } + return result; +} + const IStorage *RocksDBStorageProvider::clone() const { std::unique_lock l(m_lock); diff --git a/src/storage/rocksdb.h b/src/storage/rocksdb.h index 63d7e080b..a48b23d2f 100644 --- a/src/storage/rocksdb.h +++ b/src/storage/rocksdb.h @@ -10,6 +10,7 @@ static const char count_key[] = INTERNAL_KEY_PREFIX "__keydb__count\1"; static const char version_key[] = INTERNAL_KEY_PREFIX "__keydb__version\1"; static const char meta_key[] = INTERNAL_KEY_PREFIX "__keydb__metadata\1"; +static const char last_expire_key[] = INTERNAL_KEY_PREFIX "__keydb__last_expire_time" class RocksDBStorageFactory; class RocksDBStorageProvider : public IStorage @@ -34,8 +35,8 @@ class RocksDBStorageProvider : public IStorage virtual bool enumerate(callback fn) const override; virtual bool enumerate_hashslot(callback fn, unsigned int hashslot) const override; - virtual std::vector getExpirationCandidates() override { return std::vector(); } - virtual std::vector getEvictionCandidates() override { return std::vector(); } + virtual std::vector getExpirationCandidates() override; + virtual std::vector getEvictionCandidates() override; virtual const IStorage *clone() const override; From edae1007676a56984fc1f053a5ad8ceb4845433f Mon Sep 17 00:00:00 2001 From: Malavan Sotheeswaran Date: Tue, 23 May 2023 20:47:15 -0700 Subject: [PATCH 21/65] refactor towards new design --- src/storage/rocksdb.cpp | 74 +++++++++++----------------- src/storage/rocksdb.h | 5 +- src/storage/rocksdbfactor_internal.h | 1 + src/storage/rocksdbfactory.cpp | 38 ++++++++------ 4 files changed, 57 insertions(+), 61 deletions(-) diff --git a/src/storage/rocksdb.cpp b/src/storage/rocksdb.cpp index 42c725e3d..cd19962ce 100644 --- a/src/storage/rocksdb.cpp +++ b/src/storage/rocksdb.cpp @@ -33,8 +33,8 @@ std::string prefixKey(const char *key, size_t cchKey) return FInternalKey(key, cchKey) ? std::string(key, cchKey) : getPrefix(keyHashSlot(key, cchKey)) + std::string(key, cchKey); } -RocksDBStorageProvider::RocksDBStorageProvider(RocksDBStorageFactory *pfactory, std::shared_ptr &spdb, std::shared_ptr &spcolfam, const rocksdb::Snapshot *psnapshot, size_t count) - : m_pfactory(pfactory), m_spdb(spdb), m_psnapshot(psnapshot), m_spcolfamily(spcolfam), m_count(count) +RocksDBStorageProvider::RocksDBStorageProvider(RocksDBStorageFactory *pfactory, std::shared_ptr &spdb, std::shared_ptr &spcolfam, std::shared_ptr &spexpirecolfam, const rocksdb::Snapshot *psnapshot, size_t count) + : m_pfactory(pfactory), m_spdb(spdb), m_psnapshot(psnapshot), m_spcolfamily(spcolfam), m_spexpirecolfamily(spexpirecolfam), m_count(count) { m_readOptionsTemplate = rocksdb::ReadOptions(); m_readOptionsTemplate.verify_checksums = false; @@ -211,56 +211,40 @@ bool RocksDBStorageProvider::enumerate_hashslot(callback fn, unsigned int hashsl return full_iter; } -std::vector RocksDBStorageProvider::getExpirationCandidates() override +std::vector RocksDBStorageProvider::getExpirationCandidates() { std::vector result; - std::string slice; - auto status = m_spdb->Get(ReadOptions(), m_spcolfamily.get(), rocksdb::Slice(last_expire_key, sizeof(last_expire_key)), &slice); - long long lastTime = std::stoll(slice); - long long curTime = ustime() << TIME_BUCKET_SHIFT; - if (status.ok()) { - std::string bucketString; - int i; - for (i = lastTime; i < curTime; i++) { - std::string bucketString = INTERNAL_KEY_PREFIX + std::to_string(i); - status = m_spdb->Get(ReadOptions(), m_spcolfamily.get(), rocksdb::Slice(bucketString), &slice); - if (status.ok()) { - result += deserialize(slice); - if (m_spbatch != nullptr) - { - status = m_spbatch->Delete(m_spcolfamily.get(), rocksdb::Slice(bucketString)); - } - else - { - status = m_spdb->Delete(WriteOptions(), m_spcolfamily.get(), rocksdb::Slice(bucketString)); - } - } - } - status = m_spdb->Put(WriteOptions(), m_spcolfamily.get(), rocksdb::Slice(last_expire_key, sizeof(last_expire_key)), rocksdb::Slice(std::to_string(i))); - if (!status.ok()) { - assert("failed to update flash expire last time"); - } + std::unique_ptr it = std::unique_ptr(m_spdb->NewIterator(ReadOptions(), m_spexpirecolfamily.get())); + for (it->SeekToFirst(); it->Valid() && result.size() < 16; it->Next()) { + if (FInternalKey(it->key().data(), it->key().size())) + continue; + result.emplace(it->value().data(), it->value().size()); + if (m_spbatch != nullptr) + status = m_spbatch->Delete(m_spexpirecolfamily.get(), it->key()); + else + status = m_spdb->Delete(WriteOptions(), m_spexpirecolfamily.get(), it->key()); + if (!status.ok()) + throw "Failed to remove expired key from rocksdb" } return result; } -std::vector RocksDBStorageProvider::getEvictionCandidates() override +std::vector RocksDBStorageProvider::getEvictionCandidates() { std::vector result; - std::string slice; - auto status = m_spdb->Get(ReadOptions(), m_spcolfamily.get(), rocksdb::Slice(last_expire_key, sizeof(last_expire_key)), &slice); - long long lastTime = std::stoll(slice); - long long curTime = ustime() << TIME_BUCKET_SHIFT; - if (status.ok()) { - std::string bucketString; - int i = lastTime; - while (result.empty()) { - std::string bucketString = INTERNAL_KEY_PREFIX + std::to_string(i); - status = m_spdb->Get(ReadOptions(), m_spcolfamily.get(), rocksdb::Slice(bucketString), &slice); - if (status.ok()) { - result += deserialize(slice); - } - i++; + if (g_pserver->maxmemory_policy & MAXMEMORY_FLAG_ALLKEYS) { + std::unique_ptr it = std::unique_ptr(m_spdb->NewIterator(ReadOptions(), m_spcolfamily.get())); + for (it->Seek(randomHashSlot()); it->Valid() && result.size() < 16; it->Next()) { + if (FInternalKey(it->key().data(), it->key().size())) + continue; + result.emplace(it->value().data(), it->value().size()); + } + } else { + std::unique_ptr it = std::unique_ptr(m_spdb->NewIterator(ReadOptions(), m_spexpirecolfamily.get())); + for (it->SeekToFirst(); it->Valid() && result.size() < 16; it->Next()) { + if (FInternalKey(it->key().data(), it->key().size())) + continue; + result.emplace(it->value().data(), it->value().size()); } } return result; @@ -270,7 +254,7 @@ const IStorage *RocksDBStorageProvider::clone() const { std::unique_lock l(m_lock); const rocksdb::Snapshot *psnapshot = const_cast(this)->m_spdb->GetSnapshot(); - return new RocksDBStorageProvider(m_pfactory, const_cast(this)->m_spdb, const_cast(this)->m_spcolfamily, psnapshot, m_count); + return new RocksDBStorageProvider(m_pfactory, const_cast(this)->m_spdb, const_cast(this)->m_spcolfamily, const_cast(this)->m_spexpirecolfamily, psnapshot, m_count); } RocksDBStorageProvider::~RocksDBStorageProvider() diff --git a/src/storage/rocksdb.h b/src/storage/rocksdb.h index a48b23d2f..530ebc51d 100644 --- a/src/storage/rocksdb.h +++ b/src/storage/rocksdb.h @@ -10,7 +10,7 @@ static const char count_key[] = INTERNAL_KEY_PREFIX "__keydb__count\1"; static const char version_key[] = INTERNAL_KEY_PREFIX "__keydb__version\1"; static const char meta_key[] = INTERNAL_KEY_PREFIX "__keydb__metadata\1"; -static const char last_expire_key[] = INTERNAL_KEY_PREFIX "__keydb__last_expire_time" +static const char last_expire_key[] = INTERNAL_KEY_PREFIX "__keydb__last_expire_time"; class RocksDBStorageFactory; class RocksDBStorageProvider : public IStorage @@ -20,12 +20,13 @@ class RocksDBStorageProvider : public IStorage std::unique_ptr m_spbatch; const rocksdb::Snapshot *m_psnapshot = nullptr; std::shared_ptr m_spcolfamily; + std::shared_ptr m_spexpirecolfamily; rocksdb::ReadOptions m_readOptionsTemplate; size_t m_count = 0; mutable fastlock m_lock {"RocksDBStorageProvider"}; public: - RocksDBStorageProvider(RocksDBStorageFactory *pfactory, std::shared_ptr &spdb, std::shared_ptr &spcolfam, const rocksdb::Snapshot *psnapshot, size_t count); + RocksDBStorageProvider(RocksDBStorageFactory *pfactory, std::shared_ptr &spdb, std::shared_ptr &spcolfam, std::shared_ptr &spexpirecolfam, const rocksdb::Snapshot *psnapshot, size_t count); ~RocksDBStorageProvider(); virtual void insert(const char *key, size_t cchKey, void *data, size_t cb, bool fOverwrite) override; diff --git a/src/storage/rocksdbfactor_internal.h b/src/storage/rocksdbfactor_internal.h index 11862bb7b..086f73856 100644 --- a/src/storage/rocksdbfactor_internal.h +++ b/src/storage/rocksdbfactor_internal.h @@ -5,6 +5,7 @@ class RocksDBStorageFactory : public IStorageFactory { std::shared_ptr m_spdb; // Note: This must be first so it is deleted last std::vector> m_vecspcols; + std::vector> m_vecspexpirecols; std::shared_ptr m_pfilemanager; std::string m_path; bool m_fCreatedTempFolder = false; diff --git a/src/storage/rocksdbfactory.cpp b/src/storage/rocksdbfactory.cpp index a46d9c556..74cda40bf 100644 --- a/src/storage/rocksdbfactory.cpp +++ b/src/storage/rocksdbfactory.cpp @@ -78,6 +78,7 @@ RocksDBStorageFactory::RocksDBStorageFactory(const char *dbfile, int dbnum, cons rocksdb::ColumnFamilyOptions cf_options(options); cf_options.level_compaction_dynamic_level_bytes = true; veccoldesc.push_back(rocksdb::ColumnFamilyDescriptor(std::to_string(idb), cf_options)); + veccoldesc.push_back(rocksdb::ColumnFamilyDescriptor(std::to_string(idb) + "_expires", cf_options)); } if (rgchConfig != nullptr) @@ -97,24 +98,32 @@ RocksDBStorageFactory::RocksDBStorageFactory(const char *dbfile, int dbnum, cons throw status.ToString(); m_spdb = std::shared_ptr(db); + rocksdb::ColumnFamilyDescriptor *desc = nullptr; for (auto handle : handles) { - std::string strVersion; - auto status = m_spdb->Get(rocksdb::ReadOptions(), handle, rocksdb::Slice(version_key, sizeof(version_key)), &strVersion); - if (!status.ok()) - { - setVersion(handle); + if (!handle->GetDescriptor(desc).ok()) { + throw "Error loading rocksdb handles"; } - else - { - SymVer ver = parseVersion(strVersion.c_str()); - auto cmp = compareVersion(&ver); - if (cmp == NewerVersion) - throw "Cannot load FLASH database created by newer version of KeyDB"; - if (cmp == OlderVersion) + if (!strncmp(desc->name.substr(desc->name.size() - 7).c_str(), "expires", 7)) { + m_vecspexpirecols.emplace_back(handle); + } else { + std::string strVersion; + auto status = m_spdb->Get(rocksdb::ReadOptions(), handle, rocksdb::Slice(version_key, sizeof(version_key)), &strVersion); + if (!status.ok()) + { setVersion(handle); + } + else + { + SymVer ver = parseVersion(strVersion.c_str()); + auto cmp = compareVersion(&ver); + if (cmp == NewerVersion) + throw "Cannot load FLASH database created by newer version of KeyDB"; + if (cmp == OlderVersion) + setVersion(handle); + } + m_vecspcols.emplace_back(handle); } - m_vecspcols.emplace_back(handle); } } @@ -155,6 +164,7 @@ IStorage *RocksDBStorageFactory::create(int db, key_load_iterator iter, void *pr { ++db; // skip default col family std::shared_ptr spcolfamily(m_vecspcols[db].release()); + std::shared_ptr spexpirecolfamily(m_vecspexpirecols[db].release()); size_t count = 0; bool fUnclean = false; @@ -191,7 +201,7 @@ IStorage *RocksDBStorageFactory::create(int db, key_load_iterator iter, void *pr ++count; } } - return new RocksDBStorageProvider(this, m_spdb, spcolfamily, nullptr, count); + return new RocksDBStorageProvider(this, m_spdb, spcolfamily, spexpirecolfamily, nullptr, count); } const char *RocksDBStorageFactory::name() const From 6eb595d6b4295f7a154367c8187d5c5d8eefef30 Mon Sep 17 00:00:00 2001 From: Malavan Sotheeswaran Date: Tue, 23 May 2023 20:50:58 -0700 Subject: [PATCH 22/65] remove expire from memory when evicting from cache --- src/db.cpp | 5 +++++ src/evict.cpp | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/src/db.cpp b/src/db.cpp index dcf617c10..c148fecd0 100644 --- a/src/db.cpp +++ b/src/db.cpp @@ -3155,6 +3155,11 @@ bool redisDbPersistentData::removeCachedValue(const char *key, dictEntry **ppde) if (m_spstorage != nullptr) m_spstorage->batch_unlock(); + std::unique_lock ul(g_expireLock); + auto itr = m_setexpire->find(sdsKey); + if (itr != m_setexpire->end()) + m_setexpire->erase(itr); + return true; } diff --git a/src/evict.cpp b/src/evict.cpp index ec048c3d6..fe9b82127 100644 --- a/src/evict.cpp +++ b/src/evict.cpp @@ -843,7 +843,7 @@ int performEvictions(bool fPreSnapshot) { if (db->removeCachedValue(bestkey, &deT)) { mem_freed += splazy->addEntry(db->dictUnsafeKeyOnly(), deT); ckeysFailed = 0; - g_pserver->stat_evictedkeys++; + g_pserver->stat_evictedkeys++; } else { delta = 0; From 6a32023cfc11bd7ce730b7486acec5f26bdeda49 Mon Sep 17 00:00:00 2001 From: Malavan Sotheeswaran Date: Tue, 23 May 2023 21:05:23 -0700 Subject: [PATCH 23/65] use correct key variable --- src/db.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/db.cpp b/src/db.cpp index c148fecd0..b93c53d2d 100644 --- a/src/db.cpp +++ b/src/db.cpp @@ -3156,7 +3156,7 @@ bool redisDbPersistentData::removeCachedValue(const char *key, dictEntry **ppde) m_spstorage->batch_unlock(); std::unique_lock ul(g_expireLock); - auto itr = m_setexpire->find(sdsKey); + auto itr = m_setexpire->find(key); if (itr != m_setexpire->end()) m_setexpire->erase(itr); From f701161d0a3426465aac4d97e52584ae19b3fbdb Mon Sep 17 00:00:00 2001 From: Malavan Sotheeswaran Date: Tue, 23 May 2023 21:10:40 -0700 Subject: [PATCH 24/65] more progress --- src/storage/rocksdb.cpp | 24 +++++++++++++++--------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/src/storage/rocksdb.cpp b/src/storage/rocksdb.cpp index cd19962ce..908b7b1ef 100644 --- a/src/storage/rocksdb.cpp +++ b/src/storage/rocksdb.cpp @@ -215,16 +215,22 @@ std::vector RocksDBStorageProvider::getExpirationCandidates() { std::vector result; std::unique_ptr it = std::unique_ptr(m_spdb->NewIterator(ReadOptions(), m_spexpirecolfamily.get())); + long long curTime = ustime(); for (it->SeekToFirst(); it->Valid() && result.size() < 16; it->Next()) { if (FInternalKey(it->key().data(), it->key().size())) continue; - result.emplace(it->value().data(), it->value().size()); - if (m_spbatch != nullptr) - status = m_spbatch->Delete(m_spexpirecolfamily.get(), it->key()); - else - status = m_spdb->Delete(WriteOptions(), m_spexpirecolfamily.get(), it->key()); - if (!status.ok()) - throw "Failed to remove expired key from rocksdb" + if (*((long long *)it->key().data()) <= curTime) { + result.emplace_back(it->value().data(), it->value().size()); + rocksdb::Status status; + if (m_spbatch != nullptr) + status = m_spbatch->Delete(m_spexpirecolfamily.get(), it->key()); + else + status = m_spdb->Delete(WriteOptions(), m_spexpirecolfamily.get(), it->key()); + if (!status.ok()) + throw "Failed to remove expired key from rocksdb"; + } else { + break; + } } return result; } @@ -237,14 +243,14 @@ std::vector RocksDBStorageProvider::getEvictionCandidates() for (it->Seek(randomHashSlot()); it->Valid() && result.size() < 16; it->Next()) { if (FInternalKey(it->key().data(), it->key().size())) continue; - result.emplace(it->value().data(), it->value().size()); + result.emplace_back(it->value().data(), it->value().size()); } } else { std::unique_ptr it = std::unique_ptr(m_spdb->NewIterator(ReadOptions(), m_spexpirecolfamily.get())); for (it->SeekToFirst(); it->Valid() && result.size() < 16; it->Next()) { if (FInternalKey(it->key().data(), it->key().size())) continue; - result.emplace(it->value().data(), it->value().size()); + result.emplace_back(it->value().data(), it->value().size()); } } return result; From 6853b15d50d426785c91cd9a625319b06efcbb01 Mon Sep 17 00:00:00 2001 From: Malavan Sotheeswaran Date: Tue, 23 May 2023 21:26:01 -0700 Subject: [PATCH 25/65] add random hashslot impl --- src/storage/rocksdb.cpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/storage/rocksdb.cpp b/src/storage/rocksdb.cpp index 908b7b1ef..d307e38aa 100644 --- a/src/storage/rocksdb.cpp +++ b/src/storage/rocksdb.cpp @@ -5,6 +5,7 @@ #include #include "../server.h" #include "../cluster.h" +#include "../mt19937-64.h" #include "rocksdbfactor_internal.h" static const char keyprefix[] = INTERNAL_KEY_PREFIX; @@ -235,6 +236,10 @@ std::vector RocksDBStorageProvider::getExpirationCandidates() return result; } +std::string randomHashSlot() { + return getPrefix(genrand64_int63() % (1 << 16)); +} + std::vector RocksDBStorageProvider::getEvictionCandidates() { std::vector result; From 5cb2ead650f57a9fc9ac113696427f74fefa48d2 Mon Sep 17 00:00:00 2001 From: Malavan Sotheeswaran Date: Wed, 24 May 2023 14:55:41 -0700 Subject: [PATCH 26/65] refactor for time limited expire cycle --- src/expire.cpp | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/src/expire.cpp b/src/expire.cpp index dd18f55c8..f87104b1f 100644 --- a/src/expire.cpp +++ b/src/expire.cpp @@ -417,19 +417,17 @@ void activeExpireCycleCore(int type) { return true; }, &check); } else { - std::vector keys = db->getStorageCache()->getExpirationCandidates(); - for (std::string key : keys) { - g_pserver->asyncworkqueue->AddWorkFunction([db,key]() { - aeAcquireLock(); + std::vector keys; + do { + keys = db->getStorageCache()->getExpirationCandidates(); + for (std::string key : keys) { robj* keyobj = createStringObject(key.c_str(), key.size()); db->find(szFromObj(keyobj)); expireEntry *e = db->getExpire(keyobj); - size_t tried; activeExpireCycleExpire(db, *e, mstime(), tried); decrRefCount(keyobj); - aeReleaseLock(); - }); - } + } + } while (keys.size() > 0 && (ustime()-start < timelimit)); } total_expired += expired; From 5aff96c57734ffce9c91a95b451739b478152797 Mon Sep 17 00:00:00 2001 From: Malavan Sotheeswaran Date: Wed, 24 May 2023 15:07:15 -0700 Subject: [PATCH 27/65] fixes --- src/expire.cpp | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/expire.cpp b/src/expire.cpp index f87104b1f..5ff493998 100644 --- a/src/expire.cpp +++ b/src/expire.cpp @@ -424,10 +424,16 @@ void activeExpireCycleCore(int type) { robj* keyobj = createStringObject(key.c_str(), key.size()); db->find(szFromObj(keyobj)); expireEntry *e = db->getExpire(keyobj); - activeExpireCycleExpire(db, *e, mstime(), tried); + expired += activeExpireCycleExpire(db, *e, mstime(), tried); decrRefCount(keyobj); } - } while (keys.size() > 0 && (ustime()-start < timelimit)); + elapsed = ustime()-start; + } while (keys.size() > 0 && (elapsed < timelimit)); + + if (ustime()-start > timelimit) { + timelimit_exit = 1; + g_pserver->stat_expired_time_cap_reached_count++; + } } total_expired += expired; From 12fd46da0b26f4a1cc5bcd3592786656b02c05e6 Mon Sep 17 00:00:00 2001 From: Malavan Sotheeswaran Date: Wed, 24 May 2023 21:57:12 -0700 Subject: [PATCH 28/65] add ability to set expires in storge provider --- src/IStorage.h | 1 + src/StorageCache.h | 1 + src/expire.cpp | 2 +- src/storage/rocksdb.cpp | 14 ++++++++++++++ src/storage/rocksdb.h | 1 + src/storage/teststorageprovider.h | 1 + 6 files changed, 19 insertions(+), 1 deletion(-) diff --git a/src/IStorage.h b/src/IStorage.h index 963372b86..d535b24c2 100644 --- a/src/IStorage.h +++ b/src/IStorage.h @@ -44,6 +44,7 @@ class IStorage virtual std::vector getExpirationCandidates() = 0; virtual std::vector getEvictionCandidates() = 0; + virtual void setExpire(const char *key, size_t cchKey, long long expire) = 0; virtual void beginWriteBatch() {} // NOP virtual void endWriteBatch() {} // NOP diff --git a/src/StorageCache.h b/src/StorageCache.h index d3623bd0b..7dda1a30e 100644 --- a/src/StorageCache.h +++ b/src/StorageCache.h @@ -53,6 +53,7 @@ class StorageCache std::vector getExpirationCandidates() { return m_spstorage->getExpirationCandidates(); } std::vector getEvictionCandidates() { return m_spstorage->getEvictionCandidates(); } + void setExpire(const char *key, size_t cchKey, long long expire) { m_spstorage->setExpire(key, cchKey, expire); } void beginWriteBatch(); void endWriteBatch() { m_spstorage->endWriteBatch(); } diff --git a/src/expire.cpp b/src/expire.cpp index 5ff493998..ef7532694 100644 --- a/src/expire.cpp +++ b/src/expire.cpp @@ -383,7 +383,7 @@ void activeExpireCycleCore(int type) { now = mstime(); /* If there is nothing to expire try next DB ASAP. */ - if (db->setexpireUnsafe()->empty()) + if (db->setexpireUnsafe()->empty() && g_pserver->m_pstorageFactory == nullptr) { db->avg_ttl = 0; db->last_expire_set = now; diff --git a/src/storage/rocksdb.cpp b/src/storage/rocksdb.cpp index d307e38aa..d27889bff 100644 --- a/src/storage/rocksdb.cpp +++ b/src/storage/rocksdb.cpp @@ -212,6 +212,20 @@ bool RocksDBStorageProvider::enumerate_hashslot(callback fn, unsigned int hashsl return full_iter; } +void RocksDBStorageProvider::setExpire(const char *key, size_t cchKey, long long expire) +{ + rocksdb::Status status; + std::unique_lock l(m_lock); + std::string prefix((const char *)&expire,sizeof(long long)); + std::string strKey(key, cchKey); + if (m_spbatch != nullptr) + status = m_spbatch->Put(m_spexpirecolfamily.get(), rocksdb::Slice(prefix + strKey), rocksdb::Slice(strKey)); + else + status = m_spdb->Put(WriteOptions(), m_spexpirecolfamily.get(), rocksdb::Slice(prefix + strKey), rocksdb::Slice(strKey)); + if (!status.ok()) + throw status.ToString(); +} + std::vector RocksDBStorageProvider::getExpirationCandidates() { std::vector result; diff --git a/src/storage/rocksdb.h b/src/storage/rocksdb.h index 530ebc51d..9d8cf8ea9 100644 --- a/src/storage/rocksdb.h +++ b/src/storage/rocksdb.h @@ -38,6 +38,7 @@ class RocksDBStorageProvider : public IStorage virtual std::vector getExpirationCandidates() override; virtual std::vector getEvictionCandidates() override; + virtual void setExpire(const char *key, size_t cchKey, long long expire) override; virtual const IStorage *clone() const override; diff --git a/src/storage/teststorageprovider.h b/src/storage/teststorageprovider.h index 27c0d10e3..75271d9c1 100644 --- a/src/storage/teststorageprovider.h +++ b/src/storage/teststorageprovider.h @@ -29,6 +29,7 @@ class TestStorageProvider final : public IStorage virtual std::vector getExpirationCandidates() override { return std::vector(); } virtual std::vector getEvictionCandidates() override { return std::vector(); } + virtual void setExpire(const char *, size_t, long long) override {} virtual void flush() override; From 708b8d941b43c04da4a80e664725b13a89bb998b Mon Sep 17 00:00:00 2001 From: Malavan Sotheeswaran Date: Thu, 25 May 2023 00:17:12 -0700 Subject: [PATCH 29/65] actually add expires to rocksdb --- src/db.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/db.cpp b/src/db.cpp index b93c53d2d..3bd9ddabf 100644 --- a/src/db.cpp +++ b/src/db.cpp @@ -1871,6 +1871,7 @@ void redisDbPersistentData::resortExpire(expireEntry &e) expireEntry eT = std::move(e); m_setexpire->erase(itr); m_setexpire->insert(eT); + getStorageCache()->setExpire(e.key(), sdslen(e.key()), e.when()); } /* Set an expire to the specified key. If the expire is set in the context @@ -2801,6 +2802,7 @@ void redisDbPersistentData::setExpire(robj *key, robj *subkey, long long when) ((robj*)dictGetVal(kde))->SetFExpires(true); m_setexpire->insert(e); } + getStorageCache()->setExpire(szFromObj(key), sdslen(szFromObj(key)), when); } void redisDbPersistentData::setExpire(expireEntry &&e) @@ -2808,6 +2810,7 @@ void redisDbPersistentData::setExpire(expireEntry &&e) std::unique_lock ul(g_expireLock); trackkey(e.key(), true /* fUpdate */); m_setexpire->insert(e); + getStorageCache()->setExpire(e.key(), sdslen(e.key()), e.when()); } bool redisDb::FKeyExpires(const char *key) From ef1483e1d432ea012e90028e7bb760dab830bd4b Mon Sep 17 00:00:00 2001 From: Malavan Sotheeswaran Date: Thu, 25 May 2023 14:01:19 -0700 Subject: [PATCH 30/65] fix crash --- src/db.cpp | 9 ++++++--- src/storage/rocksdbfactory.cpp | 12 ++++-------- 2 files changed, 10 insertions(+), 11 deletions(-) diff --git a/src/db.cpp b/src/db.cpp index 3bd9ddabf..7585e8b7b 100644 --- a/src/db.cpp +++ b/src/db.cpp @@ -1871,7 +1871,8 @@ void redisDbPersistentData::resortExpire(expireEntry &e) expireEntry eT = std::move(e); m_setexpire->erase(itr); m_setexpire->insert(eT); - getStorageCache()->setExpire(e.key(), sdslen(e.key()), e.when()); + if (g_pserver->m_pstorageFactory != nullptr) + getStorageCache()->setExpire(e.key(), sdslen(e.key()), e.when()); } /* Set an expire to the specified key. If the expire is set in the context @@ -2802,7 +2803,8 @@ void redisDbPersistentData::setExpire(robj *key, robj *subkey, long long when) ((robj*)dictGetVal(kde))->SetFExpires(true); m_setexpire->insert(e); } - getStorageCache()->setExpire(szFromObj(key), sdslen(szFromObj(key)), when); + if (g_pserver->m_pstorageFactory != nullptr) + getStorageCache()->setExpire(szFromObj(key), sdslen(szFromObj(key)), when); } void redisDbPersistentData::setExpire(expireEntry &&e) @@ -2810,7 +2812,8 @@ void redisDbPersistentData::setExpire(expireEntry &&e) std::unique_lock ul(g_expireLock); trackkey(e.key(), true /* fUpdate */); m_setexpire->insert(e); - getStorageCache()->setExpire(e.key(), sdslen(e.key()), e.when()); + if (g_pserver->m_pstorageFactory != nullptr) + getStorageCache()->setExpire(e.key(), sdslen(e.key()), e.when()); } bool redisDb::FKeyExpires(const char *key) diff --git a/src/storage/rocksdbfactory.cpp b/src/storage/rocksdbfactory.cpp index 74cda40bf..fb5b28ac8 100644 --- a/src/storage/rocksdbfactory.cpp +++ b/src/storage/rocksdbfactory.cpp @@ -60,8 +60,8 @@ RocksDBStorageFactory::RocksDBStorageFactory(const char *dbfile, int dbnum, cons auto status = rocksdb::DB::ListColumnFamilies(rocksdb::Options(), dbfile, &vecT); // RocksDB requires we know the count of col families before opening, if the user only wants to see less // we still have to make room for all column family handles regardless - if (status.ok() && (int)vecT.size() > dbnum) - dbnum = (int)vecT.size(); + if (status.ok() && (int)vecT.size()/2 > dbnum) + dbnum = (int)vecT.size()/2; std::vector veccoldesc; veccoldesc.push_back(rocksdb::ColumnFamilyDescriptor(rocksdb::kDefaultColumnFamilyName, rocksdb::ColumnFamilyOptions())); // ignore default col family @@ -98,13 +98,9 @@ RocksDBStorageFactory::RocksDBStorageFactory(const char *dbfile, int dbnum, cons throw status.ToString(); m_spdb = std::shared_ptr(db); - rocksdb::ColumnFamilyDescriptor *desc = nullptr; for (auto handle : handles) { - if (!handle->GetDescriptor(desc).ok()) { - throw "Error loading rocksdb handles"; - } - if (!strncmp(desc->name.substr(desc->name.size() - 7).c_str(), "expires", 7)) { + if (handle->GetName().size() > 7 && !strncmp(handle->GetName().substr(handle->GetName().size() - 7).c_str(), "expires", 7)) { m_vecspexpirecols.emplace_back(handle); } else { std::string strVersion; @@ -124,7 +120,7 @@ RocksDBStorageFactory::RocksDBStorageFactory(const char *dbfile, int dbnum, cons } m_vecspcols.emplace_back(handle); } - } + } } RocksDBStorageFactory::~RocksDBStorageFactory() From 71045949c21b154d92ec35620241177cd28d3b1f Mon Sep 17 00:00:00 2001 From: Malavan Sotheeswaran Date: Fri, 26 May 2023 02:42:19 -0700 Subject: [PATCH 31/65] fix include and crash --- src/IStorage.h | 1 + src/expire.cpp | 3 ++- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/src/IStorage.h b/src/IStorage.h index d535b24c2..4bd82333d 100644 --- a/src/IStorage.h +++ b/src/IStorage.h @@ -1,6 +1,7 @@ #pragma once #include #include "sds.h" +#include #define METADATA_DB_IDENTIFIER "c299fde0-6d42-4ec4-b939-34f680ffe39f" diff --git a/src/expire.cpp b/src/expire.cpp index ef7532694..fb386121f 100644 --- a/src/expire.cpp +++ b/src/expire.cpp @@ -424,7 +424,8 @@ void activeExpireCycleCore(int type) { robj* keyobj = createStringObject(key.c_str(), key.size()); db->find(szFromObj(keyobj)); expireEntry *e = db->getExpire(keyobj); - expired += activeExpireCycleExpire(db, *e, mstime(), tried); + if (e != nullptr) + expired += activeExpireCycleExpire(db, *e, mstime(), tried); decrRefCount(keyobj); } elapsed = ustime()-start; From 49f48d808dfd508cdac05b9e413cd711eb4b7831 Mon Sep 17 00:00:00 2001 From: Malavan Sotheeswaran Date: Fri, 26 May 2023 03:00:15 -0700 Subject: [PATCH 32/65] double check expire time --- src/expire.cpp | 2 +- src/storage/rocksdb.cpp | 22 +++++++++------------- 2 files changed, 10 insertions(+), 14 deletions(-) diff --git a/src/expire.cpp b/src/expire.cpp index fb386121f..52da20326 100644 --- a/src/expire.cpp +++ b/src/expire.cpp @@ -424,7 +424,7 @@ void activeExpireCycleCore(int type) { robj* keyobj = createStringObject(key.c_str(), key.size()); db->find(szFromObj(keyobj)); expireEntry *e = db->getExpire(keyobj); - if (e != nullptr) + if (e != nullptr && e->when() < now) expired += activeExpireCycleExpire(db, *e, mstime(), tried); decrRefCount(keyobj); } diff --git a/src/storage/rocksdb.cpp b/src/storage/rocksdb.cpp index d27889bff..2f859a261 100644 --- a/src/storage/rocksdb.cpp +++ b/src/storage/rocksdb.cpp @@ -231,21 +231,17 @@ std::vector RocksDBStorageProvider::getExpirationCandidates() std::vector result; std::unique_ptr it = std::unique_ptr(m_spdb->NewIterator(ReadOptions(), m_spexpirecolfamily.get())); long long curTime = ustime(); - for (it->SeekToFirst(); it->Valid() && result.size() < 16; it->Next()) { + for (it->SeekToFirst(); it->Valid() && (*((long long *)it->key().data()) <= curTime) && (result.size() < 16); it->Next()) { if (FInternalKey(it->key().data(), it->key().size())) continue; - if (*((long long *)it->key().data()) <= curTime) { - result.emplace_back(it->value().data(), it->value().size()); - rocksdb::Status status; - if (m_spbatch != nullptr) - status = m_spbatch->Delete(m_spexpirecolfamily.get(), it->key()); - else - status = m_spdb->Delete(WriteOptions(), m_spexpirecolfamily.get(), it->key()); - if (!status.ok()) - throw "Failed to remove expired key from rocksdb"; - } else { - break; - } + result.emplace_back(it->value().data(), it->value().size()); + rocksdb::Status status; + if (m_spbatch != nullptr) + status = m_spbatch->Delete(m_spexpirecolfamily.get(), it->key()); + else + status = m_spdb->Delete(WriteOptions(), m_spexpirecolfamily.get(), it->key()); + if (!status.ok()) + throw "Failed to remove expired key from rocksdb"; } return result; } From f737084170d64d46996c8e0a7cac38cbbb4a2b7d Mon Sep 17 00:00:00 2001 From: Malavan Sotheeswaran Date: Fri, 26 May 2023 03:05:29 -0700 Subject: [PATCH 33/65] mstime not ustime --- src/expire.cpp | 2 +- src/storage/rocksdb.cpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/expire.cpp b/src/expire.cpp index 52da20326..fb386121f 100644 --- a/src/expire.cpp +++ b/src/expire.cpp @@ -424,7 +424,7 @@ void activeExpireCycleCore(int type) { robj* keyobj = createStringObject(key.c_str(), key.size()); db->find(szFromObj(keyobj)); expireEntry *e = db->getExpire(keyobj); - if (e != nullptr && e->when() < now) + if (e != nullptr) expired += activeExpireCycleExpire(db, *e, mstime(), tried); decrRefCount(keyobj); } diff --git a/src/storage/rocksdb.cpp b/src/storage/rocksdb.cpp index 2f859a261..2ca513255 100644 --- a/src/storage/rocksdb.cpp +++ b/src/storage/rocksdb.cpp @@ -230,7 +230,7 @@ std::vector RocksDBStorageProvider::getExpirationCandidates() { std::vector result; std::unique_ptr it = std::unique_ptr(m_spdb->NewIterator(ReadOptions(), m_spexpirecolfamily.get())); - long long curTime = ustime(); + long long curTime = mstime(); for (it->SeekToFirst(); it->Valid() && (*((long long *)it->key().data()) <= curTime) && (result.size() < 16); it->Next()) { if (FInternalKey(it->key().data(), it->key().size())) continue; From e54fd922d9a4b8f322863963ab44964c9fd3949f Mon Sep 17 00:00:00 2001 From: Malavan Sotheeswaran Date: Mon, 12 Jun 2023 21:51:40 -0700 Subject: [PATCH 34/65] update expire column during fast sync --- src/StorageCache.cpp | 7 ++++++- src/db.cpp | 4 ++-- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/src/StorageCache.cpp b/src/StorageCache.cpp index 91d4b3657..816765654 100644 --- a/src/StorageCache.cpp +++ b/src/StorageCache.cpp @@ -119,13 +119,18 @@ void StorageCache::bulkInsert(char **rgkeys, size_t *rgcbkeys, char **rgvals, si std::vector vechashes; if (m_pdict != nullptr) { vechashes.reserve(celem); + } - for (size_t ielem = 0; ielem < celem; ++ielem) { + for (size_t ielem = 0; ielem < celem; ++ielem) { + if (m_pdict != nullptr) { dictEntry *de = (dictEntry*)zmalloc(sizeof(dictEntry)); de->key = (void*)dictGenHashFunction(rgkeys[ielem], (int)rgcbkeys[ielem]); de->v.u64 = 1; vechashes.push_back(de); } + auto e = deserializeExpire(rgkeys[ielem], rgvals[ielem], rgcbvals[ielem], nullptr); + if (e != nullptr) + m_spstorage->setExpire(rgkeys[ielem], rgcbkeys[ielem], e->when()); } std::unique_lock ul(m_lock); diff --git a/src/db.cpp b/src/db.cpp index 7585e8b7b..1351b0678 100644 --- a/src/db.cpp +++ b/src/db.cpp @@ -3299,8 +3299,8 @@ std::unique_ptr deserializeExpire(sds key, const char *str, size_t if (subkey) sdsfree(subkey); } - - *poffset = offset; + if (poffset != nullptr) + *poffset = offset; return spexpire; } From 5f1884fc361a0831c4f49831fe1fdea908dd9140 Mon Sep 17 00:00:00 2001 From: Malavan Sotheeswaran Date: Mon, 12 Jun 2023 22:55:36 -0700 Subject: [PATCH 35/65] include vector --- src/storage/teststorageprovider.h | 1 + 1 file changed, 1 insertion(+) diff --git a/src/storage/teststorageprovider.h b/src/storage/teststorageprovider.h index 75271d9c1..408d63b3f 100644 --- a/src/storage/teststorageprovider.h +++ b/src/storage/teststorageprovider.h @@ -1,6 +1,7 @@ #include "../IStorage.h" #include #include +#include class TestStorageFactory : public IStorageFactory { From 63dedf054f7d2c50c3009a56c65196ee41514e79 Mon Sep 17 00:00:00 2001 From: Malavan Sotheeswaran Date: Tue, 13 Jun 2023 19:12:26 -0700 Subject: [PATCH 36/65] add unit test for flash eviction --- tests/unit/flash.tcl | 42 +++++++++++++++++++++++++++++++++++++++++- 1 file changed, 41 insertions(+), 1 deletion(-) diff --git a/tests/unit/flash.tcl b/tests/unit/flash.tcl index e66248fb2..b24f41f44 100644 --- a/tests/unit/flash.tcl +++ b/tests/unit/flash.tcl @@ -147,7 +147,47 @@ if {$::flash_enabled} { # should still be under the limit for maxmemory, however all keys set should still exist between flash and memory # check same number of keys exist in addition to values of first and last keys set err 0 - set extra_keys [expr floor([expr ($limit * 0.4) / 1024])] + set extra_keys [expr floor([expr ($limit * 0.4) / 1024])] + for {set j 0} {$j < $extra_keys} {incr j} { + catch { + r set p2$j xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx + } err + assert {$err == {OK}} + } + if {[log_file_matches [srv 0 stdout] "*Failed to evict*"]} { + fail "Server did not evict cleanly (detected full flush)" + } + r set last val + set dbsize [r dbsize] + assert {[s used_memory] < ($limit*1.2)} + assert {$dbsize == $numkeys+$extra_keys+2} + assert {[r get first] == {val}} + assert {[r get last] == {val}} + r flushall + } + + test "FLASH - is flash eviction working? (policy $policy)" { + # Get the current memory limit and calculate a new limit. + # Set limit to 100M. + set used [s used_memory] + set limit [expr {$used+60*1024*1024}] + r config set maxstorage $limit + r config set maxmemory-policy $policy + # Now add keys equivalent to 1024b until the limit is almost reached. + set numkeys 0 + r set first val + while 1 { + r set $numkeys xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx + incr numkeys + if {[s used_memory]+1024 >= $limit} { + break + } + } + # Add additional keys to force eviction + # should still be under the limit for maxmemory, however all keys set should still exist between flash and memory + # check same number of keys exist in addition to values of first and last keys + set err 0 + set extra_keys [expr floor([expr ($limit * 0.4) / 1024])] for {set j 0} {$j < $extra_keys} {incr j} { catch { r set p2$j xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx From e1afce8a4af318d724a145d20e2f8f3fb0f9fc5e Mon Sep 17 00:00:00 2001 From: Malavan Sotheeswaran Date: Tue, 13 Jun 2023 19:19:04 -0700 Subject: [PATCH 37/65] remove extra blank line --- tests/unit/flash.tcl | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/unit/flash.tcl b/tests/unit/flash.tcl index b24f41f44..c2aea0fe1 100644 --- a/tests/unit/flash.tcl +++ b/tests/unit/flash.tcl @@ -206,6 +206,5 @@ if {$::flash_enabled} { r flushall } } - } } From 8e41dc3ed2e78b2c7fc145105274e0a207ec6e97 Mon Sep 17 00:00:00 2001 From: Malavan Sotheeswaran Date: Tue, 13 Jun 2023 19:19:48 -0700 Subject: [PATCH 38/65] fix spacing --- tests/unit/flash.tcl | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/unit/flash.tcl b/tests/unit/flash.tcl index c2aea0fe1..84399c6fc 100644 --- a/tests/unit/flash.tcl +++ b/tests/unit/flash.tcl @@ -140,7 +140,7 @@ if {$::flash_enabled} { r set $numkeys xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx incr numkeys if {[s used_memory]+1024 >= $limit} { - break + break } } # Add additional keys to force eviction @@ -150,7 +150,7 @@ if {$::flash_enabled} { set extra_keys [expr floor([expr ($limit * 0.4) / 1024])] for {set j 0} {$j < $extra_keys} {incr j} { catch { - r set p2$j xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx + r set p2$j xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx } err assert {$err == {OK}} } @@ -180,7 +180,7 @@ if {$::flash_enabled} { r set $numkeys xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx incr numkeys if {[s used_memory]+1024 >= $limit} { - break + break } } # Add additional keys to force eviction @@ -190,7 +190,7 @@ if {$::flash_enabled} { set extra_keys [expr floor([expr ($limit * 0.4) / 1024])] for {set j 0} {$j < $extra_keys} {incr j} { catch { - r set p2$j xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx + r set p2$j xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx } err assert {$err == {OK}} } From 98eaebd4c808c5180c0733fbc5921669330846f4 Mon Sep 17 00:00:00 2001 From: Malavan Sotheeswaran Date: Tue, 13 Jun 2023 22:52:26 -0700 Subject: [PATCH 39/65] accurate config update message --- src/config.cpp | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/src/config.cpp b/src/config.cpp index 78cdb3877..c36f73182 100644 --- a/src/config.cpp +++ b/src/config.cpp @@ -2585,6 +2585,19 @@ static int updateMaxmemory(long long val, long long prev, const char **err) { return 1; } +static int updateFlashMaxmemory(long long val, long long prev, const char **err) { + UNUSED(prev); + UNUSED(err); + if (val && g_pserver->m_pstorageFactory) { + size_t used = g_pserver->m_pstorageFactory->totalDiskspaceUsed(); + if ((unsigned long long)val < used) { + serverLog(LL_WARNING,"WARNING: the new maxstorage value set via CONFIG SET (%llu) is smaller than the current storage usage (%zu). This will result in key eviction and/or the inability to accept new write commands depending on the maxmemory-policy.", g_pserver->maxstorage, used); + } + performEvictions(false /*fPreSnapshot*/); + } + return 1; +} + static int updateGoodSlaves(long long val, long long prev, const char **err) { UNUSED(val); UNUSED(prev); @@ -2927,7 +2940,7 @@ standardConfig configs[] = { /* Unsigned Long Long configs */ createULongLongConfig("maxmemory", NULL, MODIFIABLE_CONFIG, 0, LLONG_MAX, g_pserver->maxmemory, 0, MEMORY_CONFIG, NULL, updateMaxmemory), - createULongLongConfig("maxstorage", NULL, MODIFIABLE_CONFIG, 0, LLONG_MAX, g_pserver->maxstorage, 0, MEMORY_CONFIG, NULL, NULL), + createULongLongConfig("maxstorage", NULL, MODIFIABLE_CONFIG, 0, LLONG_MAX, g_pserver->maxstorage, 0, MEMORY_CONFIG, NULL, updateFlashMaxmemory), /* Size_t configs */ createSizeTConfig("hash-max-ziplist-entries", NULL, MODIFIABLE_CONFIG, 0, LONG_MAX, g_pserver->hash_max_ziplist_entries, 512, INTEGER_CONFIG, NULL, NULL), From eea8932aa774ef93f5c09f4a0f8dfa05f5b8d3c5 Mon Sep 17 00:00:00 2001 From: Malavan Sotheeswaran Date: Tue, 13 Jun 2023 23:34:59 -0700 Subject: [PATCH 40/65] unit test doesn't make sense --- tests/unit/flash.tcl | 40 ---------------------------------------- 1 file changed, 40 deletions(-) diff --git a/tests/unit/flash.tcl b/tests/unit/flash.tcl index 84399c6fc..d1cfe8877 100644 --- a/tests/unit/flash.tcl +++ b/tests/unit/flash.tcl @@ -165,46 +165,6 @@ if {$::flash_enabled} { assert {[r get last] == {val}} r flushall } - - test "FLASH - is flash eviction working? (policy $policy)" { - # Get the current memory limit and calculate a new limit. - # Set limit to 100M. - set used [s used_memory] - set limit [expr {$used+60*1024*1024}] - r config set maxstorage $limit - r config set maxmemory-policy $policy - # Now add keys equivalent to 1024b until the limit is almost reached. - set numkeys 0 - r set first val - while 1 { - r set $numkeys xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx - incr numkeys - if {[s used_memory]+1024 >= $limit} { - break - } - } - # Add additional keys to force eviction - # should still be under the limit for maxmemory, however all keys set should still exist between flash and memory - # check same number of keys exist in addition to values of first and last keys - set err 0 - set extra_keys [expr floor([expr ($limit * 0.4) / 1024])] - for {set j 0} {$j < $extra_keys} {incr j} { - catch { - r set p2$j xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx - } err - assert {$err == {OK}} - } - if {[log_file_matches [srv 0 stdout] "*Failed to evict*"]} { - fail "Server did not evict cleanly (detected full flush)" - } - r set last val - set dbsize [r dbsize] - assert {[s used_memory] < ($limit*1.2)} - assert {$dbsize == $numkeys+$extra_keys+2} - assert {[r get first] == {val}} - assert {[r get last] == {val}} - r flushall - } } } } From e248fec27fde10c4262fabc3d1c3d53356a5c085 Mon Sep 17 00:00:00 2001 From: Malavan Sotheeswaran Date: Wed, 14 Jun 2023 14:20:17 -0700 Subject: [PATCH 41/65] for all keys we need to remove the prefix --- src/storage/rocksdb.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/storage/rocksdb.cpp b/src/storage/rocksdb.cpp index 6ed5cc5e1..183d420db 100644 --- a/src/storage/rocksdb.cpp +++ b/src/storage/rocksdb.cpp @@ -257,7 +257,7 @@ std::vector RocksDBStorageProvider::getEvictionCandidates() for (it->Seek(randomHashSlot()); it->Valid() && result.size() < 16; it->Next()) { if (FInternalKey(it->key().data(), it->key().size())) continue; - result.emplace_back(it->value().data(), it->value().size()); + result.emplace_back(it->key().data() + 2, it->key().size() - 2); } } else { std::unique_ptr it = std::unique_ptr(m_spdb->NewIterator(ReadOptions(), m_spexpirecolfamily.get())); From ec3b1eb5c4c0c2532755a92f286c074aeda1c2a2 Mon Sep 17 00:00:00 2001 From: Malavan Sotheeswaran Date: Wed, 14 Jun 2023 15:07:50 -0700 Subject: [PATCH 42/65] handle all keys case --- src/evict.cpp | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/evict.cpp b/src/evict.cpp index 9b9e1ed7b..7b9e4861e 100644 --- a/src/evict.cpp +++ b/src/evict.cpp @@ -688,9 +688,14 @@ int performEvictions(bool fPreSnapshot) { * frequency subtracting the actual frequency to the maximum * frequency of 255. */ idle = 255-LFUDecrAndReturn(obj); - } else if (g_pserver->maxmemory_policy == MAXMEMORY_VOLATILE_TTL && e != nullptr) { + } else if (g_pserver->maxmemory_policy == MAXMEMORY_VOLATILE_TTL) { /* In this case the sooner the expire the better. */ - idle = ULLONG_MAX - e->when(); + if (e != nullptr) + idle = ULLONG_MAX - e->when(); + else + continue; + } else if (g_pserver->maxmemory_policy & MAXMEMORY_FLAG_ALLKEYS) { + idle = ULLONG_MAX; } else { serverPanic("Unknown eviction policy in storage eviction"); } From f0d94665612d977015c545eef36e2e5863e77a35 Mon Sep 17 00:00:00 2001 From: Malavan Sotheeswaran Date: Wed, 14 Jun 2023 15:11:27 -0700 Subject: [PATCH 43/65] missing nullptr check --- src/evict.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/evict.cpp b/src/evict.cpp index 7b9e4861e..62d03860f 100644 --- a/src/evict.cpp +++ b/src/evict.cpp @@ -701,7 +701,8 @@ int performEvictions(bool fPreSnapshot) { } if (bestkey == nullptr || bestidle < idle) { - decrRefCount(bestkey); + if (bestkey != nullptr) + decrRefCount(bestkey); incrRefCount(keyobj); bestkey = keyobj; bestidle = idle; From c6871a3179c638773143d3cebaa01e7db2f26ae3 Mon Sep 17 00:00:00 2001 From: Malavan Sotheeswaran Date: Wed, 21 Jun 2023 16:36:47 -0700 Subject: [PATCH 44/65] run tests with flash --- machamp_scripts/build.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/machamp_scripts/build.sh b/machamp_scripts/build.sh index e05a9b3bd..a395f26ee 100755 --- a/machamp_scripts/build.sh +++ b/machamp_scripts/build.sh @@ -2,7 +2,7 @@ # make the build git submodule init && git submodule update -make BUILD_TLS=yes -j$(nproc) KEYDB_CFLAGS='-Werror' KEYDB_CXXFLAGS='-Werror' +make BUILD_TLS=yes ENABLE_FLASH=yes -j$(nproc) KEYDB_CFLAGS='-Werror' KEYDB_CXXFLAGS='-Werror' # gen-cert ./utils/gen-test-certs.sh \ No newline at end of file From 605f369d659aa9d79afd7d17f2af47b37f496283 Mon Sep 17 00:00:00 2001 From: Malavan Sotheeswaran Date: Wed, 21 Jun 2023 18:34:49 -0700 Subject: [PATCH 45/65] remove expires from storage provider when removed from memory --- src/IStorage.h | 1 + src/StorageCache.h | 1 + src/db.cpp | 2 ++ src/storage/rocksdb.cpp | 14 ++++++++++++++ src/storage/rocksdb.h | 1 + src/storage/teststorageprovider.h | 1 + 6 files changed, 20 insertions(+) diff --git a/src/IStorage.h b/src/IStorage.h index 4bd82333d..2da7e7aad 100644 --- a/src/IStorage.h +++ b/src/IStorage.h @@ -46,6 +46,7 @@ class IStorage virtual std::vector getExpirationCandidates() = 0; virtual std::vector getEvictionCandidates() = 0; virtual void setExpire(const char *key, size_t cchKey, long long expire) = 0; + virtual void removeExpire(const char *key, size_t cchKey, long long expire) = 0; virtual void beginWriteBatch() {} // NOP virtual void endWriteBatch() {} // NOP diff --git a/src/StorageCache.h b/src/StorageCache.h index 7dda1a30e..a17d6e0d0 100644 --- a/src/StorageCache.h +++ b/src/StorageCache.h @@ -54,6 +54,7 @@ class StorageCache std::vector getExpirationCandidates() { return m_spstorage->getExpirationCandidates(); } std::vector getEvictionCandidates() { return m_spstorage->getEvictionCandidates(); } void setExpire(const char *key, size_t cchKey, long long expire) { m_spstorage->setExpire(key, cchKey, expire); } + void removeExpire(const char *key, size_t cchKey, long long expire) { m_spstorage->removeExpire(key, cchKey, expire); } void beginWriteBatch(); void endWriteBatch() { m_spstorage->endWriteBatch(); } diff --git a/src/db.cpp b/src/db.cpp index 1351b0678..a432c37c6 100644 --- a/src/db.cpp +++ b/src/db.cpp @@ -1827,6 +1827,8 @@ int redisDbPersistentData::removeExpire(robj *key, dict_iter itr) { serverAssert(itrExpire != m_setexpire->end()); m_setexpire->erase(itrExpire); val->SetFExpires(false); + if (g_pserver->m_pstorageFactory != nullptr) + getStorageCache()->removeExpire(szFromObj(key), sdslen(szFromObj(key)), itrExpire->when()); return 1; } diff --git a/src/storage/rocksdb.cpp b/src/storage/rocksdb.cpp index 183d420db..6a0b6d306 100644 --- a/src/storage/rocksdb.cpp +++ b/src/storage/rocksdb.cpp @@ -225,6 +225,20 @@ void RocksDBStorageProvider::setExpire(const char *key, size_t cchKey, long long throw status.ToString(); } +void RocksDBStorageProvider::removeExpire(const char * key, size_t cchKey, long long expire) +{ + rocksdb::Status status; + std::unique_lock l(m_lock); + std::string prefix((const char *)&expire,sizeof(long long)); + std::string strKey(key, cchKey); + if (m_spbatch != nullptr) + status = m_spbatch->Delete(m_spexpirecolfamily.get(), rocksdb::Slice(prefix + strKey), rocksdb::Slice(strKey)); + else + status = m_spdb->Delete(WriteOptions(), m_spexpirecolfamily.get(), rocksdb::Slice(prefix + strKey), rocksdb::Slice(strKey)); + if (!status.ok()) + throw status.ToString(); +} + std::vector RocksDBStorageProvider::getExpirationCandidates() { std::vector result; diff --git a/src/storage/rocksdb.h b/src/storage/rocksdb.h index 9d8cf8ea9..0ccd43e15 100644 --- a/src/storage/rocksdb.h +++ b/src/storage/rocksdb.h @@ -39,6 +39,7 @@ class RocksDBStorageProvider : public IStorage virtual std::vector getExpirationCandidates() override; virtual std::vector getEvictionCandidates() override; virtual void setExpire(const char *key, size_t cchKey, long long expire) override; + virtual void removeExpire(const char *key, size_t cchKey, long long expire) override; virtual const IStorage *clone() const override; diff --git a/src/storage/teststorageprovider.h b/src/storage/teststorageprovider.h index 408d63b3f..6fee3651b 100644 --- a/src/storage/teststorageprovider.h +++ b/src/storage/teststorageprovider.h @@ -31,6 +31,7 @@ class TestStorageProvider final : public IStorage virtual std::vector getExpirationCandidates() override { return std::vector(); } virtual std::vector getEvictionCandidates() override { return std::vector(); } virtual void setExpire(const char *, size_t, long long) override {} + virtual void removeExpire(const char *, size_t, long long) override {} virtual void flush() override; From 967bd786581cdf2803862b26d1fb4509900949f5 Mon Sep 17 00:00:00 2001 From: Malavan Sotheeswaran Date: Wed, 21 Jun 2023 19:12:38 -0700 Subject: [PATCH 46/65] only delete if exists --- src/storage/rocksdb.cpp | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/storage/rocksdb.cpp b/src/storage/rocksdb.cpp index 6a0b6d306..d9bc13455 100644 --- a/src/storage/rocksdb.cpp +++ b/src/storage/rocksdb.cpp @@ -231,9 +231,11 @@ void RocksDBStorageProvider::removeExpire(const char * key, size_t cchKey, long std::unique_lock l(m_lock); std::string prefix((const char *)&expire,sizeof(long long)); std::string strKey(key, cchKey); - if (m_spbatch != nullptr) - status = m_spbatch->Delete(m_spexpirecolfamily.get(), rocksdb::Slice(prefix + strKey), rocksdb::Slice(strKey)); - else + rocksdb::PinnableSlice slice; + if (m_spbatch) + if (m_spbatch->GetFromBatchAndDB(m_spdb.get(), ReadOptions(), m_spexpirecolfamily.get(), rocksdb::Slice(prefix + strKey), &slice).ok()) + status = m_spbatch->Delete(m_spexpirecolfamily.get(), rocksdb::Slice(prefix + strKey), rocksdb::Slice(strKey)); + else if(m_spdb->Get(ReadOptions(), m_spexpirecolfamily.get(), rocksdb::Slice(prefix + strKey), &slice).ok()) status = m_spdb->Delete(WriteOptions(), m_spexpirecolfamily.get(), rocksdb::Slice(prefix + strKey), rocksdb::Slice(strKey)); if (!status.ok()) throw status.ToString(); From 0c0f8581f5ed43ec230beb6f6efde4aa0b0258c8 Mon Sep 17 00:00:00 2001 From: Malavan Sotheeswaran Date: Wed, 21 Jun 2023 19:15:11 -0700 Subject: [PATCH 47/65] brackets to avoid ambiguos else --- src/storage/rocksdb.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/storage/rocksdb.cpp b/src/storage/rocksdb.cpp index d9bc13455..3e9cbbc26 100644 --- a/src/storage/rocksdb.cpp +++ b/src/storage/rocksdb.cpp @@ -232,9 +232,10 @@ void RocksDBStorageProvider::removeExpire(const char * key, size_t cchKey, long std::string prefix((const char *)&expire,sizeof(long long)); std::string strKey(key, cchKey); rocksdb::PinnableSlice slice; - if (m_spbatch) + if (m_spbatch) { if (m_spbatch->GetFromBatchAndDB(m_spdb.get(), ReadOptions(), m_spexpirecolfamily.get(), rocksdb::Slice(prefix + strKey), &slice).ok()) status = m_spbatch->Delete(m_spexpirecolfamily.get(), rocksdb::Slice(prefix + strKey), rocksdb::Slice(strKey)); + } else if(m_spdb->Get(ReadOptions(), m_spexpirecolfamily.get(), rocksdb::Slice(prefix + strKey), &slice).ok()) status = m_spdb->Delete(WriteOptions(), m_spexpirecolfamily.get(), rocksdb::Slice(prefix + strKey), rocksdb::Slice(strKey)); if (!status.ok()) From 7a9d4bbc31838f40959bf3aa6ae16968477d9e6a Mon Sep 17 00:00:00 2001 From: Malavan Sotheeswaran Date: Fri, 23 Jun 2023 12:03:08 -0700 Subject: [PATCH 48/65] some refactoring to avoid duplicate code --- src/evict.cpp | 146 +++++++++++++++++----------------------- src/storage/rocksdb.cpp | 23 ++----- 2 files changed, 64 insertions(+), 105 deletions(-) diff --git a/src/evict.cpp b/src/evict.cpp index 62d03860f..0a15596d4 100644 --- a/src/evict.cpp +++ b/src/evict.cpp @@ -100,6 +100,36 @@ unsigned long long estimateObjectIdleTime(robj_roptr o) { } } +unsigned long long getIdle(robj *obj, const expireEntry *e) { + unsigned long long idle; + /* Calculate the idle time according to the policy. This is called + * idle just because the code initially handled LRU, but is in fact + * just a score where an higher score means better candidate. */ + if (g_pserver->maxmemory_policy & MAXMEMORY_FLAG_LRU) { + idle = (obj != nullptr) ? estimateObjectIdleTime(obj) : 0; + } else if (g_pserver->maxmemory_policy & MAXMEMORY_FLAG_LFU) { + /* When we use an LRU policy, we sort the keys by idle time + * so that we expire keys starting from greater idle time. + * However when the policy is an LFU one, we have a frequency + * estimation, and we want to evict keys with lower frequency + * first. So inside the pool we put objects using the inverted + * frequency subtracting the actual frequency to the maximum + * frequency of 255. */ + idle = 255-LFUDecrAndReturn(obj); + } else if (g_pserver->maxmemory_policy == MAXMEMORY_VOLATILE_TTL) { + /* In this case the sooner the expire the better. */ + if (e != nullptr) + idle = ULLONG_MAX - e->when(); + else + idle = 0; + } else if (g_pserver->maxmemory_policy & MAXMEMORY_FLAG_ALLKEYS) { + idle = ULLONG_MAX; + } else { + serverPanic("Unknown eviction policy in storage eviction"); + } + return idle; +} + /* LRU approximation algorithm * * Redis uses an approximation of the LRU algorithm that runs in constant @@ -137,28 +167,7 @@ void evictionPoolAlloc(void) { void processEvictionCandidate(int dbid, sds key, robj *o, const expireEntry *e, struct evictionPoolEntry *pool) { - unsigned long long idle; - - /* Calculate the idle time according to the policy. This is called - * idle just because the code initially handled LRU, but is in fact - * just a score where an higher score means better candidate. */ - if (g_pserver->maxmemory_policy & MAXMEMORY_FLAG_LRU) { - idle = (o != nullptr) ? estimateObjectIdleTime(o) : 0; - } else if (g_pserver->maxmemory_policy & MAXMEMORY_FLAG_LFU) { - /* When we use an LRU policy, we sort the keys by idle time - * so that we expire keys starting from greater idle time. - * However when the policy is an LFU one, we have a frequency - * estimation, and we want to evict keys with lower frequency - * first. So inside the pool we put objects using the inverted - * frequency subtracting the actual frequency to the maximum - * frequency of 255. */ - idle = 255-LFUDecrAndReturn(o); - } else if (g_pserver->maxmemory_policy == MAXMEMORY_VOLATILE_TTL) { - /* In this case the sooner the expire the better. */ - idle = ULLONG_MAX - e->when(); - } else { - serverPanic("Unknown eviction policy in evictionPoolPopulate()"); - } + unsigned long long idle = getIdle(o,e); /* Insert the element inside the pool. * First, find the first empty bucket or the first populated @@ -602,6 +611,31 @@ static unsigned long evictionTimeLimitUs() { return ULONG_MAX; /* No limit to eviction time */ } +void evict(redisDb *db, robj *keyobj) { + mstime_t eviction_latency; + propagateExpire(db,keyobj,g_pserver->lazyfree_lazy_eviction); + /* We compute the amount of memory freed by db*Delete() alone. + * It is possible that actually the memory needed to propagate + * the DEL in AOF and replication link is greater than the one + * we are freeing removing the key, but we can't account for + * that otherwise we would never exit the loop. + * + * AOF and Output buffer memory will be freed eventually so + * we only care about memory used by the key space. */ + latencyStartMonitor(eviction_latency); + if (g_pserver->lazyfree_lazy_eviction) + dbAsyncDelete(db,keyobj); + else + dbSyncDelete(db,keyobj); + latencyEndMonitor(eviction_latency); + latencyAddSampleIfNeeded("eviction-del",eviction_latency); + + signalModifiedKey(NULL,db,keyobj); + notifyKeyspaceEvent(NOTIFY_EVICTED, "evicted", + keyobj, db->id); + decrRefCount(keyobj); +} + /* Check that memory usage is within the current "maxmemory" limit. If over * "maxmemory", attempt to free memory by evicting data (if it's safe to do so). * @@ -633,7 +667,7 @@ int performEvictions(bool fPreSnapshot) { int keys_freed = 0; size_t mem_reported, mem_tofree; long long mem_freed; /* May be negative */ - mstime_t latency, eviction_latency; + mstime_t latency; long long delta; int slaves = listLength(g_pserver->slaves); const bool fEvictToStorage = !cserver.delete_on_evict && g_pserver->db[0]->FStorageProvider(); @@ -671,34 +705,8 @@ int performEvictions(bool fPreSnapshot) { robj *keyobj = createStringObject(key.c_str(), key.size()); robj *obj = db->find(szFromObj(keyobj)); if (obj != nullptr) { - unsigned long long idle; expireEntry *e = db->getExpire(keyobj); - - /* Calculate the idle time according to the policy. This is called - * idle just because the code initially handled LRU, but is in fact - * just a score where an higher score means better candidate. */ - if (g_pserver->maxmemory_policy & MAXMEMORY_FLAG_LRU) { - idle = (obj != nullptr) ? estimateObjectIdleTime(obj) : 0; - } else if (g_pserver->maxmemory_policy & MAXMEMORY_FLAG_LFU) { - /* When we use an LRU policy, we sort the keys by idle time - * so that we expire keys starting from greater idle time. - * However when the policy is an LFU one, we have a frequency - * estimation, and we want to evict keys with lower frequency - * first. So inside the pool we put objects using the inverted - * frequency subtracting the actual frequency to the maximum - * frequency of 255. */ - idle = 255-LFUDecrAndReturn(obj); - } else if (g_pserver->maxmemory_policy == MAXMEMORY_VOLATILE_TTL) { - /* In this case the sooner the expire the better. */ - if (e != nullptr) - idle = ULLONG_MAX - e->when(); - else - continue; - } else if (g_pserver->maxmemory_policy & MAXMEMORY_FLAG_ALLKEYS) { - idle = ULLONG_MAX; - } else { - serverPanic("Unknown eviction policy in storage eviction"); - } + unsigned long long idle = getIdle(obj, e); if (bestkey == nullptr || bestidle < idle) { if (bestkey != nullptr) @@ -713,22 +721,7 @@ int performEvictions(bool fPreSnapshot) { } } if (bestkey) { - propagateExpire(bestdb, bestkey, g_pserver->lazyfree_lazy_eviction); - - latencyStartMonitor(eviction_latency); - if (g_pserver->lazyfree_lazy_eviction) - dbAsyncDelete(bestdb, bestkey); - else - dbSyncDelete(bestdb, bestkey); - latencyEndMonitor(eviction_latency); - latencyAddSampleIfNeeded("eviction-del", eviction_latency); - - g_pserver->stat_evictedkeys++; - signalModifiedKey(NULL, bestdb, bestkey); - notifyKeyspaceEvent(NOTIFY_EVICTED, "evicted", - bestkey, bestdb->id); - - decrRefCount(bestkey); + evict(bestdb, bestkey); } else { break; //could not find a key to evict so stop now } @@ -861,30 +854,11 @@ int performEvictions(bool fPreSnapshot) { else { robj *keyobj = createStringObject(bestkey,sdslen(bestkey)); - propagateExpire(db,keyobj,g_pserver->lazyfree_lazy_eviction); - /* We compute the amount of memory freed by db*Delete() alone. - * It is possible that actually the memory needed to propagate - * the DEL in AOF and replication link is greater than the one - * we are freeing removing the key, but we can't account for - * that otherwise we would never exit the loop. - * - * AOF and Output buffer memory will be freed eventually so - * we only care about memory used by the key space. */ delta = (long long) zmalloc_used_memory(); - latencyStartMonitor(eviction_latency); - if (g_pserver->lazyfree_lazy_eviction) - dbAsyncDelete(db,keyobj); - else - dbSyncDelete(db,keyobj); - latencyEndMonitor(eviction_latency); - latencyAddSampleIfNeeded("eviction-del",eviction_latency); + evict(db, keyobj); delta -= (long long) zmalloc_used_memory(); mem_freed += delta; g_pserver->stat_evictedkeys++; - signalModifiedKey(NULL,db,keyobj); - notifyKeyspaceEvent(NOTIFY_EVICTED, "evicted", - keyobj, db->id); - decrRefCount(keyobj); } keys_freed++; diff --git a/src/storage/rocksdb.cpp b/src/storage/rocksdb.cpp index 3e9cbbc26..9efe39bc1 100644 --- a/src/storage/rocksdb.cpp +++ b/src/storage/rocksdb.cpp @@ -231,12 +231,9 @@ void RocksDBStorageProvider::removeExpire(const char * key, size_t cchKey, long std::unique_lock l(m_lock); std::string prefix((const char *)&expire,sizeof(long long)); std::string strKey(key, cchKey); - rocksdb::PinnableSlice slice; - if (m_spbatch) { - if (m_spbatch->GetFromBatchAndDB(m_spdb.get(), ReadOptions(), m_spexpirecolfamily.get(), rocksdb::Slice(prefix + strKey), &slice).ok()) - status = m_spbatch->Delete(m_spexpirecolfamily.get(), rocksdb::Slice(prefix + strKey), rocksdb::Slice(strKey)); - } - else if(m_spdb->Get(ReadOptions(), m_spexpirecolfamily.get(), rocksdb::Slice(prefix + strKey), &slice).ok()) + if (m_spbatch) + status = m_spbatch->Delete(m_spexpirecolfamily.get(), rocksdb::Slice(prefix + strKey), rocksdb::Slice(strKey)); + else status = m_spdb->Delete(WriteOptions(), m_spexpirecolfamily.get(), rocksdb::Slice(prefix + strKey), rocksdb::Slice(strKey)); if (!status.ok()) throw status.ToString(); @@ -251,13 +248,6 @@ std::vector RocksDBStorageProvider::getExpirationCandidates() if (FInternalKey(it->key().data(), it->key().size())) continue; result.emplace_back(it->value().data(), it->value().size()); - rocksdb::Status status; - if (m_spbatch != nullptr) - status = m_spbatch->Delete(m_spexpirecolfamily.get(), it->key()); - else - status = m_spdb->Delete(WriteOptions(), m_spexpirecolfamily.get(), it->key()); - if (!status.ok()) - throw "Failed to remove expired key from rocksdb"; } return result; } @@ -277,12 +267,7 @@ std::vector RocksDBStorageProvider::getEvictionCandidates() result.emplace_back(it->key().data() + 2, it->key().size() - 2); } } else { - std::unique_ptr it = std::unique_ptr(m_spdb->NewIterator(ReadOptions(), m_spexpirecolfamily.get())); - for (it->SeekToFirst(); it->Valid() && result.size() < 16; it->Next()) { - if (FInternalKey(it->key().data(), it->key().size())) - continue; - result.emplace_back(it->value().data(), it->value().size()); - } + return getExpirationCandidates(); } return result; } From 6504e7965a006a50acd325fd510514629de54d63 Mon Sep 17 00:00:00 2001 From: Malavan Sotheeswaran Date: Fri, 23 Jun 2023 12:21:47 -0700 Subject: [PATCH 49/65] bake expire modifications into storage cache insert and erase --- src/StorageCache.cpp | 35 +++++++++++++++++++++++++---------- src/db.cpp | 8 -------- 2 files changed, 25 insertions(+), 18 deletions(-) diff --git a/src/StorageCache.cpp b/src/StorageCache.cpp index 816765654..a955f8ed5 100644 --- a/src/StorageCache.cpp +++ b/src/StorageCache.cpp @@ -84,19 +84,31 @@ void StorageCache::cacheKey(const char *rgch, size_t cch) bool StorageCache::erase(sds key) { + unsigned long long when = 0; + m_spstorage->retrieve(key, sdslen(key), [&when, &key](const char *, size_t, const void * data, size_t cbdata) { + auto e = deserializeExpire(key, (const char *)data, cbdata, nullptr); + if (e != nullptr) + when = e->when(); + }); bool result = m_spstorage->erase(key, sdslen(key)); std::unique_lock ul(m_lock); - if (result && m_pdict != nullptr) + if (result) { - uint64_t hash = dictSdsHash(key); - dictEntry *de = dictFind(m_pdict, reinterpret_cast(hash)); - serverAssert(de != nullptr); - de->v.s64--; - serverAssert(de->v.s64 >= 0); - if (de->v.s64 == 0) { - dictDelete(m_pdict, reinterpret_cast(hash)); - } else { - m_collisionCount--; + if (m_pdict != nullptr) + { + uint64_t hash = dictSdsHash(key); + dictEntry *de = dictFind(m_pdict, reinterpret_cast(hash)); + serverAssert(de != nullptr); + de->v.s64--; + serverAssert(de->v.s64 >= 0); + if (de->v.s64 == 0) { + dictDelete(m_pdict, reinterpret_cast(hash)); + } else { + m_collisionCount--; + } + } + if (when != 0) { + m_spstorage->removeExpire(key, sdslen(key), when); } } return result; @@ -111,6 +123,9 @@ void StorageCache::insert(sds key, const void *data, size_t cbdata, bool fOverwr } ul.unlock(); m_spstorage->insert(key, sdslen(key), (void*)data, cbdata, fOverwrite); + auto e = deserializeExpire(key, (const char *)data, cbdata, nullptr); + if (e != nullptr) + m_spstorage->setExpire(key, sdslen(key), e->when()); } long _dictKeyIndex(dict *d, const void *key, uint64_t hash, dictEntry **existing); diff --git a/src/db.cpp b/src/db.cpp index a432c37c6..6136fa963 100644 --- a/src/db.cpp +++ b/src/db.cpp @@ -1827,8 +1827,6 @@ int redisDbPersistentData::removeExpire(robj *key, dict_iter itr) { serverAssert(itrExpire != m_setexpire->end()); m_setexpire->erase(itrExpire); val->SetFExpires(false); - if (g_pserver->m_pstorageFactory != nullptr) - getStorageCache()->removeExpire(szFromObj(key), sdslen(szFromObj(key)), itrExpire->when()); return 1; } @@ -1873,8 +1871,6 @@ void redisDbPersistentData::resortExpire(expireEntry &e) expireEntry eT = std::move(e); m_setexpire->erase(itr); m_setexpire->insert(eT); - if (g_pserver->m_pstorageFactory != nullptr) - getStorageCache()->setExpire(e.key(), sdslen(e.key()), e.when()); } /* Set an expire to the specified key. If the expire is set in the context @@ -2805,8 +2801,6 @@ void redisDbPersistentData::setExpire(robj *key, robj *subkey, long long when) ((robj*)dictGetVal(kde))->SetFExpires(true); m_setexpire->insert(e); } - if (g_pserver->m_pstorageFactory != nullptr) - getStorageCache()->setExpire(szFromObj(key), sdslen(szFromObj(key)), when); } void redisDbPersistentData::setExpire(expireEntry &&e) @@ -2814,8 +2808,6 @@ void redisDbPersistentData::setExpire(expireEntry &&e) std::unique_lock ul(g_expireLock); trackkey(e.key(), true /* fUpdate */); m_setexpire->insert(e); - if (g_pserver->m_pstorageFactory != nullptr) - getStorageCache()->setExpire(e.key(), sdslen(e.key()), e.when()); } bool redisDb::FKeyExpires(const char *key) From 178df4ddc6853356b4b39b30899e4d636f64e3ea Mon Sep 17 00:00:00 2001 From: Malavan Sotheeswaran Date: Fri, 23 Jun 2023 16:32:50 -0700 Subject: [PATCH 50/65] respect sample count --- src/IStorage.h | 4 ++-- src/StorageCache.h | 4 ++-- src/evict.cpp | 2 +- src/expire.cpp | 2 +- src/storage/rocksdb.cpp | 10 +++++----- src/storage/rocksdb.h | 4 ++-- src/storage/teststorageprovider.h | 4 ++-- 7 files changed, 15 insertions(+), 15 deletions(-) diff --git a/src/IStorage.h b/src/IStorage.h index 2da7e7aad..377976498 100644 --- a/src/IStorage.h +++ b/src/IStorage.h @@ -43,8 +43,8 @@ class IStorage endWriteBatch(); } - virtual std::vector getExpirationCandidates() = 0; - virtual std::vector getEvictionCandidates() = 0; + virtual std::vector getExpirationCandidates(int count) = 0; + virtual std::vector getEvictionCandidates(int count) = 0; virtual void setExpire(const char *key, size_t cchKey, long long expire) = 0; virtual void removeExpire(const char *key, size_t cchKey, long long expire) = 0; diff --git a/src/StorageCache.h b/src/StorageCache.h index a17d6e0d0..21005e5b5 100644 --- a/src/StorageCache.h +++ b/src/StorageCache.h @@ -51,8 +51,8 @@ class StorageCache bool enumerate(IStorage::callback fn) const { return m_spstorage->enumerate(fn); } bool enumerate_hashslot(IStorage::callback fn, unsigned int hashslot) const { return m_spstorage->enumerate_hashslot(fn, hashslot); } - std::vector getExpirationCandidates() { return m_spstorage->getExpirationCandidates(); } - std::vector getEvictionCandidates() { return m_spstorage->getEvictionCandidates(); } + std::vector getExpirationCandidates(int count) { return m_spstorage->getExpirationCandidates(count); } + std::vector getEvictionCandidates(int count) { return m_spstorage->getEvictionCandidates(count); } void setExpire(const char *key, size_t cchKey, long long expire) { m_spstorage->setExpire(key, cchKey, expire); } void removeExpire(const char *key, size_t cchKey, long long expire) { m_spstorage->removeExpire(key, cchKey, expire); } diff --git a/src/evict.cpp b/src/evict.cpp index 0a15596d4..042d48150 100644 --- a/src/evict.cpp +++ b/src/evict.cpp @@ -700,7 +700,7 @@ int performEvictions(bool fPreSnapshot) { unsigned long long bestidle = 0; for (int i = 0; i < cserver.dbnum; i++) { db = g_pserver->db[i]; - evictionPool = db->getStorageCache()->getEvictionCandidates(); + evictionPool = db->getStorageCache()->getEvictionCandidates(g_pserver->maxmemory_samples); for (std::string key : evictionPool) { robj *keyobj = createStringObject(key.c_str(), key.size()); robj *obj = db->find(szFromObj(keyobj)); diff --git a/src/expire.cpp b/src/expire.cpp index fb386121f..5ff9ee6f7 100644 --- a/src/expire.cpp +++ b/src/expire.cpp @@ -419,7 +419,7 @@ void activeExpireCycleCore(int type) { } else { std::vector keys; do { - keys = db->getStorageCache()->getExpirationCandidates(); + keys = db->getStorageCache()->getExpirationCandidates(ACTIVE_EXPIRE_CYCLE_LOOKUPS_PER_LOOP); for (std::string key : keys) { robj* keyobj = createStringObject(key.c_str(), key.size()); db->find(szFromObj(keyobj)); diff --git a/src/storage/rocksdb.cpp b/src/storage/rocksdb.cpp index 9efe39bc1..61ff0b72c 100644 --- a/src/storage/rocksdb.cpp +++ b/src/storage/rocksdb.cpp @@ -239,12 +239,12 @@ void RocksDBStorageProvider::removeExpire(const char * key, size_t cchKey, long throw status.ToString(); } -std::vector RocksDBStorageProvider::getExpirationCandidates() +std::vector RocksDBStorageProvider::getExpirationCandidates(int count) { std::vector result; std::unique_ptr it = std::unique_ptr(m_spdb->NewIterator(ReadOptions(), m_spexpirecolfamily.get())); long long curTime = mstime(); - for (it->SeekToFirst(); it->Valid() && (*((long long *)it->key().data()) <= curTime) && (result.size() < 16); it->Next()) { + for (it->SeekToFirst(); it->Valid() && (*((long long *)it->key().data()) <= curTime) && (result.size() < count); it->Next()) { if (FInternalKey(it->key().data(), it->key().size())) continue; result.emplace_back(it->value().data(), it->value().size()); @@ -256,18 +256,18 @@ std::string randomHashSlot() { return getPrefix(genrand64_int63() % (1 << 16)); } -std::vector RocksDBStorageProvider::getEvictionCandidates() +std::vector RocksDBStorageProvider::getEvictionCandidates(int count) { std::vector result; if (g_pserver->maxmemory_policy & MAXMEMORY_FLAG_ALLKEYS) { std::unique_ptr it = std::unique_ptr(m_spdb->NewIterator(ReadOptions(), m_spcolfamily.get())); - for (it->Seek(randomHashSlot()); it->Valid() && result.size() < 16; it->Next()) { + for (it->Seek(randomHashSlot()); it->Valid() && result.size() < count; it->Next()) { if (FInternalKey(it->key().data(), it->key().size())) continue; result.emplace_back(it->key().data() + 2, it->key().size() - 2); } } else { - return getExpirationCandidates(); + return getExpirationCandidates(count); } return result; } diff --git a/src/storage/rocksdb.h b/src/storage/rocksdb.h index 0ccd43e15..2a84ff96d 100644 --- a/src/storage/rocksdb.h +++ b/src/storage/rocksdb.h @@ -36,8 +36,8 @@ class RocksDBStorageProvider : public IStorage virtual bool enumerate(callback fn) const override; virtual bool enumerate_hashslot(callback fn, unsigned int hashslot) const override; - virtual std::vector getExpirationCandidates() override; - virtual std::vector getEvictionCandidates() override; + virtual std::vector getExpirationCandidates(int count) override; + virtual std::vector getEvictionCandidates(int count) override; virtual void setExpire(const char *key, size_t cchKey, long long expire) override; virtual void removeExpire(const char *key, size_t cchKey, long long expire) override; diff --git a/src/storage/teststorageprovider.h b/src/storage/teststorageprovider.h index 6fee3651b..44c0078ee 100644 --- a/src/storage/teststorageprovider.h +++ b/src/storage/teststorageprovider.h @@ -28,8 +28,8 @@ class TestStorageProvider final : public IStorage virtual bool enumerate_hashslot(callback fn, unsigned int hashslot) const override; virtual size_t count() const override; - virtual std::vector getExpirationCandidates() override { return std::vector(); } - virtual std::vector getEvictionCandidates() override { return std::vector(); } + virtual std::vector getExpirationCandidates(int) override { return std::vector(); } + virtual std::vector getEvictionCandidates(int) override { return std::vector(); } virtual void setExpire(const char *, size_t, long long) override {} virtual void removeExpire(const char *, size_t, long long) override {} From 79b3490f49663d53d9588742d70dc83ecd8de447 Mon Sep 17 00:00:00 2001 From: Malavan Sotheeswaran Date: Fri, 23 Jun 2023 16:52:46 -0700 Subject: [PATCH 51/65] fix warnings --- src/IStorage.h | 4 ++-- src/StorageCache.h | 4 ++-- src/expire.cpp | 3 +-- src/storage/rocksdb.cpp | 21 ++++++++++++++++----- src/storage/rocksdb.h | 5 +++-- src/storage/teststorageprovider.h | 4 ++-- 6 files changed, 26 insertions(+), 15 deletions(-) diff --git a/src/IStorage.h b/src/IStorage.h index 377976498..adc04e968 100644 --- a/src/IStorage.h +++ b/src/IStorage.h @@ -43,8 +43,8 @@ class IStorage endWriteBatch(); } - virtual std::vector getExpirationCandidates(int count) = 0; - virtual std::vector getEvictionCandidates(int count) = 0; + virtual std::vector getExpirationCandidates(unsigned int count) = 0; + virtual std::vector getEvictionCandidates(unsigned int count) = 0; virtual void setExpire(const char *key, size_t cchKey, long long expire) = 0; virtual void removeExpire(const char *key, size_t cchKey, long long expire) = 0; diff --git a/src/StorageCache.h b/src/StorageCache.h index 21005e5b5..f594bfbee 100644 --- a/src/StorageCache.h +++ b/src/StorageCache.h @@ -51,8 +51,8 @@ class StorageCache bool enumerate(IStorage::callback fn) const { return m_spstorage->enumerate(fn); } bool enumerate_hashslot(IStorage::callback fn, unsigned int hashslot) const { return m_spstorage->enumerate_hashslot(fn, hashslot); } - std::vector getExpirationCandidates(int count) { return m_spstorage->getExpirationCandidates(count); } - std::vector getEvictionCandidates(int count) { return m_spstorage->getEvictionCandidates(count); } + std::vector getExpirationCandidates(unsigned int count) { return m_spstorage->getExpirationCandidates(count); } + std::vector getEvictionCandidates(unsigned int count) { return m_spstorage->getEvictionCandidates(count); } void setExpire(const char *key, size_t cchKey, long long expire) { m_spstorage->setExpire(key, cchKey, expire); } void removeExpire(const char *key, size_t cchKey, long long expire) { m_spstorage->removeExpire(key, cchKey, expire); } diff --git a/src/expire.cpp b/src/expire.cpp index 5ff9ee6f7..91f4d52b4 100644 --- a/src/expire.cpp +++ b/src/expire.cpp @@ -325,7 +325,7 @@ void activeExpireCycleCore(int type) { static int timelimit_exit = 0; /* Time limit hit in previous call? */ static long long last_fast_cycle = 0; /* When last fast cycle ran. */ - int j, iteration = 0; + int j; int dbs_per_call = CRON_DBS_PER_CALL; long long start = ustime(), timelimit, elapsed; @@ -379,7 +379,6 @@ void activeExpireCycleCore(int type) { current_db++; long long now; - iteration++; now = mstime(); /* If there is nothing to expire try next DB ASAP. */ diff --git a/src/storage/rocksdb.cpp b/src/storage/rocksdb.cpp index 61ff0b72c..70f973bd2 100644 --- a/src/storage/rocksdb.cpp +++ b/src/storage/rocksdb.cpp @@ -225,21 +225,24 @@ void RocksDBStorageProvider::setExpire(const char *key, size_t cchKey, long long throw status.ToString(); } -void RocksDBStorageProvider::removeExpire(const char * key, size_t cchKey, long long expire) +void RocksDBStorageProvider::removeExpire(const char *key, size_t cchKey, long long expire) { rocksdb::Status status; std::unique_lock l(m_lock); std::string prefix((const char *)&expire,sizeof(long long)); std::string strKey(key, cchKey); + std::string fullKey = prefix + strKey; + if (!FExpireExists(fullKey)) + return; if (m_spbatch) - status = m_spbatch->Delete(m_spexpirecolfamily.get(), rocksdb::Slice(prefix + strKey), rocksdb::Slice(strKey)); + status = m_spbatch->Delete(m_spexpirecolfamily.get(), rocksdb::Slice(fullKey), rocksdb::Slice(strKey)); else - status = m_spdb->Delete(WriteOptions(), m_spexpirecolfamily.get(), rocksdb::Slice(prefix + strKey), rocksdb::Slice(strKey)); + status = m_spdb->Delete(WriteOptions(), m_spexpirecolfamily.get(), rocksdb::Slice(fullKey), rocksdb::Slice(strKey)); if (!status.ok()) throw status.ToString(); } -std::vector RocksDBStorageProvider::getExpirationCandidates(int count) +std::vector RocksDBStorageProvider::getExpirationCandidates(unsigned int count) { std::vector result; std::unique_ptr it = std::unique_ptr(m_spdb->NewIterator(ReadOptions(), m_spexpirecolfamily.get())); @@ -256,7 +259,7 @@ std::string randomHashSlot() { return getPrefix(genrand64_int63() % (1 << 16)); } -std::vector RocksDBStorageProvider::getEvictionCandidates(int count) +std::vector RocksDBStorageProvider::getEvictionCandidates(unsigned int count) { std::vector result; if (g_pserver->maxmemory_policy & MAXMEMORY_FLAG_ALLKEYS) { @@ -337,4 +340,12 @@ bool RocksDBStorageProvider::FKeyExists(std::string& key) const if (m_spbatch) return m_spbatch->GetFromBatchAndDB(m_spdb.get(), ReadOptions(), m_spcolfamily.get(), rocksdb::Slice(key), &slice).ok(); return m_spdb->Get(ReadOptions(), m_spcolfamily.get(), rocksdb::Slice(key), &slice).ok(); +} + +bool RocksDBStorageProvider::FExpireExists(std::string& key) const +{ + rocksdb::PinnableSlice slice; + if (m_spbatch) + return m_spbatch->GetFromBatchAndDB(m_spdb.get(), ReadOptions(), m_spexpirecolfamily.get(), rocksdb::Slice(key), &slice).ok(); + return m_spdb->Get(ReadOptions(), m_spexpirecolfamily.get(), rocksdb::Slice(key), &slice).ok(); } \ No newline at end of file diff --git a/src/storage/rocksdb.h b/src/storage/rocksdb.h index 2a84ff96d..dd6196a55 100644 --- a/src/storage/rocksdb.h +++ b/src/storage/rocksdb.h @@ -36,8 +36,8 @@ class RocksDBStorageProvider : public IStorage virtual bool enumerate(callback fn) const override; virtual bool enumerate_hashslot(callback fn, unsigned int hashslot) const override; - virtual std::vector getExpirationCandidates(int count) override; - virtual std::vector getEvictionCandidates(int count) override; + virtual std::vector getExpirationCandidates(unsigned int count) override; + virtual std::vector getEvictionCandidates(unsigned int count) override; virtual void setExpire(const char *key, size_t cchKey, long long expire) override; virtual void removeExpire(const char *key, size_t cchKey, long long expire) override; @@ -57,6 +57,7 @@ class RocksDBStorageProvider : public IStorage protected: bool FKeyExists(std::string&) const; + bool FExpireExists(std::string&) const; const rocksdb::ReadOptions &ReadOptions() const { return m_readOptionsTemplate; } rocksdb::WriteOptions WriteOptions() const; diff --git a/src/storage/teststorageprovider.h b/src/storage/teststorageprovider.h index 44c0078ee..db41bbf22 100644 --- a/src/storage/teststorageprovider.h +++ b/src/storage/teststorageprovider.h @@ -28,8 +28,8 @@ class TestStorageProvider final : public IStorage virtual bool enumerate_hashslot(callback fn, unsigned int hashslot) const override; virtual size_t count() const override; - virtual std::vector getExpirationCandidates(int) override { return std::vector(); } - virtual std::vector getEvictionCandidates(int) override { return std::vector(); } + virtual std::vector getExpirationCandidates(unsigned int) override { return std::vector(); } + virtual std::vector getEvictionCandidates(unsigned int) override { return std::vector(); } virtual void setExpire(const char *, size_t, long long) override {} virtual void removeExpire(const char *, size_t, long long) override {} From a0a264a11c3c114930c4699e9579d0bdd98fd8ca Mon Sep 17 00:00:00 2001 From: Malavan Sotheeswaran Date: Fri, 23 Jun 2023 18:20:36 -0700 Subject: [PATCH 52/65] figured out the bug! --- src/storage/rocksdb.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/storage/rocksdb.cpp b/src/storage/rocksdb.cpp index 70f973bd2..b2d1775f6 100644 --- a/src/storage/rocksdb.cpp +++ b/src/storage/rocksdb.cpp @@ -235,9 +235,9 @@ void RocksDBStorageProvider::removeExpire(const char *key, size_t cchKey, long l if (!FExpireExists(fullKey)) return; if (m_spbatch) - status = m_spbatch->Delete(m_spexpirecolfamily.get(), rocksdb::Slice(fullKey), rocksdb::Slice(strKey)); + status = m_spbatch->Delete(m_spexpirecolfamily.get(), rocksdb::Slice(fullKey)); else - status = m_spdb->Delete(WriteOptions(), m_spexpirecolfamily.get(), rocksdb::Slice(fullKey), rocksdb::Slice(strKey)); + status = m_spdb->Delete(WriteOptions(), m_spexpirecolfamily.get(), rocksdb::Slice(fullKey)); if (!status.ok()) throw status.ToString(); } From 0eb118731e2cdf6a2b0204206bc432e855962e26 Mon Sep 17 00:00:00 2001 From: Malavan Sotheeswaran Date: Fri, 23 Jun 2023 18:31:19 -0700 Subject: [PATCH 53/65] fix subkey exprires --- src/db.cpp | 2 ++ src/storage/rocksdb.cpp | 14 +++++++++++++- 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/src/db.cpp b/src/db.cpp index 6136fa963..99e716ba1 100644 --- a/src/db.cpp +++ b/src/db.cpp @@ -1871,6 +1871,8 @@ void redisDbPersistentData::resortExpire(expireEntry &e) expireEntry eT = std::move(e); m_setexpire->erase(itr); m_setexpire->insert(eT); + if (g_pserver->m_pstorageFactory != nullptr) + getStorageCache()->setExpire(eT.key(), sdslen(eT.key()), eT.when()); } /* Set an expire to the specified key. If the expire is set in the context diff --git a/src/storage/rocksdb.cpp b/src/storage/rocksdb.cpp index b2d1775f6..6ed2fd04f 100644 --- a/src/storage/rocksdb.cpp +++ b/src/storage/rocksdb.cpp @@ -251,6 +251,13 @@ std::vector RocksDBStorageProvider::getExpirationCandidates(unsigne if (FInternalKey(it->key().data(), it->key().size())) continue; result.emplace_back(it->value().data(), it->value().size()); + rocksdb::Status status; + if (m_spbatch) + status = m_spbatch->Delete(m_spexpirecolfamily.get(), it->key()); + else + status = m_spdb->Delete(WriteOptions(), m_spexpirecolfamily.get(), it->key()); + if (!status.ok()) + throw status.ToString(); } return result; } @@ -270,7 +277,12 @@ std::vector RocksDBStorageProvider::getEvictionCandidates(unsigned result.emplace_back(it->key().data() + 2, it->key().size() - 2); } } else { - return getExpirationCandidates(count); + std::unique_ptr it = std::unique_ptr(m_spdb->NewIterator(ReadOptions(), m_spexpirecolfamily.get())); + for (it->SeekToFirst(); it->Valid() && result.size() < 16; it->Next()) { + if (FInternalKey(it->key().data(), it->key().size())) + continue; + result.emplace_back(it->value().data(), it->value().size()); + } } return result; } From eb2018a4233eda55372e2100f216b1c7cadf3dfb Mon Sep 17 00:00:00 2001 From: Malavan Sotheeswaran Date: Fri, 23 Jun 2023 20:33:45 -0700 Subject: [PATCH 54/65] reject versions before 6.3.4 --- src/server.cpp | 3 +++ src/storage/rocksdbfactory.cpp | 2 ++ src/version.h | 1 + 3 files changed, 6 insertions(+) diff --git a/src/server.cpp b/src/server.cpp index 70b8b4374..c8b37a7f7 100644 --- a/src/server.cpp +++ b/src/server.cpp @@ -1870,6 +1870,9 @@ VersionCompareResult compareVersion(SymVer *pver) if ((symVerThis.major == 0 && symVerThis.minor == 0 && symVerThis.build == 0) || (pver->major == 0 && pver->minor == 0 && pver->build == 0)) return VersionCompareResult::EqualVersion; + + if (pver->major <= 6 && pver->minor <= 3 && pver->build <= 3) + return VersionCompareResult::IncompatibleVersion; for (int iver = 0; iver < 3; ++iver) { diff --git a/src/storage/rocksdbfactory.cpp b/src/storage/rocksdbfactory.cpp index fb5b28ac8..cd3188909 100644 --- a/src/storage/rocksdbfactory.cpp +++ b/src/storage/rocksdbfactory.cpp @@ -115,6 +115,8 @@ RocksDBStorageFactory::RocksDBStorageFactory(const char *dbfile, int dbnum, cons auto cmp = compareVersion(&ver); if (cmp == NewerVersion) throw "Cannot load FLASH database created by newer version of KeyDB"; + if (cmp == IncompatibleVersion) + throw "Cannot load FLASH database from before 6.3.4"; if (cmp == OlderVersion) setVersion(handle); } diff --git a/src/version.h b/src/version.h index a747c45a6..26c498813 100644 --- a/src/version.h +++ b/src/version.h @@ -7,6 +7,7 @@ enum VersionCompareResult EqualVersion, OlderVersion, NewerVersion, + IncompatibleVersion, }; struct SymVer From ea7838eb46d8670c16eb4c1726711ec9c4e2b10e Mon Sep 17 00:00:00 2001 From: Malavan Sotheeswaran Date: Mon, 26 Jun 2023 21:35:09 -0700 Subject: [PATCH 55/65] fix test --- src/IStorage.h | 2 +- src/StorageCache.h | 2 +- src/db.cpp | 2 -- src/expire.cpp | 6 +++--- src/storage/rocksdb.cpp | 5 ++--- src/storage/rocksdb.h | 2 +- src/storage/teststorageprovider.h | 2 +- tests/unit/flash.tcl | 2 +- 8 files changed, 10 insertions(+), 13 deletions(-) diff --git a/src/IStorage.h b/src/IStorage.h index adc04e968..7698b491a 100644 --- a/src/IStorage.h +++ b/src/IStorage.h @@ -43,7 +43,7 @@ class IStorage endWriteBatch(); } - virtual std::vector getExpirationCandidates(unsigned int count) = 0; + virtual std::vector getExpirationCandidates(unsigned int count, long long time) = 0; virtual std::vector getEvictionCandidates(unsigned int count) = 0; virtual void setExpire(const char *key, size_t cchKey, long long expire) = 0; virtual void removeExpire(const char *key, size_t cchKey, long long expire) = 0; diff --git a/src/StorageCache.h b/src/StorageCache.h index f594bfbee..cf0295785 100644 --- a/src/StorageCache.h +++ b/src/StorageCache.h @@ -51,7 +51,7 @@ class StorageCache bool enumerate(IStorage::callback fn) const { return m_spstorage->enumerate(fn); } bool enumerate_hashslot(IStorage::callback fn, unsigned int hashslot) const { return m_spstorage->enumerate_hashslot(fn, hashslot); } - std::vector getExpirationCandidates(unsigned int count) { return m_spstorage->getExpirationCandidates(count); } + std::vector getExpirationCandidates(unsigned int count, long long time) { return m_spstorage->getExpirationCandidates(count, time); } std::vector getEvictionCandidates(unsigned int count) { return m_spstorage->getEvictionCandidates(count); } void setExpire(const char *key, size_t cchKey, long long expire) { m_spstorage->setExpire(key, cchKey, expire); } void removeExpire(const char *key, size_t cchKey, long long expire) { m_spstorage->removeExpire(key, cchKey, expire); } diff --git a/src/db.cpp b/src/db.cpp index 99e716ba1..6136fa963 100644 --- a/src/db.cpp +++ b/src/db.cpp @@ -1871,8 +1871,6 @@ void redisDbPersistentData::resortExpire(expireEntry &e) expireEntry eT = std::move(e); m_setexpire->erase(itr); m_setexpire->insert(eT); - if (g_pserver->m_pstorageFactory != nullptr) - getStorageCache()->setExpire(eT.key(), sdslen(eT.key()), eT.when()); } /* Set an expire to the specified key. If the expire is set in the context diff --git a/src/expire.cpp b/src/expire.cpp index 91f4d52b4..cd56ec7a5 100644 --- a/src/expire.cpp +++ b/src/expire.cpp @@ -418,13 +418,13 @@ void activeExpireCycleCore(int type) { } else { std::vector keys; do { - keys = db->getStorageCache()->getExpirationCandidates(ACTIVE_EXPIRE_CYCLE_LOOKUPS_PER_LOOP); + keys = db->getStorageCache()->getExpirationCandidates(ACTIVE_EXPIRE_CYCLE_LOOKUPS_PER_LOOP, now); for (std::string key : keys) { robj* keyobj = createStringObject(key.c_str(), key.size()); db->find(szFromObj(keyobj)); expireEntry *e = db->getExpire(keyobj); - if (e != nullptr) - expired += activeExpireCycleExpire(db, *e, mstime(), tried); + if (e != nullptr && e->when() < now) + expired += activeExpireCycleExpire(db, *e, now, tried); decrRefCount(keyobj); } elapsed = ustime()-start; diff --git a/src/storage/rocksdb.cpp b/src/storage/rocksdb.cpp index 6ed2fd04f..8ac413c97 100644 --- a/src/storage/rocksdb.cpp +++ b/src/storage/rocksdb.cpp @@ -242,12 +242,11 @@ void RocksDBStorageProvider::removeExpire(const char *key, size_t cchKey, long l throw status.ToString(); } -std::vector RocksDBStorageProvider::getExpirationCandidates(unsigned int count) +std::vector RocksDBStorageProvider::getExpirationCandidates(unsigned int count, long long time) { std::vector result; std::unique_ptr it = std::unique_ptr(m_spdb->NewIterator(ReadOptions(), m_spexpirecolfamily.get())); - long long curTime = mstime(); - for (it->SeekToFirst(); it->Valid() && (*((long long *)it->key().data()) <= curTime) && (result.size() < count); it->Next()) { + for (it->SeekToFirst(); it->Valid() && (*((long long *)it->key().data()) <= time) && (result.size() < count); it->Next()) { if (FInternalKey(it->key().data(), it->key().size())) continue; result.emplace_back(it->value().data(), it->value().size()); diff --git a/src/storage/rocksdb.h b/src/storage/rocksdb.h index dd6196a55..ecd8e0ccc 100644 --- a/src/storage/rocksdb.h +++ b/src/storage/rocksdb.h @@ -36,7 +36,7 @@ class RocksDBStorageProvider : public IStorage virtual bool enumerate(callback fn) const override; virtual bool enumerate_hashslot(callback fn, unsigned int hashslot) const override; - virtual std::vector getExpirationCandidates(unsigned int count) override; + virtual std::vector getExpirationCandidates(unsigned int count, long long time) override; virtual std::vector getEvictionCandidates(unsigned int count) override; virtual void setExpire(const char *key, size_t cchKey, long long expire) override; virtual void removeExpire(const char *key, size_t cchKey, long long expire) override; diff --git a/src/storage/teststorageprovider.h b/src/storage/teststorageprovider.h index db41bbf22..a86013859 100644 --- a/src/storage/teststorageprovider.h +++ b/src/storage/teststorageprovider.h @@ -28,7 +28,7 @@ class TestStorageProvider final : public IStorage virtual bool enumerate_hashslot(callback fn, unsigned int hashslot) const override; virtual size_t count() const override; - virtual std::vector getExpirationCandidates(unsigned int) override { return std::vector(); } + virtual std::vector getExpirationCandidates(unsigned int, long long) override { return std::vector(); } virtual std::vector getEvictionCandidates(unsigned int) override { return std::vector(); } virtual void setExpire(const char *, size_t, long long) override {} virtual void removeExpire(const char *, size_t, long long) override {} diff --git a/tests/unit/flash.tcl b/tests/unit/flash.tcl index d1cfe8877..25d50387d 100644 --- a/tests/unit/flash.tcl +++ b/tests/unit/flash.tcl @@ -74,7 +74,7 @@ if {$::flash_enabled} { r set testkey foo ex 1 r flushall cache assert_equal {1} [r dbsize] - after 1500 + after 2000 assert_equal {0} [r dbsize] } From 7f86f4f876de75598fef84a659b2449c96fa49fd Mon Sep 17 00:00:00 2001 From: Malavan Sotheeswaran Date: Mon, 26 Jun 2023 22:06:38 -0700 Subject: [PATCH 56/65] WIP flash eviction tests --- src/StorageCache.h | 2 ++ src/debug.cpp | 8 ++++++++ tests/unit/flash.tcl | 43 ++++++++++++++++++++++++++++++++++++++++++- 3 files changed, 52 insertions(+), 1 deletion(-) diff --git a/src/StorageCache.h b/src/StorageCache.h index cf0295785..b3bc57218 100644 --- a/src/StorageCache.h +++ b/src/StorageCache.h @@ -61,6 +61,8 @@ class StorageCache void batch_lock() { return m_spstorage->batch_lock(); } void batch_unlock() { return m_spstorage->batch_unlock(); } + void flush() { m_spstorage->flush(); } + size_t count() const; const StorageCache *clone(); diff --git a/src/debug.cpp b/src/debug.cpp index 688ca3a0e..6ff1973b2 100644 --- a/src/debug.cpp +++ b/src/debug.cpp @@ -937,6 +937,14 @@ NULL mallctl_string(c, c->argv+2, c->argc-2); return; #endif + } else if(!strcasecmp(szFromObj(c->argv[1]),"flush-storage") && c->argc == 2) { + if (g_pserver->m_pstorageFactory != nullptr) { + for (int i = 0; i < cserver.dbnum; i++) { + g_pserver->db[i]->getStorageCache()->flush(); + } + } else { + addReplyError(c, "Can't flush storage if no storage provider is set"); + } } else { addReplySubcommandSyntaxError(c); return; diff --git a/tests/unit/flash.tcl b/tests/unit/flash.tcl index 25d50387d..94e7287d5 100644 --- a/tests/unit/flash.tcl +++ b/tests/unit/flash.tcl @@ -74,7 +74,7 @@ if {$::flash_enabled} { r set testkey foo ex 1 r flushall cache assert_equal {1} [r dbsize] - after 2000 + after 5000 assert_equal {0} [r dbsize] } @@ -165,6 +165,47 @@ if {$::flash_enabled} { assert {[r get last] == {val}} r flushall } + + test "FLASH - is flash eviction working? (policy $policy)" { + # Get the current memory limit and calculate a new limit. + # Set limit to 100M. + set used [s used_memory] + set limit [expr {$used+60*1024*1024}] + r config set maxmemory $limit + r config set maxmemory-policy $policy + # Now add keys equivalent to 1024b until the limit is almost reached. + set numkeys 0 + r set first val + while 1 { + r set $numkeys xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx + incr numkeys + if {[s used_memory]+1024 >= $limit} { + break + } + } + # Add additional keys to force eviction + # should still be under the limit for maxmemory, however all keys set should still exist between flash and memory + # check same number of keys exist in addition to values of first and last keys + set err 0 + set extra_keys [expr floor([expr ($limit * 0.4) / 1024])] + for {set j 0} {$j < $extra_keys} {incr j} { + catch { + r set p2$j xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx + } err + assert {$err == {OK}} + } + if {[log_file_matches [srv 0 stdout] "*Failed to evict*"]} { + fail "Server did not evict cleanly (detected full flush)" + } + r set last val + set dbsize [r dbsize] + r config set maxstorage 1 + assert {[s used_memory] < ($limit*1.2)} + assert {$dbsize == $numkeys+$extra_keys+2} + assert {[r get first] == {val}} + assert {[r get last] == {val}} + r flushall + } } } } From 597dd12498e1bd84f40f146b7019b5df37fec50a Mon Sep 17 00:00:00 2001 From: Malavan Sotheeswaran Date: Tue, 27 Jun 2023 00:06:32 -0700 Subject: [PATCH 57/65] flash eviction test working --- src/debug.cpp | 1 + src/storage/rocksdb.cpp | 1 + tests/unit/flash.tcl | 12 +++++++----- 3 files changed, 9 insertions(+), 5 deletions(-) diff --git a/src/debug.cpp b/src/debug.cpp index 6ff1973b2..4e37dba4b 100644 --- a/src/debug.cpp +++ b/src/debug.cpp @@ -942,6 +942,7 @@ NULL for (int i = 0; i < cserver.dbnum; i++) { g_pserver->db[i]->getStorageCache()->flush(); } + addReply(c,shared.ok); } else { addReplyError(c, "Can't flush storage if no storage provider is set"); } diff --git a/src/storage/rocksdb.cpp b/src/storage/rocksdb.cpp index 8ac413c97..af6382594 100644 --- a/src/storage/rocksdb.cpp +++ b/src/storage/rocksdb.cpp @@ -343,6 +343,7 @@ void RocksDBStorageProvider::batch_unlock() void RocksDBStorageProvider::flush() { m_spdb->SyncWAL(); + m_spdb->Flush(rocksdb::FlushOptions()); } bool RocksDBStorageProvider::FKeyExists(std::string& key) const diff --git a/tests/unit/flash.tcl b/tests/unit/flash.tcl index 94e7287d5..ba1661c2b 100644 --- a/tests/unit/flash.tcl +++ b/tests/unit/flash.tcl @@ -198,12 +198,14 @@ if {$::flash_enabled} { fail "Server did not evict cleanly (detected full flush)" } r set last val - set dbsize [r dbsize] + r debug flush-storage r config set maxstorage 1 - assert {[s used_memory] < ($limit*1.2)} - assert {$dbsize == $numkeys+$extra_keys+2} - assert {[r get first] == {val}} - assert {[r get last] == {val}} + r config set maxmemory 1 + set dbsize [r dbsize] + # after setting maxstorage and memory below used amount we should evict from storage provider + assert {$dbsize < $numkeys+$extra_keys+2} + r config set maxstorage 0 + r config set maxmemory 0 r flushall } } From 336202eba52247bd6ebfbf774504de4bc87469b4 Mon Sep 17 00:00:00 2001 From: Malavan Sotheeswaran Date: Tue, 27 Jun 2023 11:43:13 -0700 Subject: [PATCH 58/65] strict < --- src/storage/rocksdb.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/storage/rocksdb.cpp b/src/storage/rocksdb.cpp index af6382594..971aa5727 100644 --- a/src/storage/rocksdb.cpp +++ b/src/storage/rocksdb.cpp @@ -246,7 +246,7 @@ std::vector RocksDBStorageProvider::getExpirationCandidates(unsigne { std::vector result; std::unique_ptr it = std::unique_ptr(m_spdb->NewIterator(ReadOptions(), m_spexpirecolfamily.get())); - for (it->SeekToFirst(); it->Valid() && (*((long long *)it->key().data()) <= time) && (result.size() < count); it->Next()) { + for (it->SeekToFirst(); it->Valid() && (*((long long *)it->key().data()) < time) && (result.size() < count); it->Next()) { if (FInternalKey(it->key().data(), it->key().size())) continue; result.emplace_back(it->value().data(), it->value().size()); From 04cc2c01c682f3121f9e2af7230bb98cc5a8676e Mon Sep 17 00:00:00 2001 From: Malavan Sotheeswaran Date: Wed, 28 Jun 2023 00:27:51 -0700 Subject: [PATCH 59/65] change expire test to wait_for_condition --- tests/unit/flash.tcl | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/tests/unit/flash.tcl b/tests/unit/flash.tcl index ba1661c2b..21a4c17eb 100644 --- a/tests/unit/flash.tcl +++ b/tests/unit/flash.tcl @@ -74,8 +74,11 @@ if {$::flash_enabled} { r set testkey foo ex 1 r flushall cache assert_equal {1} [r dbsize] - after 5000 - assert_equal {0} [r dbsize] + wait_for_condition 50 100 { + [r dbsize] == 0 + } else { + fail "key is not expired" + } } test { SUBKEY EXPIRE persists after cache flush } { From 9281b728d510b90c1109049aa986b62061deb5d0 Mon Sep 17 00:00:00 2001 From: Malavan Sotheeswaran Date: Wed, 28 Jun 2023 15:50:24 -0700 Subject: [PATCH 60/65] use dedicated db for load test to avoid loading other keys --- tests/unit/moduleapi/load.tcl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/moduleapi/load.tcl b/tests/unit/moduleapi/load.tcl index 853b9aebb..12ca22402 100644 --- a/tests/unit/moduleapi/load.tcl +++ b/tests/unit/moduleapi/load.tcl @@ -1,7 +1,7 @@ set testmodule [file normalize tests/modules/load.so] if {$::flash_enabled} { - start_server [list tags [list "modules"] overrides [list storage-provider {flash ./rocks.db.master} databases 256 loadmodule $testmodule]] { + start_server [list tags [list "modules"] overrides [list storage-provider {flash ./rocks.db.master.load.test} databases 256 loadmodule $testmodule]] { test "Module is notified of keys loaded from flash" { r flushall r set foo bar From 1d547f89e136001be5c356d1a8793bcafb440b67 Mon Sep 17 00:00:00 2001 From: Malavan Sotheeswaran Date: Wed, 28 Jun 2023 17:44:36 -0700 Subject: [PATCH 61/65] 1 second between each check --- tests/unit/flash.tcl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/flash.tcl b/tests/unit/flash.tcl index 21a4c17eb..52154f804 100644 --- a/tests/unit/flash.tcl +++ b/tests/unit/flash.tcl @@ -74,7 +74,7 @@ if {$::flash_enabled} { r set testkey foo ex 1 r flushall cache assert_equal {1} [r dbsize] - wait_for_condition 50 100 { + wait_for_condition 50 1000 { [r dbsize] == 0 } else { fail "key is not expired" From 747eb8067e7d47e79571c43385d8207cd5ca2dba Mon Sep 17 00:00:00 2001 From: Malavan Sotheeswaran Date: Thu, 29 Jun 2023 21:50:05 -0700 Subject: [PATCH 62/65] disk space test --- src/debug.cpp | 6 ++++++ tests/unit/flash.tcl | 15 +++++++++++++++ 2 files changed, 21 insertions(+) diff --git a/src/debug.cpp b/src/debug.cpp index 4e37dba4b..2854ec521 100644 --- a/src/debug.cpp +++ b/src/debug.cpp @@ -946,6 +946,12 @@ NULL } else { addReplyError(c, "Can't flush storage if no storage provider is set"); } + } else if (!strcasecmp(szFromObj(c->argv[1]),"get-storage-usage") && c->argc == 2) { + if (g_pserver->m_pstorageFactory != nullptr) { + addReplyLongLong(c, g_pserver->m_pstorageFactory->totalDiskspaceUsed()); + } else { + addReplyLongLong(c, 0); + } } else { addReplySubcommandSyntaxError(c); return; diff --git a/tests/unit/flash.tcl b/tests/unit/flash.tcl index 52154f804..60a28f738 100644 --- a/tests/unit/flash.tcl +++ b/tests/unit/flash.tcl @@ -81,6 +81,21 @@ if {$::flash_enabled} { } } + test { EXPIRE reduces used disk space } { + r flushall + r set testkey foo ex 10 + r debug flush-storage + set used [r debug get-storage-usage] + assert_equal {1} [r dbsize] + wait_for_condition 50 1000 { + [r dbsize] == 0 + } else { + fail "key is not expired" + } + set after [r debug get-storage-usage] + assert {$after < $used} + } + test { SUBKEY EXPIRE persists after cache flush } { r flushall r sadd testkey foo bar baz From fca3c77823b861a229290cd9b18093ff0c74f00f Mon Sep 17 00:00:00 2001 From: Malavan Sotheeswaran Date: Tue, 22 Aug 2023 13:03:03 -0700 Subject: [PATCH 63/65] somehow missed reference to m_setexpire --- src/db.cpp | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/db.cpp b/src/db.cpp index 1b521cfa6..12edacb17 100644 --- a/src/db.cpp +++ b/src/db.cpp @@ -3121,11 +3121,6 @@ bool redisDbPersistentData::removeCachedValue(const char *key, dictEntry **ppde) if (m_spstorage != nullptr) m_spstorage->batch_unlock(); - std::unique_lock ul(g_expireLock); - auto itr = m_setexpire->find(key); - if (itr != m_setexpire->end()) - m_setexpire->erase(itr); - return true; } From 8f8b61f21110dff0dcce6ecd9601dc54c1e1ffcc Mon Sep 17 00:00:00 2001 From: Malavan Sotheeswaran Date: Tue, 22 Aug 2023 13:18:14 -0700 Subject: [PATCH 64/65] compile fix --- src/StorageCache.cpp | 8 ++++---- src/expire.cpp | 6 ++++-- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/src/StorageCache.cpp b/src/StorageCache.cpp index a955f8ed5..ba7910399 100644 --- a/src/StorageCache.cpp +++ b/src/StorageCache.cpp @@ -85,8 +85,8 @@ void StorageCache::cacheKey(const char *rgch, size_t cch) bool StorageCache::erase(sds key) { unsigned long long when = 0; - m_spstorage->retrieve(key, sdslen(key), [&when, &key](const char *, size_t, const void * data, size_t cbdata) { - auto e = deserializeExpire(key, (const char *)data, cbdata, nullptr); + m_spstorage->retrieve(key, sdslen(key), [&when](const char *, size_t, const void * data, size_t cbdata) { + auto e = deserializeExpire((const char *)data, cbdata, nullptr); if (e != nullptr) when = e->when(); }); @@ -123,7 +123,7 @@ void StorageCache::insert(sds key, const void *data, size_t cbdata, bool fOverwr } ul.unlock(); m_spstorage->insert(key, sdslen(key), (void*)data, cbdata, fOverwrite); - auto e = deserializeExpire(key, (const char *)data, cbdata, nullptr); + auto e = deserializeExpire((const char *)data, cbdata, nullptr); if (e != nullptr) m_spstorage->setExpire(key, sdslen(key), e->when()); } @@ -143,7 +143,7 @@ void StorageCache::bulkInsert(char **rgkeys, size_t *rgcbkeys, char **rgvals, si de->v.u64 = 1; vechashes.push_back(de); } - auto e = deserializeExpire(rgkeys[ielem], rgvals[ielem], rgcbvals[ielem], nullptr); + auto e = deserializeExpire(rgvals[ielem], rgcbvals[ielem], nullptr); if (e != nullptr) m_spstorage->setExpire(rgkeys[ielem], rgcbkeys[ielem], e->when()); } diff --git a/src/expire.cpp b/src/expire.cpp index 99879950f..2532a7563 100644 --- a/src/expire.cpp +++ b/src/expire.cpp @@ -334,7 +334,7 @@ void pexpireMemberAtCommand(client *c) static int timelimit_exit = 0; /* Time limit hit in previous call? */ static long long last_fast_cycle = 0; /* When last fast cycle ran. */ - int j; + int j, iteration; int dbs_per_call = CRON_DBS_PER_CALL; long long start = ustime(), timelimit, elapsed; @@ -518,6 +518,8 @@ void pexpireMemberAtCommand(client *c) } while (sampled == 0 || (expired*100/sampled) > config_cycle_acceptable_stale); } else { + long long now; + size_t tried = 0; std::vector keys; do { keys = db->getStorageCache()->getExpirationCandidates(ACTIVE_EXPIRE_CYCLE_LOOKUPS_PER_LOOP, now); @@ -526,7 +528,7 @@ void pexpireMemberAtCommand(client *c) db->find(szFromObj(keyobj)); expireEntry *e = db->getExpire(keyobj); if (e != nullptr && e->when() < now) - total_expired += activeExpireCycleExpire(db, *e, now, tried); + total_expired += activeExpireCycleExpire(db, szFromObj(keyobj), *e, now, tried); decrRefCount(keyobj); } total_sampled += keys.size(); From 717805e1915ea5ce9396b8f6a97cec7d217faaaf Mon Sep 17 00:00:00 2001 From: Malavan Sotheeswaran Date: Tue, 22 Aug 2023 22:48:26 -0700 Subject: [PATCH 65/65] init iteration --- src/expire.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/expire.cpp b/src/expire.cpp index 2532a7563..52d9b4362 100644 --- a/src/expire.cpp +++ b/src/expire.cpp @@ -334,7 +334,7 @@ void pexpireMemberAtCommand(client *c) static int timelimit_exit = 0; /* Time limit hit in previous call? */ static long long last_fast_cycle = 0; /* When last fast cycle ran. */ - int j, iteration; + int j, iteration = 0; int dbs_per_call = CRON_DBS_PER_CALL; long long start = ustime(), timelimit, elapsed;