From: Samarah Date: Mon, 19 Jun 2023 13:44:01 +0000 (-0400) Subject: RGW: Fix find_client method; rgw crash issue X-Git-Tag: testing/wip-batrick-testing-20240411.154038~45^2~80 X-Git-Url: http://git.apps.os.sepia.ceph.com/?a=commitdiff_plain;h=13851441d1d4c10f25f53f5ae366f7bd613142d7;p=ceph-ci.git RGW: Fix find_client method; rgw crash issue Signed-off-by: Samarah --- diff --git a/src/rgw/driver/d4n/d4n_policy.cc b/src/rgw/driver/d4n/d4n_policy.cc index 22ea996a418..f648519ab50 100644 --- a/src/rgw/driver/d4n/d4n_policy.cc +++ b/src/rgw/driver/d4n/d4n_policy.cc @@ -5,8 +5,8 @@ namespace rgw { namespace d4n { -int CachePolicy::find_client(const DoutPrefixProvider* dpp) { - if (client.is_connected()) +int CachePolicy::find_client(const DoutPrefixProvider* dpp, cpp_redis::client* client) { + if (client->is_connected()) return 0; if (get_addr().host == "" || get_addr().port == 0) { @@ -14,9 +14,9 @@ int CachePolicy::find_client(const DoutPrefixProvider* dpp) { return EDESTADDRREQ; } - client.connect(get_addr().host, get_addr().port, nullptr); + client->connect(get_addr().host, get_addr().port, nullptr); - if (!client.is_connected()) + if (!client->is_connected()) return ECONNREFUSED; return 0; @@ -248,17 +248,16 @@ CacheBlock LFUDAPolicy::find_victim(const DoutPrefixProvider* dpp, rgw::cache::C } int LFUDAPolicy::get_block(const DoutPrefixProvider* dpp, CacheBlock* block, rgw::cache::CacheDriver* cacheNode) { - std::string key = "rgw-object:" + block->cacheObj.objName + ":directory"; +/* std::string key = "rgw-object:" + block->cacheObj.objName + ":directory"; std::string localWeightStr = cacheNode->get_attr(dpp, block->cacheObj.objName, "localWeight"); // change to block name eventually -Sam int localWeight = -1; if (!client.is_connected()) - find_client(dpp); + find_client(dpp, &client); if (localWeightStr.empty()) { // figure out where to set local weight -Sam - /* Local weight hasn't been set */ int ret = cacheNode->set_attr(dpp, block->cacheObj.objName, "localWeight", std::to_string(get_age())); - localWeight = 0; + localWeight = get_age(); if (ret < 0) return -1; @@ -268,7 +267,7 @@ int LFUDAPolicy::get_block(const DoutPrefixProvider* dpp, CacheBlock* block, rgw int age = get_age(); - if (cacheNode->key_exists(dpp, block->cacheObj.objName)) { /* Local copy */ + if (cacheNode->key_exists(dpp, block->cacheObj.objName)) { localWeight += age; } else { std::string hosts; @@ -294,20 +293,19 @@ int LFUDAPolicy::get_block(const DoutPrefixProvider* dpp, CacheBlock* block, rgw } // should not hold local cache IP if in this else statement -Sam - if (hosts.length() > 0) { /* Remote copy */ + if (hosts.length() > 0) { int globalWeight = get_global_weight(key); globalWeight += age; if (set_global_weight(key, globalWeight)) return -1; - } else { /* No remote copy */ + } else { // do I need to add the block to the local cache here? -Sam // update hosts list for block as well? check read workflow -Sam localWeight += age; return cacheNode->set_attr(dpp, block->cacheObj.objName, "localWeight", std::to_string(localWeight)); } - } - return 0; + }*/ } uint64_t LFUDAPolicy::eviction(const DoutPrefixProvider* dpp, rgw::cache::CacheDriver* cacheNode) { diff --git a/src/rgw/driver/d4n/d4n_policy.h b/src/rgw/driver/d4n/d4n_policy.h index 15765c94c38..dbf198517ba 100644 --- a/src/rgw/driver/d4n/d4n_policy.h +++ b/src/rgw/driver/d4n/d4n_policy.h @@ -26,7 +26,7 @@ class CachePolicy { addr.host = cct->_conf->rgw_d4n_host; addr.port = cct->_conf->rgw_d4n_port; } - virtual int find_client(const DoutPrefixProvider* dpp) = 0; + virtual int find_client(const DoutPrefixProvider* dpp, cpp_redis::client* client) = 0; virtual int exist_key(std::string key) = 0; virtual Address get_addr() { return addr; } virtual int get_block(const DoutPrefixProvider* dpp, CacheBlock* block, rgw::cache::CacheDriver* cacheNode) = 0; @@ -48,7 +48,7 @@ class LFUDAPolicy : public CachePolicy { int get_min_avg_weight(); CacheBlock find_victim(const DoutPrefixProvider* dpp, rgw::cache::CacheDriver* cacheNode); - virtual int find_client(const DoutPrefixProvider* dpp) override { return CachePolicy::find_client(dpp); } + virtual int find_client(const DoutPrefixProvider* dpp, cpp_redis::client* client) override { return CachePolicy::find_client(dpp, client); } virtual int exist_key(std::string key) override { return CachePolicy::exist_key(key); } virtual int get_block(const DoutPrefixProvider* dpp, CacheBlock* block, rgw::cache::CacheDriver* cacheNode) override; virtual uint64_t eviction(const DoutPrefixProvider* dpp, rgw::cache::CacheDriver* cacheNode) override; diff --git a/src/rgw/driver/d4n/rgw_sal_d4n.cc b/src/rgw/driver/d4n/rgw_sal_d4n.cc index 9e5474ed472..e5fdffc41b0 100644 --- a/src/rgw/driver/d4n/rgw_sal_d4n.cc +++ b/src/rgw/driver/d4n/rgw_sal_d4n.cc @@ -412,6 +412,7 @@ int D4NFilterObject::D4NFilterReadOp::iterate(const DoutPrefixProvider* dpp, int /* Local cache check */ if (source->driver->get_policy_driver()->cacheDriver->key_exists(dpp, oid)) { // Entire object for now -Sam ret = source->driver->get_policy_driver()->cacheDriver->get(dpp, source->get_key().get_oid(), ofs, len, bl, source->get_attrs()); + dout(0) << "Sam: correct" << dendl; cb->handle_data(bl, ofs, len); } else { /* Block directory check */ diff --git a/src/rgw/rgw_redis_driver.cc b/src/rgw/rgw_redis_driver.cc index e76a354e0c5..40ba31a1926 100644 --- a/src/rgw/rgw_redis_driver.cc +++ b/src/rgw/rgw_redis_driver.cc @@ -414,7 +414,7 @@ int RedisDriver::get_attrs(const DoutPrefixProvider* dpp, const std::string& key /* Retrieve existing values from cache */ try { client.hgetall(entryName, [&getFields](cpp_redis::reply &reply) { - if (reply.is_array()) { + if (reply.is_array()) { auto arr = reply.as_array(); if (!arr[0].is_null()) {