From: Samarah Date: Fri, 17 May 2024 19:19:12 +0000 (+0000) Subject: rgw/d4n: Squash the following commits related to directory, eviction and filter drive... X-Git-Tag: v20.3.0~8^2~28 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=1dac39856e129f791b10f397673d1c70373e5292;p=ceph.git rgw/d4n: Squash the following commits related to directory, eviction and filter driver logs: 1. d4n/directory: Remove `blockHosts` and change `objHosts` to store the hosts for the block 2. d4n/directory: Remove `dirtyBlock` metadata and use the cacheObj's `dirty` flag in block logic 3. rgw/d4n: This commit introduces the following changes: a. Fix `get_victim_block` method to correctly handle version b. Return `0` instead of error for non-error circumstances in `eviction` c. Switch `increase` to `decrease` for heap after localWeight update in `eviction` d. Update filter writer logs 4. d4n/filter: Make minor adjustments 5. test/d4n: Update `update_field` test calls Signed-off-by: Samarah --- diff --git a/src/rgw/driver/d4n/d4n_directory.cc b/src/rgw/driver/d4n/d4n_directory.cc index d90bcdcf8149..5c39f6efc4da 100644 --- a/src/rgw/driver/d4n/d4n_directory.cc +++ b/src/rgw/driver/d4n/d4n_directory.cc @@ -112,7 +112,7 @@ int ObjectDirectory::set(const DoutPrefixProvider* dpp, CacheObj* object, option return -EINVAL; } redisValues.push_back(std::to_string(object->dirty)); - redisValues.push_back("objHosts"); + redisValues.push_back("hosts"); for (auto const& host : object->hostsList) { if (endpoint.empty()) @@ -155,7 +155,7 @@ int ObjectDirectory::get(const DoutPrefixProvider* dpp, CacheObj* object, option fields.push_back("bucketName"); fields.push_back("creationTime"); fields.push_back("dirty"); - fields.push_back("objHosts"); + fields.push_back("hosts"); try { boost::system::error_code ec; @@ -178,7 +178,7 @@ int ObjectDirectory::get(const DoutPrefixProvider* dpp, CacheObj* object, option object->objName = std::get<0>(resp).value()[0]; object->bucketName = std::get<0>(resp).value()[1]; object->creationTime = std::get<0>(resp).value()[2]; - object->dirty = std::stoi(std::get<0>(resp).value()[3]); + object->dirty = (std::stoi(std::get<0>(resp).value()[3]) != 0); boost::split(object->hostsList, std::get<0>(resp).value()[4], boost::is_any_of("_")); } catch (std::exception &e) { ldpp_dout(dpp, 0) << "ObjectDirectory::" << __func__ << "() ERROR: " << e.what() << dendl; @@ -257,7 +257,7 @@ int ObjectDirectory::update_field(const DoutPrefixProvider* dpp, CacheObj* objec if (exist_key(dpp, object, y)) { try { - if (field == "objHosts") { + if (field == "hosts") { /* Append rather than overwrite */ ldpp_dout(dpp, 20) << "ObjectDirectory::" << __func__ << "() Appending to hosts list." << dendl; @@ -280,7 +280,8 @@ int ObjectDirectory::update_field(const DoutPrefixProvider* dpp, CacheObj* objec } else if (field == "dirty") { int ret = -1; if ((ret = check_bool(value)) != -EINVAL) { - value = std::to_string((ret != 0)); + bool val = (ret != 0); + value = std::to_string(val); } else { ldpp_dout(dpp, 0) << "ObjectDirectory::" << __func__ << "() ERROR: Invalid bool value" << dendl; return -EINVAL; @@ -352,40 +353,18 @@ int BlockDirectory::set(const DoutPrefixProvider* dpp, CacheBlock* block, option redisValues.push_back(std::to_string(block->blockID)); redisValues.push_back("version"); redisValues.push_back(block->version); - redisValues.push_back("dirtyBlock"); - int ret = -1; - if ((ret = check_bool(std::to_string(block->dirty))) != -EINVAL) { - block->dirty = (ret != 0); - } else { - ldpp_dout(dpp, 0) << "BlockDirectory::" << __func__ << "() ERROR: Invalid bool value" << dendl; - return -EINVAL; - } - redisValues.push_back(std::to_string(block->dirty)); redisValues.push_back("size"); redisValues.push_back(std::to_string(block->size)); redisValues.push_back("globalWeight"); redisValues.push_back(std::to_string(block->globalWeight)); - redisValues.push_back("blockHosts"); - - for (auto const& host : block->hostsList) { - if (endpoint.empty()) - endpoint = host + "_"; - else - endpoint = endpoint + host + "_"; - } - - if (!endpoint.empty()) - endpoint.pop_back(); - - redisValues.push_back(endpoint); - redisValues.push_back("objName"); redisValues.push_back(block->cacheObj.objName); redisValues.push_back("bucketName"); redisValues.push_back(block->cacheObj.bucketName); redisValues.push_back("creationTime"); redisValues.push_back(block->cacheObj.creationTime); - redisValues.push_back("dirtyObj"); + redisValues.push_back("dirty"); + int ret = -1; if ((ret = check_bool(std::to_string(block->cacheObj.dirty))) != -EINVAL) { block->cacheObj.dirty = (ret != 0); } else { @@ -393,7 +372,7 @@ int BlockDirectory::set(const DoutPrefixProvider* dpp, CacheBlock* block, option return -EINVAL; } redisValues.push_back(std::to_string(block->cacheObj.dirty)); - redisValues.push_back("objHosts"); + redisValues.push_back("hosts"); endpoint.clear(); for (auto const& host : block->cacheObj.hostsList) { @@ -437,16 +416,14 @@ int BlockDirectory::get(const DoutPrefixProvider* dpp, CacheBlock* block, option fields.push_back("blockID"); fields.push_back("version"); - fields.push_back("dirtyBlock"); fields.push_back("size"); fields.push_back("globalWeight"); - fields.push_back("blockHosts"); fields.push_back("objName"); fields.push_back("bucketName"); fields.push_back("creationTime"); - fields.push_back("dirtyObj"); - fields.push_back("objHosts"); + fields.push_back("dirty"); + fields.push_back("hosts"); try { boost::system::error_code ec; @@ -468,15 +445,13 @@ int BlockDirectory::get(const DoutPrefixProvider* dpp, CacheBlock* block, option block->blockID = std::stoull(std::get<0>(resp).value().value()[0]); block->version = std::get<0>(resp).value().value()[1]; - block->dirty = std::stoi(std::get<0>(resp).value().value()[2]); - block->size = std::stoull(std::get<0>(resp).value().value()[3]); - block->globalWeight = std::stoull(std::get<0>(resp).value().value()[4]); - boost::split(block->hostsList, std::get<0>(resp).value().value()[5], boost::is_any_of("_")); - block->cacheObj.objName = std::get<0>(resp).value().value()[6]; - block->cacheObj.bucketName = std::get<0>(resp).value().value()[7]; - block->cacheObj.creationTime = std::get<0>(resp).value().value()[8]; - block->cacheObj.dirty = std::stoi(std::get<0>(resp).value().value()[9]); - boost::split(block->cacheObj.hostsList, std::get<0>(resp).value().value()[10], boost::is_any_of("_")); + block->size = std::stoull(std::get<0>(resp).value().value()[2]); + block->globalWeight = std::stoull(std::get<0>(resp).value().value()[3]); + block->cacheObj.objName = std::get<0>(resp).value().value()[4]; + block->cacheObj.bucketName = std::get<0>(resp).value().value()[5]; + block->cacheObj.creationTime = std::get<0>(resp).value().value()[6]; + block->cacheObj.dirty =(std::stoi(std::get<0>(resp).value().value()[7]) != 0); + boost::split(block->cacheObj.hostsList, std::get<0>(resp).value().value()[8], boost::is_any_of("_")); } catch (std::exception &e) { ldpp_dout(dpp, 0) << "BlockDirectory::" << __func__ << "() ERROR: " << e.what() << dendl; return -EINVAL; @@ -554,7 +529,7 @@ int BlockDirectory::update_field(const DoutPrefixProvider* dpp, CacheBlock* bloc if (exist_key(dpp, block, y)) { try { - if (field == "blockHosts") { + if (field == "hosts") { /* Append rather than overwrite */ ldpp_dout(dpp, 20) << "BlockDirectory::" << __func__ << "() Appending to hosts list." << dendl; @@ -574,10 +549,11 @@ int BlockDirectory::update_field(const DoutPrefixProvider* dpp, CacheBlock* bloc std::get<0>(resp).value().value() += "_"; std::get<0>(resp).value().value() += value; value = std::get<0>(resp).value().value(); - } else if (field == "dirtyObj" || field == "dirtyBlock") { + } else if (field == "dirty") { int ret = -1; if ((ret = check_bool(value)) != -EINVAL) { - value = std::to_string((ret != 0)); + bool val = (ret != 0); + value = std::to_string(val); } else { ldpp_dout(dpp, 0) << "BlockDirectory::" << __func__ << "() ERROR: Invalid bool value" << dendl; return -EINVAL; @@ -615,7 +591,7 @@ int BlockDirectory::remove_host(const DoutPrefixProvider* dpp, CacheBlock* block boost::system::error_code ec; response< std::optional > resp; request req; - req.push("HGET", key, "blockHosts"); + req.push("HGET", key, "hosts"); redis_exec(conn, ec, req, resp, y); @@ -644,7 +620,7 @@ int BlockDirectory::remove_host(const DoutPrefixProvider* dpp, CacheBlock* block } if (result.length() == 0) /* Last host, delete entirely */ - return del(dpp, block, y); + return del(dpp, block, y); value = result; } @@ -653,7 +629,7 @@ int BlockDirectory::remove_host(const DoutPrefixProvider* dpp, CacheBlock* block boost::system::error_code ec; response resp; request req; - req.push("HSET", key, "blockHosts", value); + req.push("HSET", key, "hosts", value); redis_exec(conn, ec, req, resp, y); diff --git a/src/rgw/driver/d4n/d4n_directory.h b/src/rgw/driver/d4n/d4n_directory.h index ad1fc6e775ef..250b7ed61bd4 100644 --- a/src/rgw/driver/d4n/d4n_directory.h +++ b/src/rgw/driver/d4n/d4n_directory.h @@ -26,10 +26,9 @@ struct CacheBlock { CacheObj cacheObj; uint64_t blockID; std::string version; - bool dirty{false}; uint64_t size; /* Block size in bytes */ int globalWeight = 0; /* LFUDA policy variable */ - std::unordered_set hostsList; /* List of hostnames of block locations */ + /* Blocks use the cacheObj's dirty and hostsList metadata to store their dirty flag values and locations in the block directory. */ }; class Directory { diff --git a/src/rgw/driver/d4n/d4n_policy.cc b/src/rgw/driver/d4n/d4n_policy.cc index f73e7d3e66c6..ca1b2826c2ac 100644 --- a/src/rgw/driver/d4n/d4n_policy.cc +++ b/src/rgw/driver/d4n/d4n_policy.cc @@ -236,6 +236,8 @@ CacheBlock* LFUDAPolicy::get_victim_block(const DoutPrefixProvider* dpp, optiona victim->cacheObj.bucketName = key.substr(0, key.find('_')); key.erase(0, key.find('_') + 1); + victim->version = key.substr(0, key.find('_')); + key.erase(0, key.find('_') + 1); victim->cacheObj.objName = key.substr(0, key.find('_')); victim->blockID = entries_heap.top()->offset; victim->size = entries_heap.top()->len; @@ -257,6 +259,9 @@ int LFUDAPolicy::exist_key(std::string key) { } int LFUDAPolicy::eviction(const DoutPrefixProvider* dpp, uint64_t size, optional_yield y) { + if (entries_heap.empty()) + return 0; + uint64_t freeSpace = cacheDriver->get_free_space(dpp); while (freeSpace < size) { // TODO: Think about parallel reads and writes; can this turn into an infinite loop? @@ -265,7 +270,7 @@ int LFUDAPolicy::eviction(const DoutPrefixProvider* dpp, uint64_t size, optional if (victim == nullptr) { ldpp_dout(dpp, 0) << "LFUDAPolicy::" << __func__ << "(): Could not retrieve victim block." << dendl; delete victim; - return -ENOENT; + return 0; // not necessarily an error? -Sam } const std::lock_guard l(lfuda_lock); @@ -278,15 +283,15 @@ int LFUDAPolicy::eviction(const DoutPrefixProvider* dpp, uint64_t size, optional // check dirty flag of entry to be evicted, if the flag is dirty, all entries on the local node are dirty if (it->second->dirty) { ldpp_dout(dpp, 0) << "LFUDAPolicy::" << __func__ << "(): Top entry in min heap is dirty, no entry is available for eviction!" << dendl; - return -ENOENT; + return 0; } int avgWeight = weightSum / entries_map.size(); - if (victim->hostsList.size() == 1 && *(victim->hostsList.begin()) == dpp->get_cct()->_conf->rgw_d4n_l1_datacache_address) { /* Last copy */ + if (victim->cacheObj.hostsList.size() == 1 && *(victim->cacheObj.hostsList.begin()) == dpp->get_cct()->_conf->rgw_d4n_l1_datacache_address) { /* Last copy */ if (victim->globalWeight) { it->second->localWeight += victim->globalWeight; (*it->second->handle)->localWeight = it->second->localWeight; - entries_heap.increase(it->second->handle); + entries_heap.decrease(it->second->handle); // larger value means node must be decreased to maintain min heap if (int ret = cacheDriver->set_attr(dpp, key, "user.rgw.localWeight", std::to_string(it->second->localWeight), y) < 0) { delete victim; @@ -626,7 +631,7 @@ void LFUDAPolicy::cleaning(const DoutPrefixProvider* dpp) } } - op_ret = blockDir->update_field(dpp, &block, "dirtyBlock", "false", null_yield); + op_ret = blockDir->update_field(dpp, &block, "dirty", "false", null_yield); if (op_ret < 0) { ldpp_dout(dpp, 0) << __func__ << "updating dirty flag in block directory for head failed, ret=" << op_ret << dendl; return; @@ -642,7 +647,7 @@ void LFUDAPolicy::cleaning(const DoutPrefixProvider* dpp) return; } - op_ret = blockDir->update_field(dpp, &block, "dirtyObj", "false", null_yield); + op_ret = blockDir->update_field(dpp, &block, "dirty", "false", null_yield); if (op_ret < 0) { ldpp_dout(dpp, 0) << __func__ << "updating dirty flag in block directory failed, ret=" << op_ret << dendl; return; diff --git a/src/rgw/driver/d4n/rgw_sal_d4n.cc b/src/rgw/driver/d4n/rgw_sal_d4n.cc index ad6933a01b20..e7fbadea36bd 100644 --- a/src/rgw/driver/d4n/rgw_sal_d4n.cc +++ b/src/rgw/driver/d4n/rgw_sal_d4n.cc @@ -420,9 +420,7 @@ int D4NFilterObject::set_head_obj_dir_entry(const DoutPrefixProvider* dpp, optio .cacheObj = object, .blockID = 0, .version = this->get_object_version(), - .dirty = dirty, .size = 0, - .hostsList = { dpp->get_cct()->_conf->rgw_d4n_l1_datacache_address }, }; ret = blockDir->set(dpp, &block, y); if (ret < 0) { @@ -445,9 +443,7 @@ int D4NFilterObject::set_head_obj_dir_entry(const DoutPrefixProvider* dpp, optio .cacheObj = version_object, .blockID = 0, .version = this->get_object_version(), - .dirty = dirty, .size = 0, - .hostsList = { dpp->get_cct()->_conf->rgw_d4n_l1_datacache_address }, }; auto ret = blockDir->set(dpp, &version_block, y); @@ -485,8 +481,8 @@ bool D4NFilterObject::check_head_exists_in_cache_get_oid(const DoutPrefixProvide can be determined using the block entry. */ //uniform name for versioned and non-versioned objects, since input for versioned objects might not contain version - ldpp_dout(dpp, 10) << "D4NFilterObject::" << __func__ << "(): Is block dirty: " << block.dirty << dendl; - if (block.dirty) { + ldpp_dout(dpp, 10) << "D4NFilterObject::" << __func__ << "(): Is block dirty: " << block.cacheObj.dirty << dendl; + if (block.cacheObj.dirty) { head_oid_in_cache = "D_" + get_bucket()->get_name() + "_" + version + "_" + get_name(); } else { head_oid_in_cache = get_bucket()->get_name() + "_" + version + "_" + get_name(); @@ -812,7 +808,7 @@ int D4NFilterObject::D4NFilterReadOp::flush(const DoutPrefixProvider* dpp, rgw:: std::string oid_in_cache = prefix + "_" + std::to_string(ofs) + "_" + std::to_string(len); if (source->driver->get_block_dir()->get(dpp, &block, y) == 0){ - if (block.dirty){ + if (block.cacheObj.dirty){ dirty = true; } } @@ -827,9 +823,9 @@ int D4NFilterObject::D4NFilterReadOp::flush(const DoutPrefixProvider* dpp, rgw:: dest_block.cacheObj.dirty = true; //writing to cache dest_block.blockID = ofs; dest_block.size = len; - dest_block.hostsList.insert(dpp->get_cct()->_conf->rgw_d4n_l1_datacache_address); + dest_block.cacheObj.hostsList.insert(dpp->get_cct()->_conf->rgw_d4n_l1_datacache_address); dest_block.version = source->dest_version; - dest_block.dirty = true; + dest_block.cacheObj.dirty = true; std::string key = source->dest_bucket->get_name() + "_" + source->dest_version + "_" + source->dest_object->get_name() + "_" + std::to_string(ofs) + "_" + std::to_string(len); std::string dest_oid_in_cache = "D_" + key; @@ -934,19 +930,19 @@ int D4NFilterObject::D4NFilterReadOp::iterate(const DoutPrefixProvider* dpp, int int ret = -1; if ((ret = source->driver->get_block_dir()->get(dpp, &block, y)) == 0) { - auto it = find(block.hostsList.begin(), block.hostsList.end(), dpp->get_cct()->_conf->rgw_d4n_l1_datacache_address); + auto it = block.cacheObj.hostsList.find(dpp->get_cct()->_conf->rgw_d4n_l1_datacache_address); - if (it != block.hostsList.end()) { /* Local copy */ + if (it != block.cacheObj.hostsList.end()) { /* Local copy */ ldpp_dout(dpp, 20) << "D4NFilterObject::iterate:: " << __func__ << "(): Block found in directory. " << oid_in_cache << dendl; std::string key = oid_in_cache; - ldpp_dout(dpp, 20) << "D4NFilterObject::iterate:: " << __func__ << "(): READ FROM CACHE: block is dirty = " << block.dirty << dendl; + ldpp_dout(dpp, 20) << "D4NFilterObject::iterate:: " << __func__ << "(): READ FROM CACHE: block is dirty = " << block.cacheObj.dirty << dendl; - if (block.dirty == true) { + if (block.cacheObj.dirty == true) { key = "D_" + oid_in_cache; // we keep track of dirty data in the cache for the metadata failure case ldpp_dout(dpp, 20) << "D4NFilterObject::iterate:: " << __func__ << "(): READ FROM CACHE: key=" << key << " data is Dirty." << dendl; } - ldpp_dout(dpp, 20) << "D4NFilterObject::iterate:: " << __func__ << "(): " << __LINE__ << ": READ FROM CACHE: block dirty =" << block.dirty << dendl; + ldpp_dout(dpp, 20) << "D4NFilterObject::iterate:: " << __func__ << "(): " << __LINE__ << ": READ FROM CACHE: block dirty =" << block.cacheObj.dirty << dendl; ldpp_dout(dpp, 20) << "D4NFilterObject::iterate:: " << __func__ << "(): " << __LINE__ << ": READ FROM CACHE: key=" << key << dendl; if (block.version == version) { @@ -970,7 +966,7 @@ int D4NFilterObject::D4NFilterReadOp::iterate(const DoutPrefixProvider* dpp, int if ((r = source->driver->get_block_dir()->remove_host(dpp, &block, dpp->get_cct()->_conf->rgw_d4n_l1_datacache_address, y)) < 0) ldpp_dout(dpp, 0) << "D4NFilterObject::iterate:: " << __func__ << "(): Error: failed to remove incorrect host from block with oid=" << oid_in_cache <<", ret=" << r << dendl; - if ((block.hostsList.size() - 1) > 0 && r == 0) { /* Remote copy */ + if ((block.cacheObj.hostsList.size() - 1) > 0 && r == 0) { /* Remote copy */ ldpp_dout(dpp, 20) << "D4NFilterObject::iterate:: " << __func__ << "(): Block with oid=" << oid_in_cache << " found in remote cache." << dendl; // TODO: Retrieve remotely // Policy decision: should we cache remote blocks locally? @@ -1000,8 +996,8 @@ int D4NFilterObject::D4NFilterReadOp::iterate(const DoutPrefixProvider* dpp, int return r; } } - // if (it != block.hostsList.end()) - } else if (block.hostsList.size()) { /* Remote copy */ + // if (it != block.cacheObj.hostsList.end()) + } else if (block.cacheObj.hostsList.size()) { /* Remote copy */ ldpp_dout(dpp, 20) << "D4NFilterObject::iterate:: " << __func__ << "(): Block found in remote cache. " << oid_in_cache << dendl; // TODO: Retrieve remotely // Policy decision: should we cache remote blocks locally? @@ -1012,9 +1008,9 @@ int D4NFilterObject::D4NFilterReadOp::iterate(const DoutPrefixProvider* dpp, int block.size = obj_max_req_size; if ((ret = source->driver->get_block_dir()->get(dpp, &block, y)) == 0) { - auto it = find(block.hostsList.begin(), block.hostsList.end(), dpp->get_cct()->_conf->rgw_d4n_l1_datacache_address); + auto it = block.cacheObj.hostsList.find(dpp->get_cct()->_conf->rgw_d4n_l1_datacache_address); - if (it != block.hostsList.end()) { /* Local copy */ + if (it != block.cacheObj.hostsList.end()) { /* Local copy */ ldpp_dout(dpp, 20) << "D4NFilterObject::iterate:: " << __func__ << "(): Block with oid=" << oid_in_cache << " found in local cache." << dendl; if (block.version == version) { @@ -1027,11 +1023,11 @@ int D4NFilterObject::D4NFilterReadOp::iterate(const DoutPrefixProvider* dpp, int if ((part_len != obj_max_req_size) && source->driver->get_policy_driver()->get_cache_policy()->exist_key(oid_in_cache) > 0) { // Read From Cache - if (block.dirty == true){ + if (block.cacheObj.dirty == true){ key = "D_" + oid_in_cache; //we keep track of dirty data in the cache for the metadata failure case } - ldpp_dout(dpp, 20) << "D4NFilterObject::iterate:: " << __func__ << "(): " << __LINE__ << ": READ FROM CACHE: block dirty =" << block.dirty << dendl; + ldpp_dout(dpp, 20) << "D4NFilterObject::iterate:: " << __func__ << "(): " << __LINE__ << ": READ FROM CACHE: block dirty =" << block.cacheObj.dirty << dendl; ldpp_dout(dpp, 20) << "D4NFilterObject::iterate:: " << __func__ << "(): " << __LINE__ << ": READ FROM CACHE: key=" << key << dendl; auto completed = source->driver->get_cache_driver()->get_async(dpp, y, aio.get(), key, read_ofs, len_to_read, cost, id); @@ -1052,7 +1048,7 @@ int D4NFilterObject::D4NFilterReadOp::iterate(const DoutPrefixProvider* dpp, int if ((r = source->driver->get_block_dir()->remove_host(dpp, &block, dpp->get_cct()->_conf->rgw_d4n_l1_datacache_address, y)) < 0) ldpp_dout(dpp, 0) << "D4NFilterObject::iterate:: " << __func__ << "(): Error: failed to remove incorrect host from block with oid=" << oid_in_cache << ", ret=" << r << dendl; - if ((block.hostsList.size() - 1) > 0 && r == 0) { /* Remote copy */ + if ((block.cacheObj.hostsList.size() - 1) > 0 && r == 0) { /* Remote copy */ ldpp_dout(dpp, 20) << "D4NFilterObject::iterate:: " << __func__ << "(): Block with oid=" << oid_in_cache << " found in remote cache." << dendl; // TODO: Retrieve remotely // Policy decision: should we cache remote blocks locally? @@ -1069,8 +1065,8 @@ int D4NFilterObject::D4NFilterReadOp::iterate(const DoutPrefixProvider* dpp, int break; } } - // if (it != block.hostsList.end()) - } else if (block.hostsList.size()) { /* Remote copy */ + // if (it != block.cacheObj.hostsList.end()) + } else if (block.cacheObj.hostsList.size()) { /* Remote copy */ ldpp_dout(dpp, 20) << "D4NFilterObject::iterate:: " << __func__ << "(): Block with oid=" << oid_in_cache << " found in remote cache." << dendl; // TODO: Retrieve remotely // Policy decision: should we cache remote blocks locally? @@ -1217,17 +1213,16 @@ int D4NFilterObject::D4NFilterReadOp::D4NFilterGetCB::handle_data(bufferlist& bl block.cacheObj.bucketName = source->get_bucket()->get_name(); std::stringstream s; block.cacheObj.creationTime = std::to_string(ceph::real_clock::to_time_t(source->get_mtime())); - block.cacheObj.dirty = false; - bool dirty = block.dirty = false; //Reading from the backend, data is clean + bool dirty = block.cacheObj.dirty = false; //Reading from the backend, data is clean block.version = version; if (source->dest_object && source->dest_bucket) { dest_prefix = source->dest_bucket->get_name() + "_" + source->dest_version + "_" + source->dest_object->get_name(); - dest_block.hostsList.insert(dpp->get_cct()->_conf->rgw_d4n_l1_datacache_address); + dest_block.cacheObj.hostsList.insert(dpp->get_cct()->_conf->rgw_d4n_l1_datacache_address); dest_block.cacheObj.objName = source->dest_object->get_key().get_oid(); dest_block.cacheObj.bucketName = source->dest_object->get_bucket()->get_name(); //dest_block.cacheObj.creationTime = std::to_string(ceph::real_clock::to_time_t(source->get_mtime())); - dest_block.cacheObj.dirty = dest_block.dirty = false; + dest_block.cacheObj.dirty = false; dest_block.version = source->dest_version; } @@ -1260,7 +1255,7 @@ int D4NFilterObject::D4NFilterReadOp::D4NFilterGetCB::handle_data(bufferlist& bl block.version = version; } - block.hostsList.insert(dpp->get_cct()->_conf->rgw_d4n_l1_datacache_address); + block.cacheObj.hostsList.insert(dpp->get_cct()->_conf->rgw_d4n_l1_datacache_address); if ((ret = blockDir->set(dpp, &block, *y)) < 0) ldpp_dout(dpp, 0) << "D4NFilterObject::D4NFilterReadOp::D4NFilterGetCB::" << __func__ << "(): BlockDirectory set() method failed, ret=" << ret << dendl; @@ -1320,7 +1315,7 @@ int D4NFilterObject::D4NFilterReadOp::D4NFilterGetCB::handle_data(bufferlist& bl block.version = version; } - block.hostsList.insert(dpp->get_cct()->_conf->rgw_d4n_l1_datacache_address); + block.cacheObj.hostsList.insert(dpp->get_cct()->_conf->rgw_d4n_l1_datacache_address); if ((ret = blockDir->set(dpp, &block, *y)) < 0) ldpp_dout(dpp, 0) << "D4NFilterObject::D4NFilterReadOp::D4NFilterGetCB::" << __func__ << "(): BlockDirectory set() method failed, ret=" << ret << dendl; @@ -1378,7 +1373,7 @@ int D4NFilterObject::D4NFilterReadOp::D4NFilterGetCB::handle_data(bufferlist& bl block.version = version; } - block.hostsList.insert(dpp->get_cct()->_conf->rgw_d4n_l1_datacache_address); + block.cacheObj.hostsList.insert(dpp->get_cct()->_conf->rgw_d4n_l1_datacache_address); if ((ret = blockDir->set(dpp, &block, *y)) < 0) ldpp_dout(dpp, 0) << "D4NFilterObject::D4NFilterReadOp::D4NFilterGetCB::" << __func__ << "(): BlockDirectory set() method failed, ret=" << ret << dendl; @@ -1471,8 +1466,8 @@ int D4NFilterWriter::prepare(optional_yield y) ldpp_dout(dpp, 0) << "D4NFilterWriter::" << __func__ << "(): CacheDriver delete_data() method failed, ret=" << ret << dendl; d4n_writecache = g_conf()->d4n_writecache_enabled; - if (!d4n_writecache){ - ldpp_dout(dpp, 10) << "D4NFilterObject::D4NFilterWriteOp::" << __func__ << "(): calling next iterate" << dendl; + if (!d4n_writecache) { + ldpp_dout(dpp, 10) << "D4NFilterWriter::" << __func__ << "(): calling next process" << dendl; return next->prepare(y); } @@ -1485,10 +1480,10 @@ int D4NFilterWriter::prepare(optional_yield y) char buf[OBJ_INSTANCE_LEN + 1]; gen_rand_alphanumeric_no_underscore(dpp->get_cct(), buf, OBJ_INSTANCE_LEN); this->version = buf; // using gen_rand_alphanumeric_no_underscore for the time being - ldpp_dout(dpp, 20) << "D4NFilterObject::D4NFilterWriteOp::" << __func__ << "(): generating version: " << version << dendl; + ldpp_dout(dpp, 20) << "D4NFilterWriter::" << __func__ << "(): generating version: " << version << dendl; } } else { - ldpp_dout(dpp, 20) << "D4NFilterWriter::" << __func__ << "(): " << "version is: " << object->get_instance() << dendl; + ldpp_dout(dpp, 20) << "D4NFilterWriter::" << __func__ << "(): version is: " << object->get_instance() << dendl; } return 0; @@ -1511,8 +1506,7 @@ int D4NFilterWriter::process(bufferlist&& data, uint64_t offset) } prefix = obj->get_bucket()->get_name() + "_" + version + "_" + obj->get_name(); rgw::d4n::BlockDirectory* blockDir = driver->get_block_dir(); - - block.hostsList.insert(dpp->get_cct()->_conf->rgw_d4n_l1_datacache_address); + block.cacheObj.bucketName = obj->get_bucket()->get_name(); block.cacheObj.objName = obj->get_key().get_oid(); block.cacheObj.dirty = dirty; @@ -1523,25 +1517,25 @@ int D4NFilterWriter::process(bufferlist&& data, uint64_t offset) int ret = 0; if (!d4n_writecache) { - ldpp_dout(dpp, 10) << "D4NFilterObject::D4NFilterWriteOp::" << __func__ << "(): calling next process" << dendl; - ret = next->process(std::move(data), offset); + ldpp_dout(dpp, 10) << "D4NFilterWriter::" << __func__ << "(): calling next process" << dendl; + return next->process(std::move(data), offset); } else { std::string oid = prefix + "_" + std::to_string(ofs); std::string key = "D_" + oid + "_" + std::to_string(bl_len); std::string oid_in_cache = oid + "_" + std::to_string(bl_len); block.size = bl.length(); block.blockID = ofs; - block.dirty = true; - block.hostsList.insert(dpp->get_cct()->_conf->rgw_d4n_l1_datacache_address); + block.cacheObj.dirty = true; + block.cacheObj.hostsList.insert(dpp->get_cct()->_conf->rgw_d4n_l1_datacache_address); block.version = version; dirty = true; ret = driver->get_policy_driver()->get_cache_policy()->eviction(dpp, block.size, y); if (ret == 0) { if (bl.length() > 0) { - ldpp_dout(dpp, 10) << "D4NFilterObject::D4NFilterWriteOp::" << __func__ << "(): key is: " << key << dendl; + ldpp_dout(dpp, 10) << "D4NFilterWriter::" << __func__ << "(): key is: " << key << dendl; ret = driver->get_cache_driver()->put(dpp, key, bl, bl.length(), obj->get_attrs(), y); if (ret == 0) { - ldpp_dout(dpp, 10) << "D4NFilterObject::D4NFilterWriteOp::" << __func__ << "(): oid_in_cache is: " << oid_in_cache << dendl; + ldpp_dout(dpp, 10) << "D4NFilterWriter::" << __func__ << "(): oid_in_cache is: " << oid_in_cache << dendl; driver->get_policy_driver()->get_cache_policy()->update(dpp, oid_in_cache, ofs, bl.length(), version, dirty, y); /* Store block in directory */ @@ -1554,15 +1548,15 @@ int D4NFilterWriter::process(bufferlist&& data, uint64_t offset) block.version = version; } - block.hostsList.insert(dpp->get_cct()->_conf->rgw_d4n_l1_datacache_address); + block.cacheObj.hostsList.insert(dpp->get_cct()->_conf->rgw_d4n_l1_datacache_address); if ((ret = blockDir->set(dpp, &block, y)) < 0) - ldpp_dout(dpp, 0) << "D4NFilterObject::D4NFilterReadOp::D4NFilterGetCB::" << __func__ << "(): BlockDirectory set() method failed, ret=" << ret << dendl; + ldpp_dout(dpp, 0) << "D4NFilterWriter::" << __func__ << "(): BlockDirectory set() method failed, ret=" << ret << dendl; } else { ldpp_dout(dpp, 0) << "Failed to fetch existing block for: " << existing_block.cacheObj.objName << " blockID: " << existing_block.blockID << " block size: " << existing_block.size << ", ret=" << ret << dendl; } } else { - ldpp_dout(dpp, 0) << "D4NFilterObject::D4NFilterWriteOp::process" << __func__ << "(): ERROR: writting data to the cache failed, ret=" << ret << dendl; + ldpp_dout(dpp, 0) << "D4NFilterWriter::" << __func__ << "(): ERROR: writting data to the cache failed, ret=" << ret << dendl; return ret; } } @@ -1619,7 +1613,7 @@ int D4NFilterWriter::complete(size_t accounted_size, const std::string& etag, object->set_object_version(version); if (ret == 0) { object->set_attrs(attrs); - ldpp_dout(dpp, 20) << "D4NFilterWriter::" << __func__ << " version stored in update method is: " << version << dendl; + ldpp_dout(dpp, 20) << "D4NFilterWriter::" << __func__ << "(): version stored in update method is: " << version << dendl; driver->get_policy_driver()->get_cache_policy()->update(dpp, key, 0, bl.length(), version, dirty, y); ret = object->set_head_obj_dir_entry(dpp, y, true, true); if (ret < 0) { @@ -1644,8 +1638,8 @@ int D4NFilterWriter::complete(size_t accounted_size, const std::string& etag, } } else { //if get_cache_driver()->put() write_to_backend_store = true; - ldpp_dout(dpp, 0) << "D4NFilterWriter::" << __func__ << " put failed for head_oid_in_cache, ret=" << ret << dendl; - ldpp_dout(dpp, 0) << "D4NFilterWriter::" << __func__ << " calling complete of backend store: " << dendl; + ldpp_dout(dpp, 0) << "D4NFilterWriter::" << __func__ << "(): put failed for head_oid_in_cache, ret=" << ret << dendl; + ldpp_dout(dpp, 0) << "D4NFilterWriter::" << __func__ << "(): calling complete of backend store: " << dendl; } } else { // if d4n_writecache = true write_to_backend_store = true; @@ -1656,7 +1650,7 @@ int D4NFilterWriter::complete(size_t accounted_size, const std::string& etag, delete_at, if_match, if_nomatch, user_data, zones_trace, canceled, rctx, flags); if (ret < 0) { - ldpp_dout(dpp, 0) << "D4NFilterWriter::" << __func__ << "(): Writing to backend store failed, ret=" << ret << dendl; + ldpp_dout(dpp, 0) << "D4NFilterWriter::" << __func__ << "(): writing to backend store failed, ret=" << ret << dendl; } } diff --git a/src/test/rgw/test_d4n_directory.cc b/src/test/rgw/test_d4n_directory.cc index bb6f0b4cc850..bd7a80118a8a 100644 --- a/src/test/rgw/test_d4n_directory.cc +++ b/src/test/rgw/test_d4n_directory.cc @@ -83,7 +83,7 @@ class ObjectDirectoryFixture: public ::testing::Test { std::shared_ptr conn; std::vector vals{"testName", "testBucket", "", "0", env->redisHost}; - std::vector fields{"objName", "bucketName", "creationTime", "dirty", "objHosts"}; + std::vector fields{"objName", "bucketName", "creationTime", "dirty", "hosts"}; }; class BlockDirectoryFixture: public ::testing::Test { @@ -101,8 +101,7 @@ class BlockDirectoryFixture: public ::testing::Test { }, .blockID = 0, .version = "", - .size = 0, - .hostsList = { env->redisHost } + .size = 0 }; ASSERT_NE(block, nullptr); @@ -128,10 +127,10 @@ class BlockDirectoryFixture: public ::testing::Test { net::io_context io; std::shared_ptr conn; - std::vector vals{"0", "", "0", "0", "0", env->redisHost, + std::vector vals{"0", "", "0", "0", "testName", "testBucket", "", "0", env->redisHost}; - std::vector fields{"blockID", "version", "dirtyBlock", "size", "globalWeight", "blockHosts", - "objName", "bucketName", "creationTime", "dirtyObj", "objHosts"}; + std::vector fields{"blockID", "version", "size", "globalWeight", + "objName", "bucketName", "creationTime", "dirty", "hosts"}; }; void rethrow(std::exception_ptr eptr) { @@ -271,12 +270,14 @@ TEST_F(ObjectDirectoryFixture, UpdateFieldYield) { boost::asio::spawn(io, [this] (boost::asio::yield_context yield) { ASSERT_EQ(0, dir->set(env->dpp, obj, yield)); - ASSERT_EQ(0, dir->update_field(env->dpp, obj, "objName", "newTestName", yield)); - ASSERT_EQ(0, dir->update_field(env->dpp, obj, "objHosts", "127.0.0.1:5000", yield)); + std::string oid = "newTestName"; + std::string host = "127.0.0.1:5000"; + ASSERT_EQ(0, dir->update_field(env->dpp, obj, "objName", oid, yield)); + ASSERT_EQ(0, dir->update_field(env->dpp, obj, "hosts", host, yield)); boost::system::error_code ec; request req; - req.push("HMGET", "testBucket_testName", "objName", "objHosts"); + req.push("HMGET", "testBucket_testName", "objName", "hosts"); req.push("FLUSHALL"); response< std::vector, boost::redis::ignore_t> resp; @@ -284,7 +285,7 @@ TEST_F(ObjectDirectoryFixture, UpdateFieldYield) conn->async_exec(req, resp, yield[ec]); ASSERT_EQ((bool)ec, false); - EXPECT_EQ(std::get<0>(resp).value()[0], "newTestName"); + EXPECT_EQ(std::get<0>(resp).value()[0], oid); EXPECT_EQ(std::get<0>(resp).value()[1], "127.0.0.1:6379_127.0.0.1:5000"); conn->cancel(); @@ -374,8 +375,8 @@ TEST_F(BlockDirectoryFixture, CopyYield) EXPECT_EQ(std::get<0>(resp).value(), 1); auto copyVals = vals; - copyVals[6] = "copyTestName"; - copyVals[7] = "copyBucketName"; + copyVals[4] = "copyTestName"; + copyVals[5] = "copyBucketName"; EXPECT_EQ(std::get<1>(resp).value(), copyVals); conn->cancel(); @@ -427,12 +428,14 @@ TEST_F(BlockDirectoryFixture, UpdateFieldYield) { boost::asio::spawn(io, [this] (boost::asio::yield_context yield) { ASSERT_EQ(0, dir->set(env->dpp, block, optional_yield{yield})); - ASSERT_EQ(0, dir->update_field(env->dpp, block, "objName", "newTestName", optional_yield{yield})); - ASSERT_EQ(0, dir->update_field(env->dpp, block, "blockHosts", "127.0.0.1:5000", optional_yield{yield})); + std::string oid = "newTestName"; + std::string host = "127.0.0.1:5000"; + ASSERT_EQ(0, dir->update_field(env->dpp, block, "objName", oid, optional_yield{yield})); + ASSERT_EQ(0, dir->update_field(env->dpp, block, "hosts", host, optional_yield{yield})); boost::system::error_code ec; request req; - req.push("HMGET", "testBucket_testName_0_0", "objName", "blockHosts"); + req.push("HMGET", "testBucket_testName_0_0", "objName", "hosts"); req.push("FLUSHALL"); response< std::vector, boost::redis::ignore_t> resp; @@ -440,7 +443,7 @@ TEST_F(BlockDirectoryFixture, UpdateFieldYield) conn->async_exec(req, resp, yield[ec]); ASSERT_EQ((bool)ec, false); - EXPECT_EQ(std::get<0>(resp).value()[0], "newTestName"); + EXPECT_EQ(std::get<0>(resp).value()[0], oid); EXPECT_EQ(std::get<0>(resp).value()[1], "127.0.0.1:6379_127.0.0.1:5000"); conn->cancel(); @@ -452,15 +455,18 @@ TEST_F(BlockDirectoryFixture, UpdateFieldYield) TEST_F(BlockDirectoryFixture, RemoveHostYield) { boost::asio::spawn(io, [this] (boost::asio::yield_context yield) { - block->hostsList.insert("127.0.0.1:6000"); + block->cacheObj.hostsList.insert("127.0.0.1:6000"); ASSERT_EQ(0, dir->set(env->dpp, block, optional_yield{yield})); - ASSERT_EQ(0, dir->remove_host(env->dpp, block, "127.0.0.1:6379", optional_yield{yield})); + { + std::string host = "127.0.0.1:6379"; + ASSERT_EQ(0, dir->remove_host(env->dpp, block, host, optional_yield{yield})); + } { boost::system::error_code ec; request req; - req.push("HEXISTS", "testBucket_testName_0_0", "blockHosts"); - req.push("HGET", "testBucket_testName_0_0", "blockHosts"); + req.push("HEXISTS", "testBucket_testName_0_0", "hosts"); + req.push("HGET", "testBucket_testName_0_0", "hosts"); response resp; conn->async_exec(req, resp, yield[ec]); @@ -470,7 +476,10 @@ TEST_F(BlockDirectoryFixture, RemoveHostYield) EXPECT_EQ(std::get<1>(resp).value(), "127.0.0.1:6000"); } - ASSERT_EQ(0, dir->remove_host(env->dpp, block, "127.0.0.1:6000", optional_yield{yield})); + { + std::string host = "127.0.0.1:6000"; + ASSERT_EQ(0, dir->remove_host(env->dpp, block, host, optional_yield{yield})); + } { boost::system::error_code ec; diff --git a/src/test/rgw/test_d4n_policy.cc b/src/test/rgw/test_d4n_policy.cc index ea233227cbe3..0268f482fa1e 100644 --- a/src/test/rgw/test_d4n_policy.cc +++ b/src/test/rgw/test_d4n_policy.cc @@ -58,7 +58,7 @@ class LFUDAPolicyFixture : public ::testing::Test { .blockID = 0, .version = "", .size = bl.length(), - .hostsList = { env->redisHost } + .globalWeight = 0 }; conn = std::make_shared(net::make_strand(io)); @@ -117,7 +117,7 @@ class LFUDAPolicyFixture : public ::testing::Test { if (dir->get(env->dpp, block, y) < 0) { return -1; } else { - if (!block->hostsList.empty()) { + if (!block->cacheObj.hostsList.empty()) { block->globalWeight += age; auto globalWeight = std::to_string(block->globalWeight); if (dir->update_field(env->dpp, block, "globalWeight", globalWeight, y) < 0) { @@ -130,7 +130,7 @@ class LFUDAPolicyFixture : public ::testing::Test { } } } else if (!exists) { /* No remote copy */ - block->hostsList.insert(env->dpp->get_cct()->_conf->rgw_d4n_l1_datacache_address); + block->cacheObj.hostsList.insert(env->dpp->get_cct()->_conf->rgw_d4n_l1_datacache_address); if (dir->set(env->dpp, block, y) < 0) return -1; @@ -212,7 +212,6 @@ TEST_F(LFUDAPolicyFixture, RemoteGetBlockYield) .version = "", .size = bl.length(), .globalWeight = 5, - .hostsList = { env->redisHost } }; bufferlist attrVal; @@ -227,15 +226,15 @@ TEST_F(LFUDAPolicyFixture, RemoteGetBlockYield) policyDriver->get_cache_policy()->init(env->cct, env->dpp, io, driver); ASSERT_EQ(0, dir->set(env->dpp, &victim, optional_yield{yield})); - std::string victimKey = victim.cacheObj.bucketName + "_" + victim.cacheObj.objName + "_" + std::to_string(victim.blockID) + "_" + std::to_string(victim.size); + std::string victimKey = victim.cacheObj.bucketName + "_version_" + victim.cacheObj.objName + "_" + std::to_string(victim.blockID) + "_" + std::to_string(victim.size); ASSERT_EQ(0, cacheDriver->put(env->dpp, victimKey, bl, bl.length(), attrs, optional_yield{yield})); policyDriver->get_cache_policy()->update(env->dpp, victimKey, 0, bl.length(), "", false, optional_yield{yield}); /* Remote block */ block->size = cacheDriver->get_free_space(env->dpp) + 1; /* To trigger eviction */ - block->hostsList.clear(); + block->cacheObj.hostsList.clear(); block->cacheObj.hostsList.clear(); - block->hostsList.insert("127.0.0.1:6000"); + block->cacheObj.hostsList.insert("127.0.0.1:6000"); block->cacheObj.hostsList.insert("127.0.0.1:6000"); ASSERT_EQ(0, dir->set(env->dpp, block, optional_yield{yield}));