refactor: return OpResult in DbSlice::AddOrFind instead of throwing std::bad_alloc (#2427)

* return OpResult in AddOrFind instead of throwing bad_alloc
* small refactor

src/facade/op_status.h

@@ -82,15 +82,19 @@ template <typename V> class OpResult : public OpResultBase {
     return &v_;
   }
 
-  V& operator*() {
+  V& operator*() & {
     return v_;
   }
 
+  V&& operator*() && {
+    return std::move(v_);
+  }
+
   const V* operator->() const {
     return &v_;
   }
 
-  const V& operator*() const {
+  const V& operator*() const& {
     return v_;
   }
 

src/server/bitops_family.cc

@@ -323,21 +323,18 @@ std::optional<bool> ElementAccess::Exists(EngineShard* shard) {
 }
 
 OpStatus ElementAccess::Find(EngineShard* shard) {
-  try {
+  auto op_res = shard->db_slice().AddOrFindAndFetch(context_, key_);
-    auto add_res = shard->db_slice().AddOrFindAndFetch(context_, key_);
+  RETURN_ON_BAD_STATUS(op_res);
-    if (!add_res.is_new) {
+  auto& add_res = *op_res;
-      if (add_res.it->second.ObjType() != OBJ_STRING) {
+
-        return OpStatus::WRONG_TYPE;
+  if (!add_res.is_new && add_res.it->second.ObjType() != OBJ_STRING) {
-      }
+    return OpStatus::WRONG_TYPE;
-    }
-    element_iter_ = add_res.it;
-    added_ = add_res.is_new;
-    shard_ = shard;
-    post_updater_ = std::move(add_res.post_updater);
-    return OpStatus::OK;
-  } catch (const std::bad_alloc&) {
-    return OpStatus::OUT_OF_MEMORY;
   }
+  element_iter_ = add_res.it;
+  added_ = add_res.is_new;
+  shard_ = shard;
+  post_updater_ = std::move(add_res.post_updater);
+  return OpStatus::OK;
 }
 
 std::string ElementAccess::Value() const {

src/server/db_slice.cc

@@ -14,6 +14,7 @@ extern "C" {
 #include "base/logging.h"
 #include "generic_family.h"
 #include "server/engine_shard_set.h"
+#include "server/error.h"
 #include "server/journal/journal.h"
 #include "server/server_state.h"
 #include "server/tiered_storage.h"
@@ -536,16 +537,17 @@ OpResult<pair<PrimeConstIterator, unsigned>> DbSlice::FindFirstReadOnly(const Co
   return OpStatus::KEY_NOTFOUND;
 }
 
-DbSlice::AddOrFindResult DbSlice::AddOrFind(const Context& cntx, string_view key) {
+OpResult<DbSlice::AddOrFindResult> DbSlice::AddOrFind(const Context& cntx, string_view key) {
   return AddOrFindInternal(cntx, key, LoadExternalMode::kDontLoad);
 }
 
-DbSlice::AddOrFindResult DbSlice::AddOrFindAndFetch(const Context& cntx, string_view key) {
+OpResult<DbSlice::AddOrFindResult> DbSlice::AddOrFindAndFetch(const Context& cntx,
+                                                              string_view key) {
   return AddOrFindInternal(cntx, key, LoadExternalMode::kLoad);
 }
 
-DbSlice::AddOrFindResult DbSlice::AddOrFindInternal(const Context& cntx, string_view key,
+OpResult<DbSlice::AddOrFindResult> DbSlice::AddOrFindInternal(const Context& cntx, string_view key,
-                                                    LoadExternalMode load_mode) noexcept(false) {
+                                                              LoadExternalMode load_mode) {
   DCHECK(IsDbValid(cntx.db_index));
 
   DbTable& db = *db_arr_[cntx.db_index];
@@ -553,16 +555,18 @@ DbSlice::AddOrFindResult DbSlice::AddOrFindInternal(const Context& cntx, string_
 
   if (res.ok()) {
     PreUpdate(cntx.db_index, res->it);
-    return {.it = res->it,
+    return DbSlice::AddOrFindResult{
-            .exp_it = res->exp_it,
+        .it = res->it,
-            .is_new = false,
+        .exp_it = res->exp_it,
-            .post_updater = AutoUpdater({.action = AutoUpdater::DestructorAction::kRun,
+        .is_new = false,
-                                         .db_slice = this,
+        .post_updater = AutoUpdater({.action = AutoUpdater::DestructorAction::kRun,
-                                         .db_ind = cntx.db_index,
+                                     .db_slice = this,
-                                         .it = res->it,
+                                     .db_ind = cntx.db_index,
-                                         .key = key})};
+                                     .it = res->it,
+                                     .key = key})};
   }
-  CHECK_EQ(res.status(), OpStatus::KEY_NOTFOUND);
+  auto status = res.status();
+  CHECK(status == OpStatus::KEY_NOTFOUND || status == OpStatus::OUT_OF_MEMORY) << status;
 
   // It's a new entry.
   DVLOG(2) << "Running callbacks for key " << key << " in dbid " << cntx.db_index;
@@ -592,7 +596,7 @@ DbSlice::AddOrFindResult DbSlice::AddOrFindInternal(const Context& cntx, string_
   if (apply_memory_limit && !caching_mode_ && evp.mem_budget() < 0) {
     VLOG(2) << "AddOrFind: over limit, budget: " << evp.mem_budget();
     events_.insertion_rejections++;
-    throw bad_alloc();
+    return OpStatus::OUT_OF_MEMORY;
   }
 
   // Fast-path if change_cb_ is empty so we Find or Add using
@@ -606,8 +610,7 @@ DbSlice::AddOrFindResult DbSlice::AddOrFindInternal(const Context& cntx, string_
   } catch (bad_alloc& e) {
     VLOG(2) << "AddOrFind2: bad alloc exception, budget: " << evp.mem_budget();
     events_.insertion_rejections++;
-
+    return OpStatus::OUT_OF_MEMORY;
-    throw e;
   }
 
   size_t evicted_obj_bytes = 0;
@@ -639,14 +642,15 @@ DbSlice::AddOrFindResult DbSlice::AddOrFindInternal(const Context& cntx, string_
     db.slots_stats[sid].key_count += 1;
   }
 
-  return {.it = it,
+  return DbSlice::AddOrFindResult{
-          .exp_it = ExpireIterator{},
+      .it = it,
-          .is_new = true,
+      .exp_it = ExpireIterator{},
-          .post_updater = AutoUpdater({.action = AutoUpdater::DestructorAction::kRun,
+      .is_new = true,
-                                       .db_slice = this,
+      .post_updater = AutoUpdater({.action = AutoUpdater::DestructorAction::kRun,
-                                       .db_ind = cntx.db_index,
+                                   .db_slice = this,
-                                       .it = it,
+                                   .db_ind = cntx.db_index,
-                                       .key = key})};
+                                   .it = it,
+                                   .key = key})};
 }
 
 void DbSlice::ActivateDb(DbIndex db_ind) {
@@ -824,12 +828,15 @@ uint32_t DbSlice::GetMCFlag(DbIndex db_ind, const PrimeKey& key) const {
   return it->second;
 }
 
-DbSlice::ItAndUpdater DbSlice::AddNew(const Context& cntx, string_view key, PrimeValue obj,
+OpResult<DbSlice::ItAndUpdater> DbSlice::AddNew(const Context& cntx, string_view key,
-                                      uint64_t expire_at_ms) noexcept(false) {
+                                                PrimeValue obj, uint64_t expire_at_ms) {
-  auto res = AddOrUpdateInternal(cntx, key, std::move(obj), expire_at_ms, false);
+  auto op_result = AddOrUpdateInternal(cntx, key, std::move(obj), expire_at_ms, false);
+  RETURN_ON_BAD_STATUS(op_result);
+  auto& res = *op_result;
   CHECK(res.is_new);
 
-  return {.it = res.it, .exp_it = res.exp_it, .post_updater = std::move(res.post_updater)};
+  return DbSlice::ItAndUpdater{
+      .it = res.it, .exp_it = res.exp_it, .post_updater = std::move(res.post_updater)};
 }
 
 pair<int64_t, int64_t> DbSlice::ExpireParams::Calculate(int64_t now_ms) const {
@@ -882,14 +889,19 @@ OpResult<int64_t> DbSlice::UpdateExpire(const Context& cntx, PrimeIterator prime
   }
 }
 
-DbSlice::AddOrFindResult DbSlice::AddOrUpdateInternal(const Context& cntx, std::string_view key,
+OpResult<DbSlice::AddOrFindResult> DbSlice::AddOrUpdateInternal(const Context& cntx,
-                                                      PrimeValue obj, uint64_t expire_at_ms,
+                                                                std::string_view key,
-                                                      bool force_update) noexcept(false) {
+                                                                PrimeValue obj,
+                                                                uint64_t expire_at_ms,
+                                                                bool force_update) {
   DCHECK(!obj.IsRef());
 
-  auto res = AddOrFind(cntx, key);
+  auto op_result = AddOrFind(cntx, key);
+  RETURN_ON_BAD_STATUS(op_result);
+
+  auto& res = *op_result;
   if (!res.is_new && !force_update)  // have not inserted.
-    return res;
+    return op_result;
 
   auto& db = *db_arr_[cntx.db_index];
   auto& it = res.it;
@@ -906,11 +918,11 @@ DbSlice::AddOrFindResult DbSlice::AddOrUpdateInternal(const Context& cntx, std::
     }
   }
 
-  return res;
+  return op_result;
 }
 
-DbSlice::AddOrFindResult DbSlice::AddOrUpdate(const Context& cntx, string_view key, PrimeValue obj,
+OpResult<DbSlice::AddOrFindResult> DbSlice::AddOrUpdate(const Context& cntx, string_view key,
-                                              uint64_t expire_at_ms) noexcept(false) {
+                                                        PrimeValue obj, uint64_t expire_at_ms) {
   return AddOrUpdateInternal(cntx, key, std::move(obj), expire_at_ms, true);
 }
 

src/server/db_slice.h

@@ -227,18 +227,18 @@ class DbSlice {
     AddOrFindResult& operator=(ItAndUpdater&& o);
   };
 
-  AddOrFindResult AddOrFind(const Context& cntx, std::string_view key) noexcept(false);
+  OpResult<AddOrFindResult> AddOrFind(const Context& cntx, std::string_view key);
-  AddOrFindResult AddOrFindAndFetch(const Context& cntx, std::string_view key) noexcept(false);
+  OpResult<AddOrFindResult> AddOrFindAndFetch(const Context& cntx, std::string_view key);
 
   // Same as AddOrSkip, but overwrites in case entry exists.
-  AddOrFindResult AddOrUpdate(const Context& cntx, std::string_view key, PrimeValue obj,
+  OpResult<AddOrFindResult> AddOrUpdate(const Context& cntx, std::string_view key, PrimeValue obj,
-                              uint64_t expire_at_ms) noexcept(false);
+                                        uint64_t expire_at_ms);
 
   // Adds a new entry. Requires: key does not exist in this slice.
   // Returns the iterator to the newly added entry.
-  // throws: bad_alloc is insertion could not happen due to out of memory.
+  // Returns OpStatus::OUT_OF_MEMORY if bad_alloc is thrown
-  ItAndUpdater AddNew(const Context& cntx, std::string_view key, PrimeValue obj,
+  OpResult<ItAndUpdater> AddNew(const Context& cntx, std::string_view key, PrimeValue obj,
-                      uint64_t expire_at_ms) noexcept(false);
+                                uint64_t expire_at_ms);
 
   // Update entry expiration. Return epxiration timepoint in abs milliseconds, or -1 if the entry
   // already expired and was deleted;
@@ -412,8 +412,9 @@ class DbSlice {
   void PreUpdate(DbIndex db_ind, PrimeIterator it);
   void PostUpdate(DbIndex db_ind, PrimeIterator it, std::string_view key, size_t orig_size);
 
-  AddOrFindResult AddOrUpdateInternal(const Context& cntx, std::string_view key, PrimeValue obj,
+  OpResult<AddOrFindResult> AddOrUpdateInternal(const Context& cntx, std::string_view key,
-                                      uint64_t expire_at_ms, bool force_update) noexcept(false);
+                                                PrimeValue obj, uint64_t expire_at_ms,
+                                                bool force_update);
 
   void FlushSlotsFb(const SlotSet& slot_ids);
   void FlushDbIndexes(const std::vector<DbIndex>& indexes);
@@ -444,8 +445,8 @@ class DbSlice {
   OpResult<ItAndExp> FindInternal(const Context& cntx, std::string_view key,
                                   std::optional<unsigned> req_obj_type, UpdateStatsMode stats_mode,
                                   LoadExternalMode load_mode);
-  AddOrFindResult AddOrFindInternal(const Context& cntx, std::string_view key,
+  OpResult<AddOrFindResult> AddOrFindInternal(const Context& cntx, std::string_view key,
-                                    LoadExternalMode load_mode) noexcept(false);
+                                              LoadExternalMode load_mode);
   OpResult<ItAndUpdater> FindMutableInternal(const Context& cntx, std::string_view key,
                                              std::optional<unsigned> req_obj_type,
                                              LoadExternalMode load_mode);

src/server/dragonfly_test.cc

@@ -541,19 +541,19 @@ TEST_F(DflyEngineTest, Bug496) {
         db.RegisterOnChange([&cb_hits](DbIndex, const DbSlice::ChangeReq&) { cb_hits++; });
 
     {
-      auto res = db.AddOrFind({}, "key-1");
+      auto res = *db.AddOrFind({}, "key-1");
       EXPECT_TRUE(res.is_new);
       EXPECT_EQ(cb_hits, 1);
     }
 
     {
-      auto res = db.AddOrFind({}, "key-1");
+      auto res = *db.AddOrFind({}, "key-1");
       EXPECT_FALSE(res.is_new);
       EXPECT_EQ(cb_hits, 2);
     }
 
     {
-      auto res = db.AddOrFind({}, "key-2");
+      auto res = *db.AddOrFind({}, "key-2");
       EXPECT_TRUE(res.is_new);
       EXPECT_EQ(cb_hits, 3);
     }

src/server/error.h

@@ -33,6 +33,17 @@ using facade::kWrongTypeErr;
 
 #endif  // RETURN_ON_ERR
 
+#ifndef RETURN_ON_BAD_STATUS
+
+#define RETURN_ON_BAD_STATUS(x) \
+  do {                          \
+    if (!(x)) {                 \
+      return (x).status();      \
+    }                           \
+  } while (0)
+
+#endif  // RETURN_ON_BAD_STATUS
+
 namespace rdb {
 
 enum errc {

src/server/generic_family.cc

@@ -138,8 +138,8 @@ bool RdbRestoreValue::Add(std::string_view data, std::string_view key, DbSlice&
   }
 
   DbContext context{.db_index = index, .time_now_ms = GetCurrentTimeMs()};
-  db_slice.AddNew(context, key, std::move(pv), expire_ms);
+  auto res = db_slice.AddNew(context, key, std::move(pv), expire_ms);
-  return true;
+  return res.ok();
 }
 
 class RestoreArgs {
@@ -369,7 +369,10 @@ OpStatus Renamer::UpdateDest(Transaction* t, EngineShard* es) {
       if (src_res_.ref_val.ObjType() == OBJ_STRING) {
         pv_.SetString(str_val_);
       }
-      res = db_slice.AddNew(t->GetDbContext(), dest_key, std::move(pv_), src_res_.expire_ts);
+      auto op_res =
+          db_slice.AddNew(t->GetDbContext(), dest_key, std::move(pv_), src_res_.expire_ts);
+      RETURN_ON_BAD_STATUS(op_res);
+      res = std::move(*op_res);
     }
 
     dest_it->first.SetSticky(src_res_.sticky);
@@ -1433,7 +1436,9 @@ OpResult<void> GenericFamily::OpRen(const OpArgs& op_args, string_view from_key,
     // the value in `from_obj`.
     from_res.post_updater.Run();
     CHECK(db_slice.Del(op_args.db_cntx.db_index, from_res.it));
-    to_res = db_slice.AddNew(op_args.db_cntx, to_key, std::move(from_obj), exp_ts);
+    auto op_result = db_slice.AddNew(op_args.db_cntx, to_key, std::move(from_obj), exp_ts);
+    RETURN_ON_BAD_STATUS(op_result);
+    to_res = std::move(*op_result);
   }
 
   to_res.it->first.SetSticky(sticky);
@@ -1491,7 +1496,9 @@ OpStatus GenericFamily::OpMove(const OpArgs& op_args, string_view key, DbIndex t
   from_res.it->second.SetExpire(IsValid(from_res.exp_it));
 
   CHECK(db_slice.Del(op_args.db_cntx.db_index, from_res.it));
-  auto add_res = db_slice.AddNew(target_cntx, key, std::move(from_obj), exp_ts);
+  auto op_result = db_slice.AddNew(target_cntx, key, std::move(from_obj), exp_ts);
+  RETURN_ON_BAD_STATUS(op_result);
+  auto& add_res = *op_result;
   add_res.it->first.SetSticky(sticky);
 
   if (add_res.it->second.ObjType() == OBJ_LIST && op_args.shard->blocking_controller()) {

src/server/hll_family.cc

@@ -18,6 +18,7 @@ extern "C" {
 #include "server/conn_context.h"
 #include "server/container_utils.h"
 #include "server/engine_shard_set.h"
+#include "server/error.h"
 #include "server/transaction.h"
 
 namespace dfly {
@@ -70,33 +71,31 @@ OpResult<int> AddToHll(const OpArgs& op_args, string_view key, CmdArgList values
 
   string hll;
 
-  try {
+  auto op_res = db_slice.AddOrFind(op_args.db_cntx, key);
-    auto res = db_slice.AddOrFind(op_args.db_cntx, key);
+  RETURN_ON_BAD_STATUS(op_res);
-    if (res.is_new) {
+  auto& res = *op_res;
-      hll.resize(getDenseHllSize());
+  if (res.is_new) {
-      createDenseHll(StringToHllPtr(hll));
+    hll.resize(getDenseHllSize());
-    } else if (res.it->second.ObjType() != OBJ_STRING) {
+    createDenseHll(StringToHllPtr(hll));
-      return OpStatus::WRONG_TYPE;
+  } else if (res.it->second.ObjType() != OBJ_STRING) {
-    } else {
+    return OpStatus::WRONG_TYPE;
-      res.it->second.GetString(&hll);
+  } else {
-      ConvertToDenseIfNeeded(&hll);
+    res.it->second.GetString(&hll);
-    }
+    ConvertToDenseIfNeeded(&hll);
-
-    int updated = 0;
-    for (const auto& value : values) {
-      int added = pfadd(StringToHllPtr(hll), (unsigned char*)value.data(), value.size());
-      if (added < 0) {
-        return OpStatus::INVALID_VALUE;
-      }
-      updated += added;
-    }
-
-    res.it->second.SetString(hll);
-
-    return std::min(updated, 1);
-  } catch (const std::bad_alloc&) {
-    return OpStatus::OUT_OF_MEMORY;
   }
+
+  int updated = 0;
+  for (const auto& value : values) {
+    int added = pfadd(StringToHllPtr(hll), (unsigned char*)value.data(), value.size());
+    if (added < 0) {
+      return OpStatus::INVALID_VALUE;
+    }
+    updated += added;
+  }
+
+  res.it->second.SetString(hll);
+
+  return std::min(updated, 1);
 }
 
 void PFAdd(CmdArgList args, ConnectionContext* cntx) {
@@ -258,12 +257,9 @@ OpResult<int> PFMergeInternal(CmdArgList args, ConnectionContext* cntx) {
     string_view key = ArgS(args, 0);
     const OpArgs& op_args = t->GetOpArgs(shard);
     auto& db_slice = op_args.shard->db_slice();
-    DbSlice::AddOrFindResult res;
+    auto op_res = db_slice.AddOrFind(t->GetDbContext(), key);
-    try {
+    RETURN_ON_BAD_STATUS(op_res);
-      res = db_slice.AddOrFind(t->GetDbContext(), key);
+    auto& res = *op_res;
-    } catch (const bad_alloc& e) {
-      return OpStatus::OUT_OF_MEMORY;
-    }
     res.it->second.SetString(hll);
 
     if (op_args.shard->journal()) {

src/server/hset_family.cc

@@ -14,12 +14,12 @@ extern "C" {
 
 #include "base/logging.h"
 #include "core/string_map.h"
-#include "facade/error.h"
 #include "server/acl/acl_commands_def.h"
 #include "server/command_registry.h"
 #include "server/conn_context.h"
 #include "server/container_utils.h"
 #include "server/engine_shard_set.h"
+#include "server/error.h"
 #include "server/search/doc_index.h"
 #include "server/transaction.h"
 
@@ -168,12 +168,12 @@ OpStatus IncrementValue(optional<string_view> prev_val, IncrByParam* param) {
 
 OpStatus OpIncrBy(const OpArgs& op_args, string_view key, string_view field, IncrByParam* param) {
   auto& db_slice = op_args.shard->db_slice();
-  DbSlice::AddOrFindResult add_res;
+  auto op_res = db_slice.AddOrFind(op_args.db_cntx, key);
-  try {
+  RETURN_ON_BAD_STATUS(op_res);
-    add_res = db_slice.AddOrFind(op_args.db_cntx, key);
+  if (!op_res) {
-  } catch (const bad_alloc& e) {
+    return op_res.status();
-    return OpStatus::OUT_OF_MEMORY;
   }
+  auto& add_res = *op_res;
 
   DbTableStats* stats = db_slice.MutableStats(op_args.db_cntx.db_index);
 
@@ -620,12 +620,9 @@ OpResult<uint32_t> OpSet(const OpArgs& op_args, string_view key, CmdArgList valu
   VLOG(2) << "OpSet(" << key << ")";
 
   auto& db_slice = op_args.shard->db_slice();
-  DbSlice::AddOrFindResult add_res;
+  auto op_res = db_slice.AddOrFind(op_args.db_cntx, key);
-  try {
+  RETURN_ON_BAD_STATUS(op_res);
-    add_res = db_slice.AddOrFind(op_args.db_cntx, key);
+  auto& add_res = *op_res;
-  } catch (bad_alloc&) {
-    return OpStatus::OUT_OF_MEMORY;
-  }
 
   DbTableStats* stats = db_slice.MutableStats(op_args.db_cntx.db_index);
 

src/server/json_family.cc

@@ -4,6 +4,8 @@
 
 #include "server/json_family.h"
 
+#include "facade/op_status.h"
+
 extern "C" {
 #include "redis/object.h"
 }
@@ -50,15 +52,20 @@ inline OpStatus JsonReplaceVerifyNoOp(JsonType&) {
   return OpStatus::OK;
 }
 
-void SetJson(const OpArgs& op_args, string_view key, JsonType&& value) {
+facade::OpStatus SetJson(const OpArgs& op_args, string_view key, JsonType&& value) {
   auto& db_slice = op_args.shard->db_slice();
-  auto res = db_slice.AddOrFind(op_args.db_cntx, key);
+
+  auto op_res = db_slice.AddOrFind(op_args.db_cntx, key);
+  RETURN_ON_BAD_STATUS(op_res);
+
+  auto& res = *op_res;
 
   op_args.shard->search_indices()->RemoveDoc(key, op_args.db_cntx, res.it->second);
 
   res.it->second.SetJson(std::move(value));
 
   op_args.shard->search_indices()->AddDoc(key, op_args.db_cntx, res.it->second);
+  return OpStatus::OK;
 }
 
 string JsonTypeToName(const JsonType& val) {
@@ -1076,12 +1083,10 @@ OpResult<bool> OpSet(const OpArgs& op_args, string_view key, string_view path,
       }
     }
 
-    try {
+    if (SetJson(op_args, key, std::move(parsed_json.value())) == OpStatus::OUT_OF_MEMORY) {
-      SetJson(op_args, key, std::move(parsed_json.value()));
-
-    } catch (const bad_alloc& e) {
       return OpStatus::OUT_OF_MEMORY;
     }
+
     return true;
   }
 

src/server/list_family.cc

@@ -247,12 +247,9 @@ OpResult<string> OpMoveSingleShard(const OpArgs& op_args, string_view src, strin
 
   quicklist* dest_ql = nullptr;
   src_res->post_updater.Run();
-  DbSlice::AddOrFindResult dest_res;
+  auto op_res = db_slice.AddOrFind(op_args.db_cntx, dest);
-  try {
+  RETURN_ON_BAD_STATUS(op_res);
-    dest_res = db_slice.AddOrFind(op_args.db_cntx, dest);
+  auto& dest_res = *op_res;
-  } catch (bad_alloc&) {
-    return OpStatus::OUT_OF_MEMORY;
-  }
 
   // Insertion of dest could invalidate src_it. Find it again.
   src_res = db_slice.FindMutable(op_args.db_cntx, src, OBJ_LIST);
@@ -321,11 +318,9 @@ OpResult<uint32_t> OpPush(const OpArgs& op_args, std::string_view key, ListDir d
       return 0;  // Redis returns 0 for nonexisting keys for the *PUSHX actions.
     res = std::move(*tmp_res);
   } else {
-    try {
+    auto op_res = es->db_slice().AddOrFind(op_args.db_cntx, key);
-      res = es->db_slice().AddOrFind(op_args.db_cntx, key);
+    RETURN_ON_BAD_STATUS(op_res);
-    } catch (bad_alloc&) {
+    res = std::move(*op_res);
-      return OpStatus::OUT_OF_MEMORY;
-    }
   }
 
   quicklist* ql = nullptr;

src/server/rdb_load.cc

@@ -2364,18 +2364,19 @@ void RdbLoader::LoadItemsBuffer(DbIndex db_ind, const ItemsBuf& ib) {
     if (item->expire_ms > 0 && db_cntx.time_now_ms >= item->expire_ms)
       continue;
 
-    try {
+    auto op_res = db_slice.AddOrUpdate(db_cntx, item->key, std::move(pv), item->expire_ms);
-      auto res = db_slice.AddOrUpdate(db_cntx, item->key, std::move(pv), item->expire_ms);
+    if (!op_res) {
-      res.it->first.SetSticky(item->is_sticky);
-      if (!res.is_new) {
-        LOG(WARNING) << "RDB has duplicated key '" << item->key << "' in DB " << db_ind;
-      }
-    } catch (const std::bad_alloc&) {
       LOG(ERROR) << "OOM failed to add key '" << item->key << "' in DB " << db_ind;
       ec_ = RdbError(errc::out_of_memory);
       stop_early_ = true;
       break;
     }
+
+    auto& res = *op_res;
+    res.it->first.SetSticky(item->is_sticky);
+    if (!res.is_new) {
+      LOG(WARNING) << "RDB has duplicated key '" << item->key << "' in DB " << db_ind;
+    }
   }
 
   for (auto* item : ib) {

src/server/set_family.cc

@@ -566,13 +566,9 @@ OpResult<uint32_t> OpAdd(const OpArgs& op_args, std::string_view key, ArgSlice v
     return 0;
   }
 
-  DbSlice::AddOrFindResult add_res;
+  auto op_res = db_slice.AddOrFind(op_args.db_cntx, key);
-
+  RETURN_ON_BAD_STATUS(op_res);
-  try {
+  auto& add_res = *op_res;
-    add_res = db_slice.AddOrFind(op_args.db_cntx, key);
-  } catch (bad_alloc& e) {
-    return OpStatus::OUT_OF_MEMORY;
-  }
 
   CompactObj& co = add_res.it->second;
 
@@ -645,13 +641,9 @@ OpResult<uint32_t> OpAddEx(const OpArgs& op_args, string_view key, uint32_t ttl_
   auto* es = op_args.shard;
   auto& db_slice = es->db_slice();
 
-  DbSlice::AddOrFindResult add_res;
+  auto op_res = db_slice.AddOrFind(op_args.db_cntx, key);
-
+  RETURN_ON_BAD_STATUS(op_res);
-  try {
+  auto& add_res = *op_res;
-    add_res = db_slice.AddOrFind(op_args.db_cntx, key);
-  } catch (bad_alloc& e) {
-    return OpStatus::OUT_OF_MEMORY;
-  }
 
   CompactObj& co = add_res.it->second;
 

src/server/stream_family.cc

@@ -13,12 +13,12 @@ extern "C" {
 }
 
 #include "base/logging.h"
-#include "facade/error.h"
 #include "server/acl/acl_commands_def.h"
 #include "server/blocking_controller.h"
 #include "server/command_registry.h"
 #include "server/conn_context.h"
 #include "server/engine_shard_set.h"
+#include "server/error.h"
 #include "server/server_state.h"
 #include "server/transaction.h"
 
@@ -616,11 +616,9 @@ OpResult<streamID> OpAdd(const OpArgs& op_args, const AddTrimOpts& opts, CmdArgL
     }
     add_res = std::move(*res_it);
   } else {
-    try {
+    auto op_res = db_slice.AddOrFind(op_args.db_cntx, opts.key);
-      add_res = db_slice.AddOrFind(op_args.db_cntx, opts.key);
+    RETURN_ON_BAD_STATUS(op_res);
-    } catch (bad_alloc&) {
+    add_res = std::move(*op_res);
-      return OpStatus::OUT_OF_MEMORY;
-    }
   }
 
   robj* stream_obj = nullptr;

src/server/string_family.cc

@@ -75,28 +75,25 @@ OpResult<uint32_t> OpSetRange(const OpArgs& op_args, string_view key, size_t sta
     }
   }
 
-  DbSlice::AddOrFindResult res;
+  auto op_res = db_slice.AddOrFindAndFetch(op_args.db_cntx, key);
+  RETURN_ON_BAD_STATUS(op_res);
+  auto& res = *op_res;
 
-  try {
+  string s;
-    res = db_slice.AddOrFindAndFetch(op_args.db_cntx, key);
-    string s;
 
-    if (res.is_new) {
+  if (res.is_new) {
+    s.resize(range_len);
+  } else {
+    if (res.it->second.ObjType() != OBJ_STRING)
+      return OpStatus::WRONG_TYPE;
+
+    s = GetString(res.it->second);
+    if (s.size() < range_len)
       s.resize(range_len);
-    } else {
-      if (res.it->second.ObjType() != OBJ_STRING)
-        return OpStatus::WRONG_TYPE;
-
-      s = GetString(res.it->second);
-      if (s.size() < range_len)
-        s.resize(range_len);
-    }
-
-    memcpy(s.data() + start, value.data(), value.size());
-    res.it->second.SetString(s);
-  } catch (const std::bad_alloc& e) {
-    return OpStatus::OUT_OF_MEMORY;
   }
+
+  memcpy(s.data() + start, value.data(), value.size());
+  res.it->second.SetString(s);
   return res.it->second.Size();
 }
 
@@ -153,14 +150,9 @@ OpResult<uint32_t> ExtendOrSet(const OpArgs& op_args, string_view key, string_vi
                                bool prepend) {
   auto* shard = op_args.shard;
   auto& db_slice = shard->db_slice();
-
+  auto op_res = db_slice.AddOrFindAndFetch(op_args.db_cntx, key);
-  DbSlice::AddOrFindResult add_res;
+  RETURN_ON_BAD_STATUS(op_res);
-  try {
+  auto& add_res = *op_res;
-    add_res = db_slice.AddOrFindAndFetch(op_args.db_cntx, key);
-  } catch (const std::bad_alloc& e) {
-    return OpStatus::OUT_OF_MEMORY;
-  }
-
   if (add_res.is_new) {
     add_res.it->second.SetString(val);
     return val.size();
@@ -223,12 +215,10 @@ OpResult<string> OpMutableGet(const OpArgs& op_args, string_view key, bool del_h
 
 OpResult<double> OpIncrFloat(const OpArgs& op_args, string_view key, double val) {
   auto& db_slice = op_args.shard->db_slice();
-  DbSlice::AddOrFindResult add_res;
+
-  try {
+  auto op_res = db_slice.AddOrFind(op_args.db_cntx, key);
-    add_res = db_slice.AddOrFind(op_args.db_cntx, key);
+  RETURN_ON_BAD_STATUS(op_res);
-  } catch (const std::bad_alloc& e) {
+  auto& add_res = *op_res;
-    return OpStatus::OUT_OF_MEMORY;
-  }
 
   char buf[128];
 
@@ -281,11 +271,8 @@ OpResult<int64_t> OpIncrBy(const OpArgs& op_args, string_view key, int64_t incr,
     CompactObj cobj;
     cobj.SetInt(incr);
 
-    try {
+    auto op_result = db_slice.AddNew(op_args.db_cntx, key, std::move(cobj), 0);
-      res = db_slice.AddNew(op_args.db_cntx, key, std::move(cobj), 0);
+    RETURN_ON_BAD_STATUS(op_result);
-    } catch (bad_alloc&) {
-      return OpStatus::OUT_OF_MEMORY;
-    }
 
     return incr;
   }
@@ -469,10 +456,9 @@ OpResult<array<int64_t, 5>> OpThrottle(const OpArgs& op_args, const string_view
       CompactObj cobj;
       cobj.SetInt(new_tat_ms);
 
-      try {
+      auto res = db_slice.AddNew(op_args.db_cntx, key, std::move(cobj), new_tat_ms);
-        db_slice.AddNew(op_args.db_cntx, key, std::move(cobj), new_tat_ms);
+      if (!res) {
-      } catch (bad_alloc&) {
+        return res.status();
-        return OpStatus::OUT_OF_MEMORY;
       }
     }
   }
@@ -596,12 +582,9 @@ OpResult<optional<string>> SetCmd::Set(const SetParams& params, string_view key,
   // At this point we either need to add missing entry, or we
   // will override an existing one
   // Trying to add a new entry.
-  DbSlice::AddOrFindResult add_res;
+  auto op_res = db_slice.AddOrFind(op_args_.db_cntx, key);
-  try {
+  RETURN_ON_BAD_STATUS(op_res);
-    add_res = db_slice.AddOrFind(op_args_.db_cntx, key);  // TODO do we free the space for existing?
+  auto& add_res = *op_res;
-  } catch (bad_alloc& e) {
-    return OpStatus::OUT_OF_MEMORY;
-  }
 
   PrimeIterator it = add_res.it;
   if (!add_res.is_new) {

src/server/zset_family.cc

@@ -28,6 +28,7 @@ extern "C" {
 #include "server/conn_context.h"
 #include "server/container_utils.h"
 #include "server/engine_shard_set.h"
+#include "server/error.h"
 #include "server/transaction.h"
 
 namespace dfly {
@@ -193,13 +194,9 @@ OpResult<DbSlice::ItAndUpdater> FindZEntry(const ZParams& zparams, const OpArgs&
     return db_slice.FindMutable(op_args.db_cntx, key, OBJ_ZSET);
   }
 
-  DbSlice::AddOrFindResult add_res;
+  auto op_res = db_slice.AddOrFind(op_args.db_cntx, key);
-
+  RETURN_ON_BAD_STATUS(op_res);
-  try {
+  auto& add_res = *op_res;
-    add_res = db_slice.AddOrFind(op_args.db_cntx, key);
-  } catch (bad_alloc&) {
-    return OpStatus::OUT_OF_MEMORY;
-  }
 
   PrimeIterator& it = add_res.it;
   PrimeValue& pv = it->second;