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 <prsrivas@redhat.com>

diff --git a/src/rgw/driver/d4n/rgw_sal_d4n.cc b/src/rgw/driver/d4n/rgw_sal_d4n.cc
index db419142deb2707bc470fa4034bfed87afd7b110..c1d3701baf5975dec41deaab5aa5df21c441b300 100644 (file)
--- 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);