diff --git a/src/facade/op_status.h b/src/facade/op_status.h index a4d39bab6..60f18204f 100644 --- a/src/facade/op_status.h +++ b/src/facade/op_status.h @@ -82,15 +82,19 @@ template 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_; } diff --git a/src/server/bitops_family.cc b/src/server/bitops_family.cc index 02c945589..1714340cd 100644 --- a/src/server/bitops_family.cc +++ b/src/server/bitops_family.cc @@ -323,21 +323,18 @@ std::optional ElementAccess::Exists(EngineShard* shard) { } OpStatus ElementAccess::Find(EngineShard* shard) { - try { - auto add_res = shard->db_slice().AddOrFindAndFetch(context_, key_); - if (!add_res.is_new) { - if (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; + auto op_res = shard->db_slice().AddOrFindAndFetch(context_, key_); + RETURN_ON_BAD_STATUS(op_res); + auto& add_res = *op_res; + + 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; } std::string ElementAccess::Value() const { diff --git a/src/server/db_slice.cc b/src/server/db_slice.cc index 71c1dafaf..590e34e69 100644 --- a/src/server/db_slice.cc +++ b/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> DbSlice::FindFirstReadOnly(const Co return OpStatus::KEY_NOTFOUND; } -DbSlice::AddOrFindResult DbSlice::AddOrFind(const Context& cntx, string_view key) { +OpResult 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::AddOrFindAndFetch(const Context& cntx, + string_view key) { return AddOrFindInternal(cntx, key, LoadExternalMode::kLoad); } -DbSlice::AddOrFindResult DbSlice::AddOrFindInternal(const Context& cntx, string_view key, - LoadExternalMode load_mode) noexcept(false) { +OpResult DbSlice::AddOrFindInternal(const Context& cntx, string_view key, + 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, - .exp_it = res->exp_it, - .is_new = false, - .post_updater = AutoUpdater({.action = AutoUpdater::DestructorAction::kRun, - .db_slice = this, - .db_ind = cntx.db_index, - .it = res->it, - .key = key})}; + return DbSlice::AddOrFindResult{ + .it = res->it, + .exp_it = res->exp_it, + .is_new = false, + .post_updater = AutoUpdater({.action = AutoUpdater::DestructorAction::kRun, + .db_slice = this, + .db_ind = cntx.db_index, + .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++; - - throw e; + return OpStatus::OUT_OF_MEMORY; } 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, - .exp_it = ExpireIterator{}, - .is_new = true, - .post_updater = AutoUpdater({.action = AutoUpdater::DestructorAction::kRun, - .db_slice = this, - .db_ind = cntx.db_index, - .it = it, - .key = key})}; + return DbSlice::AddOrFindResult{ + .it = it, + .exp_it = ExpireIterator{}, + .is_new = true, + .post_updater = AutoUpdater({.action = AutoUpdater::DestructorAction::kRun, + .db_slice = this, + .db_ind = cntx.db_index, + .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, - uint64_t expire_at_ms) noexcept(false) { - auto res = AddOrUpdateInternal(cntx, key, std::move(obj), expire_at_ms, false); +OpResult DbSlice::AddNew(const Context& cntx, string_view key, + PrimeValue obj, uint64_t expire_at_ms) { + 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 DbSlice::ExpireParams::Calculate(int64_t now_ms) const { @@ -882,14 +889,19 @@ OpResult DbSlice::UpdateExpire(const Context& cntx, PrimeIterator prime } } -DbSlice::AddOrFindResult DbSlice::AddOrUpdateInternal(const Context& cntx, std::string_view key, - PrimeValue obj, uint64_t expire_at_ms, - bool force_update) noexcept(false) { +OpResult DbSlice::AddOrUpdateInternal(const Context& cntx, + std::string_view key, + 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, - uint64_t expire_at_ms) noexcept(false) { +OpResult DbSlice::AddOrUpdate(const Context& cntx, string_view key, + PrimeValue obj, uint64_t expire_at_ms) { return AddOrUpdateInternal(cntx, key, std::move(obj), expire_at_ms, true); } diff --git a/src/server/db_slice.h b/src/server/db_slice.h index 9367e8e87..ea0d16f4e 100644 --- a/src/server/db_slice.h +++ b/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); - AddOrFindResult AddOrFindAndFetch(const Context& cntx, std::string_view key) noexcept(false); + OpResult AddOrFind(const Context& cntx, std::string_view key); + OpResult 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, - uint64_t expire_at_ms) noexcept(false); + OpResult AddOrUpdate(const Context& cntx, std::string_view key, PrimeValue obj, + 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. - ItAndUpdater AddNew(const Context& cntx, std::string_view key, PrimeValue obj, - uint64_t expire_at_ms) noexcept(false); + // Returns OpStatus::OUT_OF_MEMORY if bad_alloc is thrown + OpResult AddNew(const Context& cntx, std::string_view key, PrimeValue obj, + 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, - uint64_t expire_at_ms, bool force_update) noexcept(false); + OpResult AddOrUpdateInternal(const Context& cntx, std::string_view key, + PrimeValue obj, uint64_t expire_at_ms, + bool force_update); void FlushSlotsFb(const SlotSet& slot_ids); void FlushDbIndexes(const std::vector& indexes); @@ -444,8 +445,8 @@ class DbSlice { OpResult FindInternal(const Context& cntx, std::string_view key, std::optional req_obj_type, UpdateStatsMode stats_mode, LoadExternalMode load_mode); - AddOrFindResult AddOrFindInternal(const Context& cntx, std::string_view key, - LoadExternalMode load_mode) noexcept(false); + OpResult AddOrFindInternal(const Context& cntx, std::string_view key, + LoadExternalMode load_mode); OpResult FindMutableInternal(const Context& cntx, std::string_view key, std::optional req_obj_type, LoadExternalMode load_mode); diff --git a/src/server/dragonfly_test.cc b/src/server/dragonfly_test.cc index 25308541c..7da960851 100644 --- a/src/server/dragonfly_test.cc +++ b/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); } diff --git a/src/server/error.h b/src/server/error.h index c5b0c5a03..60d7309fa 100644 --- a/src/server/error.h +++ b/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 { diff --git a/src/server/generic_family.cc b/src/server/generic_family.cc index 0e1892bfc..8d008863e 100644 --- a/src/server/generic_family.cc +++ b/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); - return true; + auto res = db_slice.AddNew(context, key, std::move(pv), expire_ms); + 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 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()) { diff --git a/src/server/hll_family.cc b/src/server/hll_family.cc index 7d30e664a..0c1f045e2 100644 --- a/src/server/hll_family.cc +++ b/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 AddToHll(const OpArgs& op_args, string_view key, CmdArgList values string hll; - try { - auto res = db_slice.AddOrFind(op_args.db_cntx, key); - if (res.is_new) { - hll.resize(getDenseHllSize()); - createDenseHll(StringToHllPtr(hll)); - } else if (res.it->second.ObjType() != OBJ_STRING) { - return OpStatus::WRONG_TYPE; - } else { - 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; + auto op_res = db_slice.AddOrFind(op_args.db_cntx, key); + RETURN_ON_BAD_STATUS(op_res); + auto& res = *op_res; + if (res.is_new) { + hll.resize(getDenseHllSize()); + createDenseHll(StringToHllPtr(hll)); + } else if (res.it->second.ObjType() != OBJ_STRING) { + return OpStatus::WRONG_TYPE; + } else { + 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); } void PFAdd(CmdArgList args, ConnectionContext* cntx) { @@ -258,12 +257,9 @@ OpResult 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; - try { - res = db_slice.AddOrFind(t->GetDbContext(), key); - } catch (const bad_alloc& e) { - return OpStatus::OUT_OF_MEMORY; - } + auto op_res = db_slice.AddOrFind(t->GetDbContext(), key); + RETURN_ON_BAD_STATUS(op_res); + auto& res = *op_res; res.it->second.SetString(hll); if (op_args.shard->journal()) { diff --git a/src/server/hset_family.cc b/src/server/hset_family.cc index 9e2d7d8bb..c78bbbab8 100644 --- a/src/server/hset_family.cc +++ b/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 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; - try { - add_res = db_slice.AddOrFind(op_args.db_cntx, key); - } catch (const bad_alloc& e) { - return OpStatus::OUT_OF_MEMORY; + auto op_res = db_slice.AddOrFind(op_args.db_cntx, key); + RETURN_ON_BAD_STATUS(op_res); + if (!op_res) { + return op_res.status(); } + auto& add_res = *op_res; DbTableStats* stats = db_slice.MutableStats(op_args.db_cntx.db_index); @@ -620,12 +620,9 @@ OpResult 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; - try { - add_res = db_slice.AddOrFind(op_args.db_cntx, key); - } catch (bad_alloc&) { - return OpStatus::OUT_OF_MEMORY; - } + auto op_res = db_slice.AddOrFind(op_args.db_cntx, key); + RETURN_ON_BAD_STATUS(op_res); + auto& add_res = *op_res; DbTableStats* stats = db_slice.MutableStats(op_args.db_cntx.db_index); diff --git a/src/server/json_family.cc b/src/server/json_family.cc index 7e320f3e6..3dbdb45bc 100644 --- a/src/server/json_family.cc +++ b/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 OpSet(const OpArgs& op_args, string_view key, string_view path, } } - try { - SetJson(op_args, key, std::move(parsed_json.value())); - - } catch (const bad_alloc& e) { + if (SetJson(op_args, key, std::move(parsed_json.value())) == OpStatus::OUT_OF_MEMORY) { return OpStatus::OUT_OF_MEMORY; } + return true; } diff --git a/src/server/list_family.cc b/src/server/list_family.cc index 2c42f2687..1923c3e19 100644 --- a/src/server/list_family.cc +++ b/src/server/list_family.cc @@ -247,12 +247,9 @@ OpResult OpMoveSingleShard(const OpArgs& op_args, string_view src, strin quicklist* dest_ql = nullptr; src_res->post_updater.Run(); - DbSlice::AddOrFindResult dest_res; - try { - dest_res = db_slice.AddOrFind(op_args.db_cntx, dest); - } catch (bad_alloc&) { - return OpStatus::OUT_OF_MEMORY; - } + auto op_res = db_slice.AddOrFind(op_args.db_cntx, dest); + RETURN_ON_BAD_STATUS(op_res); + auto& dest_res = *op_res; // 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 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 { - res = es->db_slice().AddOrFind(op_args.db_cntx, key); - } catch (bad_alloc&) { - return OpStatus::OUT_OF_MEMORY; - } + auto op_res = es->db_slice().AddOrFind(op_args.db_cntx, key); + RETURN_ON_BAD_STATUS(op_res); + res = std::move(*op_res); } quicklist* ql = nullptr; diff --git a/src/server/rdb_load.cc b/src/server/rdb_load.cc index 151774f68..95a1a6bc3 100644 --- a/src/server/rdb_load.cc +++ b/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 res = db_slice.AddOrUpdate(db_cntx, item->key, std::move(pv), item->expire_ms); - 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&) { + auto op_res = db_slice.AddOrUpdate(db_cntx, item->key, std::move(pv), item->expire_ms); + if (!op_res) { 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) { diff --git a/src/server/set_family.cc b/src/server/set_family.cc index b9bda9462..64207ff83 100644 --- a/src/server/set_family.cc +++ b/src/server/set_family.cc @@ -566,13 +566,9 @@ OpResult OpAdd(const OpArgs& op_args, std::string_view key, ArgSlice v return 0; } - DbSlice::AddOrFindResult add_res; - - try { - add_res = db_slice.AddOrFind(op_args.db_cntx, key); - } catch (bad_alloc& e) { - return OpStatus::OUT_OF_MEMORY; - } + auto op_res = db_slice.AddOrFind(op_args.db_cntx, key); + RETURN_ON_BAD_STATUS(op_res); + auto& add_res = *op_res; CompactObj& co = add_res.it->second; @@ -645,13 +641,9 @@ OpResult 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; - - try { - add_res = db_slice.AddOrFind(op_args.db_cntx, key); - } catch (bad_alloc& e) { - return OpStatus::OUT_OF_MEMORY; - } + auto op_res = db_slice.AddOrFind(op_args.db_cntx, key); + RETURN_ON_BAD_STATUS(op_res); + auto& add_res = *op_res; CompactObj& co = add_res.it->second; diff --git a/src/server/stream_family.cc b/src/server/stream_family.cc index d27f4ef8a..e727a5171 100644 --- a/src/server/stream_family.cc +++ b/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 OpAdd(const OpArgs& op_args, const AddTrimOpts& opts, CmdArgL } add_res = std::move(*res_it); } else { - try { - add_res = db_slice.AddOrFind(op_args.db_cntx, opts.key); - } catch (bad_alloc&) { - return OpStatus::OUT_OF_MEMORY; - } + auto op_res = db_slice.AddOrFind(op_args.db_cntx, opts.key); + RETURN_ON_BAD_STATUS(op_res); + add_res = std::move(*op_res); } robj* stream_obj = nullptr; diff --git a/src/server/string_family.cc b/src/server/string_family.cc index adb3dcc54..b66926572 100644 --- a/src/server/string_family.cc +++ b/src/server/string_family.cc @@ -75,28 +75,25 @@ OpResult 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 { - res = db_slice.AddOrFindAndFetch(op_args.db_cntx, key); - string s; + 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 ExtendOrSet(const OpArgs& op_args, string_view key, string_vi bool prepend) { auto* shard = op_args.shard; auto& db_slice = shard->db_slice(); - - DbSlice::AddOrFindResult add_res; - try { - add_res = db_slice.AddOrFindAndFetch(op_args.db_cntx, key); - } catch (const std::bad_alloc& e) { - return OpStatus::OUT_OF_MEMORY; - } - + auto op_res = db_slice.AddOrFindAndFetch(op_args.db_cntx, key); + RETURN_ON_BAD_STATUS(op_res); + auto& add_res = *op_res; if (add_res.is_new) { add_res.it->second.SetString(val); return val.size(); @@ -223,12 +215,10 @@ OpResult OpMutableGet(const OpArgs& op_args, string_view key, bool del_h OpResult OpIncrFloat(const OpArgs& op_args, string_view key, double val) { auto& db_slice = op_args.shard->db_slice(); - DbSlice::AddOrFindResult add_res; - try { - add_res = db_slice.AddOrFind(op_args.db_cntx, key); - } catch (const std::bad_alloc& e) { - return OpStatus::OUT_OF_MEMORY; - } + + auto op_res = db_slice.AddOrFind(op_args.db_cntx, key); + RETURN_ON_BAD_STATUS(op_res); + auto& add_res = *op_res; char buf[128]; @@ -281,11 +271,8 @@ OpResult OpIncrBy(const OpArgs& op_args, string_view key, int64_t incr, CompactObj cobj; cobj.SetInt(incr); - try { - res = db_slice.AddNew(op_args.db_cntx, key, std::move(cobj), 0); - } catch (bad_alloc&) { - return OpStatus::OUT_OF_MEMORY; - } + auto op_result = db_slice.AddNew(op_args.db_cntx, key, std::move(cobj), 0); + RETURN_ON_BAD_STATUS(op_result); return incr; } @@ -469,10 +456,9 @@ OpResult> OpThrottle(const OpArgs& op_args, const string_view CompactObj cobj; cobj.SetInt(new_tat_ms); - try { - db_slice.AddNew(op_args.db_cntx, key, std::move(cobj), new_tat_ms); - } catch (bad_alloc&) { - return OpStatus::OUT_OF_MEMORY; + auto res = db_slice.AddNew(op_args.db_cntx, key, std::move(cobj), new_tat_ms); + if (!res) { + return res.status(); } } } @@ -596,12 +582,9 @@ OpResult> 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; - try { - add_res = db_slice.AddOrFind(op_args_.db_cntx, key); // TODO do we free the space for existing? - } catch (bad_alloc& e) { - return OpStatus::OUT_OF_MEMORY; - } + auto op_res = db_slice.AddOrFind(op_args_.db_cntx, key); + RETURN_ON_BAD_STATUS(op_res); + auto& add_res = *op_res; PrimeIterator it = add_res.it; if (!add_res.is_new) { diff --git a/src/server/zset_family.cc b/src/server/zset_family.cc index ce1da02b1..03ed3764c 100644 --- a/src/server/zset_family.cc +++ b/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 FindZEntry(const ZParams& zparams, const OpArgs& return db_slice.FindMutable(op_args.db_cntx, key, OBJ_ZSET); } - DbSlice::AddOrFindResult add_res; - - try { - add_res = db_slice.AddOrFind(op_args.db_cntx, key); - } catch (bad_alloc&) { - return OpStatus::OUT_OF_MEMORY; - } + auto op_res = db_slice.AddOrFind(op_args.db_cntx, key); + RETURN_ON_BAD_STATUS(op_res); + auto& add_res = *op_res; PrimeIterator& it = add_res.it; PrimeValue& pv = it->second;