From 318dc19652db32f05748ff86930b4d8f8640d9f7 Mon Sep 17 00:00:00 2001 From: Pritha Srivastava Date: Thu, 24 Aug 2023 09:57:07 +0530 Subject: [PATCH] rgw/cache: this commit removes indexing from ssd and redis driver and squashes the following commits. rgw/cache: modifications to the ssd cache driver after removing indexing from the cache backend. rgw/cache: modifications to redis cache driver, after removing indexing from cache backend. rgw/cache: modifications to policy driver code, commenting out some pieces which refer to cache apis. Those apis have been removed as part of modification to remove indexing from the cache backend. These need to appropriately re-instated, once we finalize which module will contain the data structures related to cache policy. Signed-off-by: Pritha Srivastava --- src/rgw/driver/d4n/d4n_policy.cc | 10 +- src/rgw/rgw_redis_driver.cc | 332 +++++++++++-------------------- src/rgw/rgw_redis_driver.h | 8 - src/rgw/rgw_ssd_driver.cc | 46 +---- src/rgw/rgw_ssd_driver.h | 10 - 5 files changed, 127 insertions(+), 279 deletions(-) diff --git a/src/rgw/driver/d4n/d4n_policy.cc b/src/rgw/driver/d4n/d4n_policy.cc index 13da777d15b06..6283779f4aee3 100644 --- a/src/rgw/driver/d4n/d4n_policy.cc +++ b/src/rgw/driver/d4n/d4n_policy.cc @@ -214,6 +214,7 @@ int LFUDAPolicy::get_min_avg_weight() { } CacheBlock LFUDAPolicy::find_victim(const DoutPrefixProvider* dpp, rgw::cache::CacheDriver* cacheNode) { + #if 0 std::vector entries = cacheNode->list_entries(dpp); std::string victimName; int minWeight = INT_MAX; @@ -243,7 +244,8 @@ CacheBlock LFUDAPolicy::find_victim(const DoutPrefixProvider* dpp, rgw::cache::C if (ret < 0) return {}; - + #endif + CacheBlock victimBlock; return victimBlock; } @@ -267,7 +269,8 @@ int LFUDAPolicy::get_block(const DoutPrefixProvider* dpp, CacheBlock* block, rgw int age = get_age(); - if (cacheNode->key_exists(dpp, block->cacheObj.objName)) { /* Local copy */ + bool key_exists = true; //cacheNode->key_exists(dpp, block->cacheObj.objName) //TODO- correct this + if (key_exists) { /* Local copy */ localWeight += age; } else { std::string hosts; @@ -371,7 +374,8 @@ uint64_t LFUDAPolicy::eviction(const DoutPrefixProvider* dpp, rgw::cache::CacheD int ret = cacheNode->delete_data(dpp, victim.cacheObj.objName); if (!ret) { - ret = set_min_avg_weight(avgWeight - (localWeight/cacheNode->get_num_entries(dpp)), ""/*local cache location*/); // Where else must this be set? -Sam + uint64_t num_entries = 100; //cacheNode->get_num_entries(dpp) TODO - correct this + ret = set_min_avg_weight(avgWeight - (localWeight/num_entries), ""/*local cache location*/); // Where else must this be set? -Sam if (!ret) { int age = get_age(); diff --git a/src/rgw/rgw_redis_driver.cc b/src/rgw/rgw_redis_driver.cc index 83c1bc64e3f80..f02c40e745230 100644 --- a/src/rgw/rgw_redis_driver.cc +++ b/src/rgw/rgw_redis_driver.cc @@ -43,28 +43,6 @@ int RedisDriver::find_client(const DoutPrefixProvider* dpp) return 0; } -int RedisDriver::insert_entry(const DoutPrefixProvider* dpp, std::string key, off_t offset, uint64_t len) -{ - auto ret = entries.emplace(key, Entry(key, offset, len)); - return ret.second; -} - -std::optional RedisDriver::get_entry(const DoutPrefixProvider* dpp, std::string key) -{ - auto iter = entries.find(key); - - if (iter != entries.end()) { - return iter->second; - } - - return std::nullopt; -} - -int RedisDriver::remove_entry(const DoutPrefixProvider* dpp, std::string key) -{ - return entries.erase(key); -} - int RedisDriver::add_partition_info(Partition& info) { std::string key = info.name + info.type; @@ -79,44 +57,6 @@ int RedisDriver::remove_partition_info(Partition& info) return partitions.erase(key); } -bool RedisDriver::key_exists(const DoutPrefixProvider* dpp, const std::string& key) -{ - int result; - std::string entry = partition_info.location + key; - std::vector keys; - keys.push_back(entry); - - if (!client.is_connected()) - find_client(dpp); - - try { - client.exists(keys, [&result](cpp_redis::reply &reply) { - if (reply.is_integer()) { - result = reply.as_integer(); - } - }); - - client.sync_commit(std::chrono::milliseconds(1000)); - } catch(std::exception &e) {} - - return result; -} - -std::vector RedisDriver::list_entries(const DoutPrefixProvider* dpp) -{ - std::vector result; - - for (auto it = entries.begin(); it != entries.end(); ++it) - result.push_back(it->second); - - return result; -} - -size_t RedisDriver::get_num_entries(const DoutPrefixProvider* dpp) -{ - return entries.size(); -} - /* uint64_t RedisDriver::get_free_space(const DoutPrefixProvider* dpp) { @@ -247,7 +187,7 @@ int RedisDriver::put(const DoutPrefixProvider* dpp, const std::string& key, buff return -1; } - return insert_entry(dpp, key, 0, len); // why is offset necessarily 0? -Sam + return 0; // why is offset necessarily 0? -Sam } int RedisDriver::get(const DoutPrefixProvider* dpp, const std::string& key, off_t offset, uint64_t len, bufferlist& bl, rgw::sal::Attrs& attrs) @@ -257,9 +197,8 @@ int RedisDriver::get(const DoutPrefixProvider* dpp, const std::string& key, off_ if (!client.is_connected()) find_client(dpp); - if (key_exists(dpp, key)) { /* Retrieve existing values from cache */ - try { + try { client.hgetall(entry, [&bl, &attrs](cpp_redis::reply &reply) { if (reply.is_array()) { auto arr = reply.as_array(); @@ -283,10 +222,7 @@ int RedisDriver::get(const DoutPrefixProvider* dpp, const std::string& key, off_ } catch(std::exception &e) { return -1; } - } else { - ldpp_dout(dpp, 20) << "RGW Redis Cache: Object was not retrievable." << dendl; - return -2; - } + return 0; } @@ -299,18 +235,16 @@ int RedisDriver::append_data(const DoutPrefixProvider* dpp, const::std::string& if (!client.is_connected()) find_client(dpp); - if (key_exists(dpp, key)) { - try { - client.hget(entry, "data", [&value](cpp_redis::reply &reply) { - if (!reply.is_null()) { - value = reply.as_string(); - } - }); + try { + client.hget(entry, "data", [&value](cpp_redis::reply &reply) { + if (!reply.is_null()) { + value = reply.as_string(); + } + }); - client.sync_commit(std::chrono::milliseconds(1000)); - } catch(std::exception &e) { - return -1; - } + client.sync_commit(std::chrono::milliseconds(1000)); + } catch(std::exception &e) { + return -1; } try { // do we want key check here? -Sam @@ -345,10 +279,9 @@ int RedisDriver::delete_data(const DoutPrefixProvider* dpp, const::std::string& if (!client.is_connected()) find_client(dpp); - if (key_exists(dpp, key)) { - int exists = -2; + int exists = -2; - try { + try { client.hexists(entry, "data", [&exists](cpp_redis::reply &reply) { if (!reply.is_null()) { exists = reply.as_integer(); @@ -376,8 +309,6 @@ int RedisDriver::delete_data(const DoutPrefixProvider* dpp, const::std::string& if (!result) { return -1; - } else { - return remove_entry(dpp, key); } } catch(std::exception &e) { return -1; @@ -385,9 +316,7 @@ int RedisDriver::delete_data(const DoutPrefixProvider* dpp, const::std::string& } else { return 0; /* No delete was necessary */ } - } else { - return 0; /* No delete was necessary */ - } + return 0; } int RedisDriver::get_attrs(const DoutPrefixProvider* dpp, const std::string& key, rgw::sal::Attrs& attrs) @@ -397,8 +326,7 @@ int RedisDriver::get_attrs(const DoutPrefixProvider* dpp, const std::string& key if (!client.is_connected()) find_client(dpp); - if (key_exists(dpp, key)) { - try { + try { client.hgetall(entry, [&attrs](cpp_redis::reply &reply) { if (reply.is_array()) { auto arr = reply.as_array(); @@ -420,10 +348,6 @@ int RedisDriver::get_attrs(const DoutPrefixProvider* dpp, const std::string& key } catch(std::exception &e) { return -1; } - } else { - ldpp_dout(dpp, 20) << "RGW Redis Cache: Object was not retrievable." << dendl; - return -2; - } return 0; } @@ -438,29 +362,24 @@ int RedisDriver::set_attrs(const DoutPrefixProvider* dpp, const std::string& key if (!client.is_connected()) find_client(dpp); - if (key_exists(dpp, key)) { - /* Every attr set will be treated as new */ - try { - std::string result; - auto redisAttrs = build_attrs(&attrs); - - client.hmset(entry, redisAttrs, [&result](cpp_redis::reply &reply) { - if (!reply.is_null()) { - result = reply.as_string(); - } - }); - - client.sync_commit(std::chrono::milliseconds(1000)); + /* Every attr set will be treated as new */ + try { + std::string result; + auto redisAttrs = build_attrs(&attrs); - if (result != "OK") { - return -1; + client.hmset(entry, redisAttrs, [&result](cpp_redis::reply &reply) { + if (!reply.is_null()) { + result = reply.as_string(); } - } catch(std::exception &e) { - return -1; + }); + + client.sync_commit(std::chrono::milliseconds(1000)); + + if (result != "OK") { +return -1; } - } else { - ldpp_dout(dpp, 20) << "RGW Redis Cache: Object was not retrievable." << dendl; - return -2; + } catch(std::exception &e) { + return -1; } return 0; @@ -473,28 +392,23 @@ int RedisDriver::update_attrs(const DoutPrefixProvider* dpp, const std::string& if (!client.is_connected()) find_client(dpp); - if (key_exists(dpp, key)) { - try { - std::string result; - auto redisAttrs = build_attrs(&attrs); + try { + std::string result; + auto redisAttrs = build_attrs(&attrs); - client.hmset(entry, redisAttrs, [&result](cpp_redis::reply &reply) { - if (!reply.is_null()) { - result = reply.as_string(); - } - }); + client.hmset(entry, redisAttrs, [&result](cpp_redis::reply &reply) { + if (!reply.is_null()) { + result = reply.as_string(); + } + }); - client.sync_commit(std::chrono::milliseconds(1000)); + client.sync_commit(std::chrono::milliseconds(1000)); - if (result != "OK") { - return -1; - } - } catch(std::exception &e) { + if (result != "OK") { return -1; } - } else { - ldpp_dout(dpp, 20) << "RGW Redis Cache: Object was not retrievable." << dendl; - return -2; + } catch(std::exception &e) { + return -1; } return 0; @@ -507,55 +421,53 @@ int RedisDriver::delete_attrs(const DoutPrefixProvider* dpp, const std::string& if (!client.is_connected()) find_client(dpp); - if (key_exists(dpp, key)) { - std::vector getFields; - - try { - client.hgetall(entry, [&getFields](cpp_redis::reply &reply) { - if (reply.is_array()) { - auto arr = reply.as_array(); - - if (!arr[0].is_null()) { - for (long unsigned int i = 0; i < arr.size() - 1; i += 2) { - getFields.push_back(arr[i].as_string()); - } - } - } - }); + std::vector getFields; - client.sync_commit(std::chrono::milliseconds(1000)); - } catch(std::exception &e) { - return -1; + try { + client.hgetall(entry, [&getFields](cpp_redis::reply &reply) { +if (reply.is_array()) { + auto arr = reply.as_array(); + + if (!arr[0].is_null()) { + for (long unsigned int i = 0; i < arr.size() - 1; i += 2) { + getFields.push_back(arr[i].as_string()); } + } +} + }); + + client.sync_commit(std::chrono::milliseconds(1000)); + } catch(std::exception &e) { + return -1; + } - auto redisAttrs = build_attrs(&del_attrs); - std::vector redisFields; + auto redisAttrs = build_attrs(&del_attrs); + std::vector redisFields; - std::transform(begin(redisAttrs), end(redisAttrs), std::back_inserter(redisFields), - [](auto const& pair) { return pair.first; }); + std::transform(begin(redisAttrs), end(redisAttrs), std::back_inserter(redisFields), + [](auto const& pair) { return pair.first; }); - /* Only delete attributes that have been stored */ - for (const auto& it : redisFields) { - if (std::find(getFields.begin(), getFields.end(), it) == getFields.end()) { - redisFields.erase(std::find(redisFields.begin(), redisFields.end(), it)); - } + /* Only delete attributes that have been stored */ + for (const auto& it : redisFields) { + if (std::find(getFields.begin(), getFields.end(), it) == getFields.end()) { + redisFields.erase(std::find(redisFields.begin(), redisFields.end(), it)); } + } - try { - int result = 0; + try { + int result = 0; - client.hdel(entry, redisFields, [&result](cpp_redis::reply &reply) { - if (reply.is_integer()) { - result = reply.as_integer(); - } - }); + client.hdel(entry, redisFields, [&result](cpp_redis::reply &reply) { + if (reply.is_integer()) { + result = reply.as_integer(); + } + }); - client.sync_commit(std::chrono::milliseconds(1000)); + client.sync_commit(std::chrono::milliseconds(1000)); - return result - 1; - } catch(std::exception &e) { - return -1; - } + return result - 1; + } catch(std::exception &e) { + return -1; } ldpp_dout(dpp, 20) << "RGW Redis Cache: Object is not in cache." << dendl; @@ -570,42 +482,37 @@ std::string RedisDriver::get_attr(const DoutPrefixProvider* dpp, const std::stri if (!client.is_connected()) find_client(dpp); - if (key_exists(dpp, key)) { - int exists = -2; - std::string getValue; + int exists = -2; + std::string getValue; - /* Ensure field was set */ - try { - client.hexists(entry, attr_name, [&exists](cpp_redis::reply& reply) { - if (!reply.is_null()) { - exists = reply.as_integer(); - } - }); + /* Ensure field was set */ + try { + client.hexists(entry, attr_name, [&exists](cpp_redis::reply& reply) { + if (!reply.is_null()) { + exists = reply.as_integer(); + } + }); - client.sync_commit(std::chrono::milliseconds(1000)); - } catch(std::exception &e) { - return {}; - } - - if (!exists) { - ldpp_dout(dpp, 20) << "RGW Redis Cache: Attribute was not set." << dendl; - return {}; - } + client.sync_commit(std::chrono::milliseconds(1000)); + } catch(std::exception &e) { + return {}; + } + + if (!exists) { + ldpp_dout(dpp, 20) << "RGW Redis Cache: Attribute was not set." << dendl; + return {}; + } - /* Retrieve existing value from cache */ - try { - client.hget(entry, attr_name, [&exists, &attrValue](cpp_redis::reply &reply) { - if (!reply.is_null()) { - attrValue = reply.as_string(); - } - }); + /* Retrieve existing value from cache */ + try { + client.hget(entry, attr_name, [&exists, &attrValue](cpp_redis::reply &reply) { + if (!reply.is_null()) { + attrValue = reply.as_string(); + } + }); - client.sync_commit(std::chrono::milliseconds(1000)); - } catch(std::exception &e) { - return {}; - } - } else { - ldpp_dout(dpp, 20) << "RGW Redis Cache: Object is not in cache." << dendl; + client.sync_commit(std::chrono::milliseconds(1000)); + } catch(std::exception &e) { return {}; } @@ -620,22 +527,17 @@ int RedisDriver::set_attr(const DoutPrefixProvider* dpp, const std::string& key, if (!client.is_connected()) find_client(dpp); - if (key_exists(dpp, key)) { - /* Every attr set will be treated as new */ - try { - client.hset(entry, attr_name, attrVal, [&result](cpp_redis::reply& reply) { - if (!reply.is_null()) { - result = reply.as_integer(); - } - }); + /* Every attr set will be treated as new */ + try { + client.hset(entry, attr_name, attrVal, [&result](cpp_redis::reply& reply) { + if (!reply.is_null()) { + result = reply.as_integer(); + } + }); - client.sync_commit(std::chrono::milliseconds(1000)); - } catch(std::exception &e) { - return -1; - } - } else { - ldpp_dout(dpp, 20) << "RGW Redis Cache: Object is not in cache." << dendl; - return -2; + client.sync_commit(std::chrono::milliseconds(1000)); + } catch(std::exception &e) { + return -1; } return result - 1; diff --git a/src/rgw/rgw_redis_driver.h b/src/rgw/rgw_redis_driver.h index cbaae3c2820bd..4c12578ab1769 100644 --- a/src/rgw/rgw_redis_driver.h +++ b/src/rgw/rgw_redis_driver.h @@ -25,10 +25,6 @@ class RedisDriver : public CacheDriver { remove_partition_info(partition_info); } - /* Entry */ - virtual bool key_exists(const DoutPrefixProvider* dpp, const std::string& key) override; - virtual std::vector list_entries(const DoutPrefixProvider* dpp) override; - virtual size_t get_num_entries(const DoutPrefixProvider* dpp) override; //int update_local_weight(const DoutPrefixProvider* dpp, std::string key, int localWeight); // may need to exist for base class -Sam /* Partition */ @@ -71,16 +67,12 @@ class RedisDriver : public CacheDriver { cpp_redis::client client; rgw::d4n::Address addr; static std::unordered_map partitions; - std::unordered_map entries; Partition partition_info; uint64_t free_space; uint64_t outstanding_write_size; CephContext* cct; int find_client(const DoutPrefixProvider* dpp); - int insert_entry(const DoutPrefixProvider* dpp, std::string key, off_t offset, uint64_t len); - std::optional get_entry(const DoutPrefixProvider* dpp, std::string key); - int remove_entry(const DoutPrefixProvider* dpp, std::string key); int add_partition_info(Partition& info); int remove_partition_info(Partition& info); diff --git a/src/rgw/rgw_ssd_driver.cc b/src/rgw/rgw_ssd_driver.cc index 317590afcb01b..2f89fef73cfc0 100644 --- a/src/rgw/rgw_ssd_driver.cc +++ b/src/rgw/rgw_ssd_driver.cc @@ -50,38 +50,8 @@ int SSDDriver::remove_partition_info(Partition& info) return partitions.erase(key); } -int SSDDriver::insert_entry(const DoutPrefixProvider* dpp, std::string key, off_t offset, uint64_t len) -{ - auto ret = entries.emplace(key, Entry(key, offset, len)); - return ret.second; -} - -int SSDDriver::remove_entry(const DoutPrefixProvider* dpp, std::string key) -{ - return entries.erase(key); -} - -std::optional SSDDriver::get_entry(const DoutPrefixProvider* dpp, std::string key) -{ - auto iter = entries.find(key); - if (iter != entries.end()) { - return iter->second; - } - - return std::nullopt; -} - -std::vector SSDDriver::list_entries(const DoutPrefixProvider* dpp) -{ - std::vector entries_v; - for (auto& it : entries) { - entries_v.emplace_back(it.second); - } - return entries_v; -} - SSDDriver::SSDDriver(Partition& partition_info) : partition_info(partition_info), - free_space(partition_info.size), outstanding_write_size(0) + free_space(partition_info.size) { add_partition_info(partition_info); } @@ -130,10 +100,6 @@ int SSDDriver::initialize(CephContext* cct, const DoutPrefixProvider* dpp) int SSDDriver::put(const DoutPrefixProvider* dpp, const std::string& key, bufferlist& bl, uint64_t len, rgw::sal::Attrs& attrs) { - if (key_exists(dpp, key)) { - return 0; - } - std::string location = partition_info.location + key; ldpp_dout(dpp, 20) << __func__ << "(): location=" << location << dendl; @@ -170,15 +136,11 @@ int SSDDriver::put(const DoutPrefixProvider* dpp, const std::string& key, buffer } } - return insert_entry(dpp, key, 0, len); + return 0; } int SSDDriver::get(const DoutPrefixProvider* dpp, const std::string& key, off_t offset, uint64_t len, bufferlist& bl, rgw::sal::Attrs& attrs) { - if (!key_exists(dpp, key)) { - return -ENOENT; - } - char buffer[len]; std::string location = partition_info.location + key; @@ -278,8 +240,6 @@ void SSDDriver::libaio_write_completion_cb(AsyncWriteRequest* c) { efs::space_info space = efs::space(partition_info.location); this->free_space = space.available; - - insert_entry(c->dpp, c->key, 0, c->cb->aio_nbytes); } int SSDDriver::put_async(const DoutPrefixProvider* dpp, const std::string& key, bufferlist& bl, uint64_t len, rgw::sal::Attrs& attrs) @@ -318,7 +278,7 @@ int SSDDriver::delete_data(const DoutPrefixProvider* dpp, const::std::string& ke efs::space_info space = efs::space(partition_info.location); this->free_space = space.available; - return remove_entry(dpp, key); + return 0; } int SSDDriver::append_data(const DoutPrefixProvider* dpp, const::std::string& key, bufferlist& bl_data) diff --git a/src/rgw/rgw_ssd_driver.h b/src/rgw/rgw_ssd_driver.h index 6f0c4773c7586..e012180827a20 100644 --- a/src/rgw/rgw_ssd_driver.h +++ b/src/rgw/rgw_ssd_driver.h @@ -26,11 +26,6 @@ public: virtual int set_attr(const DoutPrefixProvider* dpp, const std::string& key, const std::string& attr_name, const std::string& attr_val) override; int delete_attr(const DoutPrefixProvider* dpp, const std::string& key, const std::string& attr_name); - /* Entry */ - virtual bool key_exists(const DoutPrefixProvider* dpp, const std::string& key) override { return entries.count(key) != 0; } - virtual std::vector list_entries(const DoutPrefixProvider* dpp) override; - virtual size_t get_num_entries(const DoutPrefixProvider* dpp) override { return entries.size(); } - /* Partition */ virtual Partition get_current_partition_info(const DoutPrefixProvider* dpp) override { return partition_info; } virtual uint64_t get_free_space(const DoutPrefixProvider* dpp) override { return free_space; } @@ -50,17 +45,12 @@ public: protected: inline static std::unordered_map partitions; - std::unordered_map entries; Partition partition_info; uint64_t free_space; - uint64_t outstanding_write_size; CephContext* cct; int add_partition_info(Partition& info); int remove_partition_info(Partition& info); - int insert_entry(const DoutPrefixProvider* dpp, std::string key, off_t offset, uint64_t len); - int remove_entry(const DoutPrefixProvider* dpp, std::string key); - std::optional get_entry(const DoutPrefixProvider* dpp, std::string key); private: -- 2.39.5