From: Pritha Srivastava Date: Thu, 22 May 2025 11:11:55 +0000 (+0530) Subject: rgw/d4n: modified update method to optionally take a bool X-Git-Url: http://git.apps.os.sepia.ceph.com/?a=commitdiff_plain;h=d1bd77e4c5e42c5270c6eb249c33e5753a90b2ec;p=ceph.git rgw/d4n: modified update method to optionally take a bool for dirty flag, such that when a flag is not set, then the method re-uses the old value of dirty flag using in-memory data structure. This is helpful in eliminating a call to directory to fetch the 'dirty' flag in the flush() method. Signed-off-by: Pritha Srivastava --- diff --git a/src/rgw/driver/d4n/d4n_policy.cc b/src/rgw/driver/d4n/d4n_policy.cc index 145acdc5efe77..3fc73f27579a7 100644 --- a/src/rgw/driver/d4n/d4n_policy.cc +++ b/src/rgw/driver/d4n/d4n_policy.cc @@ -428,7 +428,7 @@ bool LFUDAPolicy::update_refcount_if_key_exists(const DoutPrefixProvider* dpp, c return true; } -void LFUDAPolicy::update(const DoutPrefixProvider* dpp, const std::string& key, uint64_t offset, uint64_t len, const std::string& version, bool dirty, uint8_t op, optional_yield y, std::string& restore_val) +void LFUDAPolicy::update(const DoutPrefixProvider* dpp, const std::string& key, uint64_t offset, uint64_t len, const std::string& version, std::optional dirty, uint8_t op, optional_yield y, std::string& restore_val) { ldpp_dout(dpp, 10) << "LFUDAPolicy::" << __func__ << "(): updating entry: " << key << dendl; using handle_type = boost::heap::fibonacci_heap>>::handle_type; @@ -447,12 +447,16 @@ void LFUDAPolicy::update(const DoutPrefixProvider* dpp, const std::string& key, /* check the dirty flag in the existing entry for the key and the incoming dirty flag. If the incoming dirty flag is false, that means update() is invoked as part of cleaning process, so we must not update its localWeight. */ - if (entry != nullptr) { + if (entry) { refcount = entry->refcount; - if ((entry->dirty && !dirty)) { - localWeight = entry->localWeight; - updateLocalWeight = false; - } else { + if (entry->dirty && dirty.has_value()) { + bool is_dirty = dirty.value(); + if (!is_dirty) { + localWeight = entry->localWeight; + updateLocalWeight = false; + } + } + if (updateLocalWeight) { localWeight = entry->localWeight + age; } if (op == RefCount::INCR) { @@ -463,10 +467,17 @@ void LFUDAPolicy::update(const DoutPrefixProvider* dpp, const std::string& key, refcount -= 1; } } - } + } + //pick the existing value of dirty, if no value has been passed in + bool is_dirty = false; + if (dirty.has_value()) { + is_dirty = dirty.value(); + } else if (entry) { + is_dirty = entry->dirty; + } _erase(dpp, key, y); ldpp_dout(dpp, 10) << "LFUDAPolicy::" << __func__ << "(): updated refcount is: " << refcount << dendl; - LFUDAEntry* e = new LFUDAEntry(key, offset, len, version, dirty, refcount, localWeight); + LFUDAEntry* e = new LFUDAEntry(key, offset, len, version, is_dirty, refcount, localWeight); handle_type handle = entries_heap.push(e); e->set_handle(handle); entries_map.emplace(key, e); @@ -998,11 +1009,15 @@ int LRUPolicy::eviction(const DoutPrefixProvider* dpp, uint64_t size, optional_y return 0; } -void LRUPolicy::update(const DoutPrefixProvider* dpp, const std::string& key, uint64_t offset, uint64_t len, const std::string& version, bool dirty, uint8_t op, optional_yield y, std::string& restore_val) +void LRUPolicy::update(const DoutPrefixProvider* dpp, const std::string& key, uint64_t offset, uint64_t len, const std::string& version, std::optional dirty, uint8_t op, optional_yield y, std::string& restore_val) { const std::lock_guard l(lru_lock); _erase(dpp, key, y); - Entry* e = new Entry(key, offset, len, version, dirty, 0); + bool is_dirty = false; + if (dirty.has_value()) { + is_dirty = dirty.value(); + } + Entry* e = new Entry(key, offset, len, version, is_dirty, 0); entries_lru_list.push_back(*e); entries_map.emplace(key, e); } diff --git a/src/rgw/driver/d4n/d4n_policy.h b/src/rgw/driver/d4n/d4n_policy.h index a2b0d6508cb92..0a6a9de856e91 100644 --- a/src/rgw/driver/d4n/d4n_policy.h +++ b/src/rgw/driver/d4n/d4n_policy.h @@ -76,7 +76,7 @@ class CachePolicy { virtual int exist_key(const std::string& key) = 0; virtual int eviction(const DoutPrefixProvider* dpp, uint64_t size, optional_yield y) = 0; virtual bool update_refcount_if_key_exists(const DoutPrefixProvider* dpp, const std::string& key, uint8_t op, optional_yield y) = 0; - virtual void update(const DoutPrefixProvider* dpp, const std::string& key, uint64_t offset, uint64_t len, const std::string& version, bool dirty, uint8_t op, optional_yield y, std::string& restore_val=empty) = 0; + virtual void update(const DoutPrefixProvider* dpp, const std::string& key, uint64_t offset, uint64_t len, const std::string& version, std::optional dirty, uint8_t op, optional_yield y, std::string& restore_val=empty) = 0; virtual void update_dirty_object(const DoutPrefixProvider* dpp, const std::string& key, const std::string& version, bool deleteMarker, uint64_t size, double creationTime, const rgw_user& user, const std::string& etag, const std::string& bucket_name, const std::string& bucket_id, const rgw_obj_key& obj_key, uint8_t op, optional_yield y, std::string& restore_val=empty) = 0; @@ -205,7 +205,7 @@ class LFUDAPolicy : public CachePolicy { virtual int exist_key(const std::string& key) override; virtual int eviction(const DoutPrefixProvider* dpp, uint64_t size, optional_yield y) override; virtual bool update_refcount_if_key_exists(const DoutPrefixProvider* dpp, const std::string& key, uint8_t op, optional_yield y) override; - virtual void update(const DoutPrefixProvider* dpp, const std::string& key, uint64_t offset, uint64_t len, const std::string& version, bool dirty, uint8_t op, optional_yield y, std::string& restore_val=empty) override; + virtual void update(const DoutPrefixProvider* dpp, const std::string& key, uint64_t offset, uint64_t len, const std::string& version, std::optional dirty, uint8_t op, optional_yield y, std::string& restore_val=empty) override; virtual bool erase(const DoutPrefixProvider* dpp, const std::string& key, optional_yield y) override; virtual bool _erase(const DoutPrefixProvider* dpp, const std::string& key, optional_yield y); virtual void update_dirty_object(const DoutPrefixProvider* dpp, const std::string& key, const std::string& version, bool deleteMarker, uint64_t size, @@ -243,7 +243,7 @@ class LRUPolicy : public CachePolicy { virtual int exist_key(const std::string& key) override; virtual int eviction(const DoutPrefixProvider* dpp, uint64_t size, optional_yield y) override; virtual bool update_refcount_if_key_exists(const DoutPrefixProvider* dpp, const std::string& key, uint8_t op, optional_yield y) override { return false; } - virtual void update(const DoutPrefixProvider* dpp, const std::string& key, uint64_t offset, uint64_t len, const std::string& version, bool dirty, uint8_t op, optional_yield y, std::string& restore_val=empty) override; + virtual void update(const DoutPrefixProvider* dpp, const std::string& key, uint64_t offset, uint64_t len, const std::string& version, std::optional dirty, uint8_t op, optional_yield y, std::string& restore_val=empty) override; virtual void update_dirty_object(const DoutPrefixProvider* dpp, const std::string& key, const std::string& version, bool deleteMarker, uint64_t size, double creationTime, const rgw_user& user, const std::string& etag, const std::string& bucket_name, const std::string& bucket_id, const rgw_obj_key& obj_key, uint8_t op, optional_yield y, std::string& restore_val=empty) override; diff --git a/src/rgw/driver/d4n/rgw_sal_d4n.cc b/src/rgw/driver/d4n/rgw_sal_d4n.cc index 3995e5e6f19d5..bdbff1e418513 100644 --- a/src/rgw/driver/d4n/rgw_sal_d4n.cc +++ b/src/rgw/driver/d4n/rgw_sal_d4n.cc @@ -1849,25 +1849,12 @@ int D4NFilterObject::D4NFilterReadOp::flush(const DoutPrefixProvider* dpp, rgw:: std::pair ofs_len_pair = it->second; uint64_t ofs = ofs_len_pair.first; uint64_t len = ofs_len_pair.second; - bool dirty = false; - - rgw::d4n::CacheBlock block; - block.cacheObj.objName = source->get_key().get_oid(); - block.cacheObj.bucketName = source->get_bucket()->get_bucket_id(); - block.blockID = ofs; - block.size = len; std::string oid_in_cache = get_key_in_cache(prefix, std::to_string(ofs), std::to_string(len)); - if (source->driver->get_block_dir()->get(dpp, &block, y) == 0){ - if (block.cacheObj.dirty){ - dirty = true; - } - } - ldpp_dout(dpp, 20) << "D4NFilterObject::" << __func__ << " calling update for offset: " << offset << " adjusted offset: " << ofs << " length: " << len << " oid_in_cache: " << oid_in_cache << dendl; ldpp_dout(dpp, 20) << "D4NFilterObject::" << __func__ << " version stored in update method is: " << version << " " << source->get_object_version() << dendl; - source->driver->get_policy_driver()->get_cache_policy()->update(dpp, oid_in_cache, ofs, len, version, dirty, rgw::d4n::RefCount::DECR, y); + source->driver->get_policy_driver()->get_cache_policy()->update(dpp, oid_in_cache, ofs, len, version, std::nullopt, rgw::d4n::RefCount::DECR, y); blocks_info.erase(it); if (source->dest_object && source->dest_bucket) { D4NFilterObject* d4n_dest_object = dynamic_cast(source->dest_object);