From: Pritha Srivastava Date: Mon, 19 May 2025 11:27:15 +0000 (+0530) Subject: rgw/d4n: reading from local cache first in iterate() X-Git-Tag: testing/wip-vshankar-testing-20250829.190601-debug~16^2~8 X-Git-Url: http://git.apps.os.sepia.ceph.com/?a=commitdiff_plain;h=ad9d92f45eda60a5ddf6cc8cb8e79a76a154ff64;p=ceph-ci.git rgw/d4n: reading from local cache first in iterate() method instead of checking directory first for existence of cache blocks. Signed-off-by: Pritha Srivastava --- diff --git a/src/rgw/driver/d4n/rgw_sal_d4n.cc b/src/rgw/driver/d4n/rgw_sal_d4n.cc index db419142deb..c1d3701baf5 100644 --- a/src/rgw/driver/d4n/rgw_sal_d4n.cc +++ b/src/rgw/driver/d4n/rgw_sal_d4n.cc @@ -1926,11 +1926,6 @@ int D4NFilterObject::D4NFilterReadOp::iterate(const DoutPrefixProvider* dpp, int this->cb->set_client_cb(cb, dpp, &y); source->set_prefix(prefix); - /* This algorithm stores chunks for ranged requests also in the cache, which might be smaller than max_chunk_size - One simplification could be to overwrite the smaller chunks with a bigger chunk of max_chunk_size, and to serve requests for smaller - chunks using the larger chunk, but all corner cases need to be considered like the last chunk which might be smaller than max_chunk_size - and also ranged requests where a smaller chunk is overwritten by a larger chunk size != max_chunk_size */ - uint64_t max_chunk_size = std::min(g_conf()->rgw_max_chunk_size, source->get_size()); uint64_t start_part_num = 0; uint64_t part_num = ofs/max_chunk_size; //part num of ofs wrt start of the object @@ -1988,48 +1983,47 @@ int D4NFilterObject::D4NFilterReadOp::iterate(const DoutPrefixProvider* dpp, int " read_ofs: " << read_ofs << " part len: " << part_len << dendl; int ret; - if ((ret = source->driver->get_block_dir()->get(dpp, &block, y)) == 0) { - auto it = block.cacheObj.hostsList.find(dpp->get_cct()->_conf->rgw_d4n_l1_datacache_address); - - if (it != block.cacheObj.hostsList.end()) { /* Local copy */ - ldpp_dout(dpp, 20) << "D4NFilterObject::iterate:: " << __func__ << "(): Block found in directory. " << oid_in_cache << dendl; - // we keep track of dirty data in the cache for the metadata failure case - ldpp_dout(dpp, 20) << "D4NFilterObject::iterate:: " << __func__ << "(): READ FROM CACHE: block is dirty = " << block.cacheObj.dirty << dendl; - ldpp_dout(dpp, 20) << "D4NFilterObject::iterate:: " << __func__ << "(): " << __LINE__ << ": READ FROM CACHE: oid_in_cache=" << oid_in_cache << dendl; - - if (block.version == version) { - if (source->driver->get_policy_driver()->get_cache_policy()->update_refcount_if_key_exists(dpp, oid_in_cache, rgw::d4n::RefCount::INCR, y) > 0) { - // Read From Cache - auto completed = source->driver->get_cache_driver()->get_async(dpp, y, aio.get(), oid_in_cache, read_ofs, len_to_read, cost, id); - this->blocks_info.insert(std::make_pair(id, std::make_pair(adjusted_start_ofs, part_len))); - ldpp_dout(dpp, 20) << "D4NFilterObject::iterate:: " << __func__ << "(): Info: flushing data for oid: " << oid_in_cache << dendl; - auto r = flush(dpp, std::move(completed), y); - if (r < 0) { - drain(dpp, y); - ldpp_dout(dpp, 0) << "D4NFilterObject::iterate:: " << __func__ << "(): Error: failed to flush, ret=" << r << dendl; - return r; - } - } else { // end - if update_refcount_if_key_exists - int r = -1; - 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, 10) << "D4NFilterObject::iterate:: " << __func__ << "(): Error: failed to remove incorrect host from block with oid=" << oid_in_cache <<", ret=" << r << dendl; - - 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? - } else { - ldpp_dout(dpp, 20) << "D4NFilterObject::iterate:: " << __func__ << "(): Info: draining data for oid: " << oid_in_cache << dendl; - auto r = drain(dpp, y); - if (r < 0) { - ldpp_dout(dpp, 0) << "D4NFilterObject::iterate:: " << __func__ << "(): Error: failed to drain, ret=" << r << dendl; - return r; - } - break; - } - } //end - else - } else { // if (block.version == version) - // TODO: If data has already been returned for any older versioned block, then return ‘retry’ error, else + auto policy = source->driver->get_policy_driver()->get_cache_policy(); + auto cache_driver = source->driver->get_cache_driver(); + auto block_dir = source->driver->get_block_dir(); + if (policy->update_refcount_if_key_exists(dpp, oid_in_cache, rgw::d4n::RefCount::INCR, y) > 0) { + ldpp_dout(dpp, 20) << "D4NFilterObject::iterate:: " << __func__ << "(): " << __LINE__ << ": READ FROM CACHE: oid_in_cache=" << oid_in_cache << dendl; + // Read From Cache + auto completed = cache_driver->get_async(dpp, y, aio.get(), oid_in_cache, read_ofs, len_to_read, cost, id); + this->blocks_info.insert(std::make_pair(id, std::make_pair(adjusted_start_ofs, part_len))); + ldpp_dout(dpp, 20) << "D4NFilterObject::iterate:: " << __func__ << "(): Info: flushing data for oid: " << oid_in_cache << dendl; + auto r = flush(dpp, std::move(completed), y); + if (r < 0) { + drain(dpp, y); + ldpp_dout(dpp, 0) << "D4NFilterObject::iterate:: " << __func__ << "(): Error: failed to flush, ret=" << r << dendl; + return r; + } + } else { // else - if update_refcount_if_key_exists + int r = -1; + if ((ret = block_dir->get(dpp, &block, y)) == 0) { + if (block.version != version) { + // TODO: If data has already been returned for any older versioned block, then return ‘retry’ error + ldpp_dout(dpp, 20) << "D4NFilterObject::iterate:: " << __func__ << "(): Info: Version mismatch, draining data for oid: " << oid_in_cache << dendl; + auto r = drain(dpp, y); + if (r < 0) { + ldpp_dout(dpp, 0) << "D4NFilterObject::iterate:: " << __func__ << "(): Error: failed to drain, ret=" << r << dendl; + return r; + } + break; + } //end if block.version != version + auto it = block.cacheObj.hostsList.find(dpp->get_cct()->_conf->rgw_d4n_l1_datacache_address); + auto hostsListSize = block.cacheObj.hostsList.size(); + if (it != block.cacheObj.hostsList.end()) { + if ((r = block_dir->remove_host(dpp, &block, dpp->get_cct()->_conf->rgw_d4n_l1_datacache_address, y)) < 0) { + ldpp_dout(dpp, 10) << "D4NFilterObject::iterate:: " << __func__ << "(): Error: failed to remove incorrect host from block with oid=" << oid_in_cache <<", ret=" << r << dendl; + hostsListSize = hostsListSize - 1; + } + } + if (hostsListSize > 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? + } else { ldpp_dout(dpp, 20) << "D4NFilterObject::iterate:: " << __func__ << "(): Info: draining data for oid: " << oid_in_cache << dendl; auto r = drain(dpp, y); if (r < 0) { @@ -2038,23 +2032,17 @@ int D4NFilterObject::D4NFilterReadOp::iterate(const DoutPrefixProvider* dpp, int } break; } - } 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? - } - } else { // else if (ret == -ENOENT) - if (ret < 0) - ldpp_dout(dpp, 0) << "Failed to fetch existing block for: " << block.cacheObj.objName << " blockID: " << block.blockID << " block size: " << block.size << ", ret=" << ret << dendl; - ldpp_dout(dpp, 20) << "D4NFilterObject::iterate:: " << __func__ << "(): Info: draining data for oid: " << oid_in_cache << dendl; - auto r = drain(dpp, y); - if (r < 0) { - ldpp_dout(dpp, 10) << "D4NFilterObject::iterate:: " << __func__ << "(): Error: failed to drain, ret=" << r << dendl; - return r; + } else { // else - if source->driver->get_block_dir()->get + ldpp_dout(dpp, 5) << "Failed to fetch block for: " << block.cacheObj.objName << " blockID: " << block.blockID << " block size: " << block.size << ", ret=" << ret << dendl; + ldpp_dout(dpp, 20) << "D4NFilterObject::iterate:: " << __func__ << "(): Info: draining data for oid: " << oid_in_cache << dendl; + auto r = drain(dpp, y); + if (r < 0) { + ldpp_dout(dpp, 10) << "D4NFilterObject::iterate:: " << __func__ << "(): Error: failed to drain, ret=" << r << dendl; + return r; + } + break; } - break; } //end - else - if (start_part_num == (num_parts - 1)) { ldpp_dout(dpp, 20) << "D4NFilterObject::iterate:: " << __func__ << "(): Info: draining data for oid: " << oid_in_cache << dendl; return drain(dpp, y);