From c38b8b2d96fc67b957f4d7f947f50bf4533e5bc2 Mon Sep 17 00:00:00 2001 From: Samarah Date: Thu, 9 Nov 2023 21:27:04 +0000 Subject: [PATCH] d4n: Update block `mtime` in filter and change `creationTime` to string in directory due to seg faults Signed-off-by: Samarah --- src/rgw/driver/d4n/d4n_directory.cc | 8 ++--- src/rgw/driver/d4n/d4n_directory.h | 2 +- src/rgw/driver/d4n/rgw_sal_d4n.cc | 51 ++++++++++++----------------- src/rgw/driver/d4n/rgw_sal_d4n.h | 1 + src/test/rgw/test_d4n_directory.cc | 8 ++--- src/test/rgw/test_d4n_policy.cc | 4 +-- 6 files changed, 33 insertions(+), 41 deletions(-) diff --git a/src/rgw/driver/d4n/d4n_directory.cc b/src/rgw/driver/d4n/d4n_directory.cc index b0971349698d0..62b071f801703 100644 --- a/src/rgw/driver/d4n/d4n_directory.cc +++ b/src/rgw/driver/d4n/d4n_directory.cc @@ -84,7 +84,7 @@ int ObjectDirectory::set(CacheObj* object, optional_yield y) { redisValues.push_back("bucketName"); redisValues.push_back(object->bucketName); redisValues.push_back("creationTime"); - redisValues.push_back(std::to_string(object->creationTime)); + redisValues.push_back(object->creationTime); redisValues.push_back("dirty"); redisValues.push_back(std::to_string(object->dirty)); redisValues.push_back("objHosts"); @@ -145,7 +145,7 @@ int ObjectDirectory::get(CacheObj* object, optional_yield y) { object->objName = std::get<0>(resp).value()[0]; object->bucketName = std::get<0>(resp).value()[1]; - object->creationTime = boost::lexical_cast(std::get<0>(resp).value()[2]); + object->creationTime = std::get<0>(resp).value()[2]; object->dirty = boost::lexical_cast(std::get<0>(resp).value()[3]); { @@ -354,7 +354,7 @@ int BlockDirectory::set(CacheBlock* block, optional_yield y) { redisValues.push_back("bucketName"); redisValues.push_back(block->cacheObj.bucketName); redisValues.push_back("creationTime"); - redisValues.push_back(std::to_string(block->cacheObj.creationTime)); + redisValues.push_back(block->cacheObj.creationTime); redisValues.push_back("dirty"); redisValues.push_back(std::to_string(block->cacheObj.dirty)); redisValues.push_back("objHosts"); @@ -438,7 +438,7 @@ int BlockDirectory::get(CacheBlock* block, optional_yield y) { block->cacheObj.objName = std::get<0>(resp).value()[5]; block->cacheObj.bucketName = std::get<0>(resp).value()[6]; - block->cacheObj.creationTime = boost::lexical_cast(std::get<0>(resp).value()[7]); + block->cacheObj.creationTime = std::get<0>(resp).value()[7]; block->cacheObj.dirty = boost::lexical_cast(std::get<0>(resp).value()[8]); { diff --git a/src/rgw/driver/d4n/d4n_directory.h b/src/rgw/driver/d4n/d4n_directory.h index b98859d70dbe9..e9f62dd510721 100644 --- a/src/rgw/driver/d4n/d4n_directory.h +++ b/src/rgw/driver/d4n/d4n_directory.h @@ -19,7 +19,7 @@ using boost::redis::response; struct CacheObj { std::string objName; /* S3 object name */ std::string bucketName; /* S3 bucket name */ - time_t creationTime; /* Creation time of the S3 Object */ + std::string creationTime; /* Creation time of the S3 Object */ bool dirty; std::vector hostsList; /* List of hostnames of object locations for multiple backends */ }; diff --git a/src/rgw/driver/d4n/rgw_sal_d4n.cc b/src/rgw/driver/d4n/rgw_sal_d4n.cc index b1d8e33187aad..622d91dac0251 100644 --- a/src/rgw/driver/d4n/rgw_sal_d4n.cc +++ b/src/rgw/driver/d4n/rgw_sal_d4n.cc @@ -645,18 +645,17 @@ int D4NFilterObject::D4NFilterReadOp::D4NFilterGetCB::handle_data(bufferlist& bl const std::lock_guard l(d4n_get_data_lock); rgw::d4n::CacheBlock block; rgw::d4n::BlockDirectory* blockDir = source->driver->get_block_dir(); - block.version = ""; + block.version = ""; // TODO: initialize correctly block.hostsList.push_back(blockDir->cct->_conf->rgw_local_cache_address); block.cacheObj.objName = source->get_key().get_oid(); block.cacheObj.bucketName = source->get_bucket()->get_name(); - block.cacheObj.creationTime = 0; - block.cacheObj.dirty = false;// update hostsList since may overwrite existing hosts -Sam - block.cacheObj.hostsList.push_back(blockDir->cct->_conf->rgw_local_cache_address); // Is the entire object getting stored in the local cache as well or only blocks? -Sam - Attrs attrs; // empty attrs for block sets + block.cacheObj.creationTime = to_iso_8601(source->get_mtime()); + block.cacheObj.dirty = false; + Attrs attrs; // empty attrs for cache sets if (bl.length() > 0 && last_part) { // if bl = bl_rem has data and this is the last part, write it to cache std::string oid = this->oid + "_" + std::to_string(ofs) + "_" + std::to_string(bl_len); - block.blockID = ofs; // TODO: fill out block correctly + block.blockID = ofs; block.size = bl.length(); if (filter->get_policy_driver()->get_cache_policy()->eviction(dpp, block.size, *y) == 0) { if (filter->get_cache_driver()->put_async(dpp, oid, bl, bl.length(), attrs) == 0) { @@ -702,7 +701,7 @@ int D4NFilterObject::D4NFilterReadOp::D4NFilterGetCB::handle_data(bufferlist& bl if (bl_rem.length() == rgw_get_obj_max_req_size) { std::string oid = this->oid + "_" + std::to_string(ofs) + "_" + std::to_string(bl_rem.length()); ofs += bl_rem.length(); - block.blockID = ofs; // TODO: fill out block correctly + block.blockID = ofs; block.size = bl_rem.length(); if (filter->get_policy_driver()->get_cache_policy()->eviction(dpp, block.size, *y) == 0) { if (filter->get_cache_driver()->put_async(dpp, oid, bl_rem, bl_rem.length(), attrs) == 0) { @@ -731,15 +730,13 @@ int D4NFilterObject::D4NFilterReadOp::D4NFilterGetCB::handle_data(bufferlist& bl int D4NFilterObject::D4NFilterDeleteOp::delete_obj(const DoutPrefixProvider* dpp, optional_yield y, uint32_t flags) { - rgw::d4n::CacheBlock block = rgw::d4n::CacheBlock{ - .cacheObj = { - .objName = source->get_key().get_oid(), - .bucketName = source->get_bucket()->get_name() - }, - .blockID = 0 // TODO: get correct blockID - }; - if (source->driver->get_block_dir()->del(&block, y) < 0) - ldpp_dout(dpp, 10) << "D4NFilterObject::" << __func__ << "(): BlockDirectory del method failed." << dendl; + rgw::d4n::CacheObj obj = rgw::d4n::CacheObj{ // TODO: Add logic to ObjectDirectory del method to also delete all blocks belonging to that object + .objName = source->get_key().get_oid(), + .bucketName = source->get_bucket()->get_name() + }; + + if (source->driver->get_obj_dir()->del(&obj, y) < 0) + ldpp_dout(dpp, 10) << "D4NFilterObject::" << __func__ << "(): ObjectDirectory del method failed." << dendl; Attrs::iterator attrs; Attrs currentattrs = source->get_attrs(); @@ -789,22 +786,16 @@ int D4NFilterWriter::complete(size_t accounted_size, const std::string& etag, const req_context& rctx, uint32_t flags) { - rgw::d4n::CacheBlock block = rgw::d4n::CacheBlock{ - .cacheObj = { - .objName = obj->get_key().get_oid(), - .bucketName = obj->get_bucket()->get_name(), - .creationTime = 0, // TODO: get correct value - .dirty = false, - .hostsList = { driver->get_block_dir()->cct->_conf->rgw_local_cache_address } - }, - .blockID = 0, // TODO: get correct blockID - .version = "", - .size = accounted_size, - .hostsList = { driver->get_block_dir()->cct->_conf->rgw_local_cache_address } + rgw::d4n::CacheObj object = rgw::d4n::CacheObj{ + .objName = obj->get_key().get_oid(), + .bucketName = obj->get_bucket()->get_name(), + .creationTime = to_iso_8601(*mtime), + .dirty = false, + .hostsList = { driver->get_block_dir()->cct->_conf->rgw_local_cache_address } }; - if (driver->get_block_dir()->set(&block, y) < 0) - ldpp_dout(save_dpp, 10) << "D4NFilterWriter::" << __func__ << "(): BlockDirectory set method failed." << dendl; + if (driver->get_obj_dir()->set(&object, y) < 0) + ldpp_dout(save_dpp, 10) << "D4NFilterWriter::" << __func__ << "(): ObjectDirectory set method failed." << dendl; /* Retrieve complete set of attrs */ int ret = next->complete(accounted_size, etag, mtime, set_mtime, attrs, diff --git a/src/rgw/driver/d4n/rgw_sal_d4n.h b/src/rgw/driver/d4n/rgw_sal_d4n.h index a7d7d1c6f8462..fc7f844ee7c77 100644 --- a/src/rgw/driver/d4n/rgw_sal_d4n.h +++ b/src/rgw/driver/d4n/rgw_sal_d4n.h @@ -198,6 +198,7 @@ class D4NFilterObject : public FilterObject { optional_yield y, const DoutPrefixProvider* dpp) override; virtual int delete_obj_attrs(const DoutPrefixProvider* dpp, const char* attr_name, optional_yield y) override; + virtual ceph::real_time get_mtime(void) const override { return next->get_mtime(); }; virtual std::unique_ptr get_read_op() override; virtual std::unique_ptr get_delete_op() override; diff --git a/src/test/rgw/test_d4n_directory.cc b/src/test/rgw/test_d4n_directory.cc index dfb01de2e9461..8d00a2b023942 100644 --- a/src/test/rgw/test_d4n_directory.cc +++ b/src/test/rgw/test_d4n_directory.cc @@ -51,7 +51,7 @@ class ObjectDirectoryFixture: public ::testing::Test { obj = new rgw::d4n::CacheObj{ .objName = "testName", .bucketName = "testBucket", - .creationTime = 0, + .creationTime = "", .dirty = false, .hostsList = { env->redisHost } }; @@ -84,7 +84,7 @@ class ObjectDirectoryFixture: public ::testing::Test { net::io_context io; connection* conn; - std::vector vals{"testName", "testBucket", "0", "0", env->redisHost}; + std::vector vals{"testName", "testBucket", "", "0", env->redisHost}; std::vector fields{"objName", "bucketName", "creationTime", "dirty", "objHosts"}; }; @@ -96,7 +96,7 @@ class BlockDirectoryFixture: public ::testing::Test { .cacheObj = { .objName = "testName", .bucketName = "testBucket", - .creationTime = 0, + .creationTime = "", .dirty = false, .hostsList = { env->redisHost } }, @@ -135,7 +135,7 @@ class BlockDirectoryFixture: public ::testing::Test { connection* conn; std::vector vals{"0", "", "0", "0", env->redisHost, - "testName", "testBucket", "0", "0", env->redisHost}; + "testName", "testBucket", "", "0", env->redisHost}; std::vector fields{"blockID", "version", "size", "globalWeight", "blockHosts", "objName", "bucketName", "creationTime", "dirty", "objHosts"}; }; diff --git a/src/test/rgw/test_d4n_policy.cc b/src/test/rgw/test_d4n_policy.cc index 2d299ebf2d1a0..5aff9b26a2a6a 100644 --- a/src/test/rgw/test_d4n_policy.cc +++ b/src/test/rgw/test_d4n_policy.cc @@ -48,7 +48,7 @@ class LFUDAPolicyFixture : public ::testing::Test { .cacheObj = { .objName = "testName", .bucketName = "testBucket", - .creationTime = 0, + .creationTime = "", .dirty = false, .hostsList = { env->redisHost } }, @@ -192,7 +192,7 @@ TEST_F(LFUDAPolicyFixture, RemoteGetBlockYield) .cacheObj = { .objName = "victimName", .bucketName = "testBucket", - .creationTime = 0, + .creationTime = "", .dirty = false, .hostsList = { env->redisHost } }, -- 2.39.5