From 6c32c8004dc93dc6b724344422bff51cde475d9d Mon Sep 17 00:00:00 2001
From: Shahar Mike <chakaz@users.noreply.github.com>
Date: Wed, 20 Dec 2023 09:53:52 +0200
Subject: [PATCH] refactor(server): Make `FindFirst()` read-only (#2317)

* refactor(DbSlice): Replace `FindExt()` with `FindMutable()` and `FindReadOnly`

* fix

* `ExpireConstIterator`

* Don't update stats on FindMutable()

* auto&

* FindFirst

* Remove old Find() method
---
 src/server/container_utils.cc |  4 ++--
 src/server/db_slice.cc        | 21 ++++-----------------
 src/server/db_slice.h         |  8 +++-----
 3 files changed, 9 insertions(+), 24 deletions(-)

diff --git a/src/server/container_utils.cc b/src/server/container_utils.cc
index aa3901c42..7c4832e88 100644
--- a/src/server/container_utils.cc
+++ b/src/server/container_utils.cc
@@ -186,8 +186,8 @@ OpResult<ShardFFResult> FindFirstNonEmptyKey(Transaction* trans, int req_obj_typ
 
   auto cb = [&](Transaction* t, EngineShard* shard) {
     auto args = t->GetShardArgs(shard->shard_id());
-    OpResult<std::pair<PrimeIterator, unsigned>> ff_res =
-        shard->db_slice().FindFirst(t->GetDbContext(), args, req_obj_type);
+    OpResult<std::pair<PrimeConstIterator, unsigned>> ff_res =
+        shard->db_slice().FindFirstReadOnly(t->GetDbContext(), args, req_obj_type);
 
     if (ff_res) {
       FFResult ff_result(ff_res->first->first.AsRef(), ff_res->second);
diff --git a/src/server/db_slice.cc b/src/server/db_slice.cc
index 1702c19ab..81e79f4d8 100644
--- a/src/server/db_slice.cc
+++ b/src/server/db_slice.cc
@@ -330,20 +330,6 @@ void DbSlice::Reserve(DbIndex db_ind, size_t key_size) {
   db->prime.Reserve(key_size);
 }
 
-OpResult<PrimeIterator> DbSlice::Find(const Context& cntx, string_view key,
-                                      unsigned req_obj_type) const {
-  auto it = FindInternal(cntx, key, FindInternalMode::kDontUpdateCacheStats).it;
-
-  if (!IsValid(it))
-    return OpStatus::KEY_NOTFOUND;
-
-  if (it->second.ObjType() != req_obj_type) {
-    return OpStatus::WRONG_TYPE;
-  }
-
-  return it;
-}
-
 DbSlice::AutoUpdater::AutoUpdater() {
 }
 
@@ -503,13 +489,14 @@ DbSlice::ItAndExp DbSlice::FindInternal(const Context& cntx, std::string_view ke
   return res;
 }
 
-OpResult<pair<PrimeIterator, unsigned>> DbSlice::FindFirst(const Context& cntx, ArgSlice args,
-                                                           int req_obj_type) {
+OpResult<pair<PrimeConstIterator, unsigned>> DbSlice::FindFirstReadOnly(const Context& cntx,
+                                                                        ArgSlice args,
+                                                                        int req_obj_type) {
   DCHECK(!args.empty());
 
   for (unsigned i = 0; i < args.size(); ++i) {
     string_view s = args[i];
-    OpResult<PrimeIterator> res = Find(cntx, s, req_obj_type);
+    OpResult<PrimeConstIterator> res = FindReadOnly(cntx, s, req_obj_type);
     if (res)
       return make_pair(res.value(), i);
     if (res.status() != OpStatus::KEY_NOTFOUND)
diff --git a/src/server/db_slice.h b/src/server/db_slice.h
index ab74e3500..73fa93c0e 100644
--- a/src/server/db_slice.h
+++ b/src/server/db_slice.h
@@ -207,8 +207,9 @@ class DbSlice {
 
   // Returns (iterator, args-index) if found, KEY_NOTFOUND otherwise.
   // If multiple keys are found, returns the first index in the ArgSlice.
-  OpResult<std::pair<PrimeIterator, unsigned>> FindFirst(const Context& cntx, ArgSlice args,
-                                                         int req_obj_type);
+  OpResult<std::pair<PrimeConstIterator, unsigned>> FindFirstReadOnly(const Context& cntx,
+                                                                      ArgSlice args,
+                                                                      int req_obj_type);
 
   struct AddOrFindResult {
     PrimeIterator it;
@@ -394,9 +395,6 @@ class DbSlice {
   void ReleaseNormalized(IntentLock::Mode m, DbIndex db_index, std::string_view key,
                          unsigned count);
 
-  OpResult<PrimeIterator> Find(const Context& cntx, std::string_view key,
-                               unsigned req_obj_type) const;
-
   AddOrFindResult AddOrUpdateInternal(const Context& cntx, std::string_view key, PrimeValue obj,
                                       uint64_t expire_at_ms, bool force_update) noexcept(false);