From 194a3d3eaf59e8cea1b8ba42e3f11b986efc814c Mon Sep 17 00:00:00 2001 From: Samarah Date: Mon, 31 Jul 2023 13:32:49 -0400 Subject: [PATCH] rgw/cache: this commit squashes following commits related to changes in redis driver and ssd driver. rgw/redis: Add `del`, `put_async`, and `calculate_free_space` methods to RedisDriver rgw/redis: Work on RedisDriver `put_async` and `get_async` methods rgw/cache: Add error-checking to `SSDDriver::set_attr` rgw/redis: Remove `calculate_free_space` method from RedisDriver, fix `free_space` calculations, clean up async code Signed-off-by: Samarah --- src/rgw/driver/d4n/rgw_sal_d4n.cc | 4 +- src/rgw/driver/d4n/rgw_sal_d4n.h | 1 + src/rgw/rgw_redis_driver.cc | 142 +++++++++++++++--------------- src/rgw/rgw_redis_driver.h | 8 +- src/rgw/rgw_ssd_driver.cc | 9 +- src/test/rgw/test_redis_driver.cc | 4 +- 6 files changed, 84 insertions(+), 84 deletions(-) diff --git a/src/rgw/driver/d4n/rgw_sal_d4n.cc b/src/rgw/driver/d4n/rgw_sal_d4n.cc index 754ba16439004..430bf0c0c8a39 100644 --- a/src/rgw/driver/d4n/rgw_sal_d4n.cc +++ b/src/rgw/driver/d4n/rgw_sal_d4n.cc @@ -45,8 +45,8 @@ D4NFilterDriver::D4NFilterDriver(Driver* _next, boost::asio::io_context& io_cont partition_info.type = "read-cache"; partition_info.size = g_conf()->rgw_d4n_l1_datacache_size; - //cacheDriver = new rgw::cache::RedisDriver(io_context, partition_info); // change later -Sam - cacheDriver = new rgw::cache::SSDDriver(partition_info); + cacheDriver = new rgw::cache::RedisDriver(io_context, partition_info); // change later -Sam + //cacheDriver = new rgw::cache::SSDDriver(partition_info); objDir = new rgw::d4n::ObjectDirectory(io_context); blockDir = new rgw::d4n::BlockDirectory(io_context); cacheBlock = new rgw::d4n::CacheBlock(); diff --git a/src/rgw/driver/d4n/rgw_sal_d4n.h b/src/rgw/driver/d4n/rgw_sal_d4n.h index 0e0bb6e4d9422..c30d4d59d2ee0 100644 --- a/src/rgw/driver/d4n/rgw_sal_d4n.h +++ b/src/rgw/driver/d4n/rgw_sal_d4n.h @@ -22,6 +22,7 @@ #include "common/dout.h" #include "rgw_aio_throttle.h" #include "rgw_ssd_driver.h" +#include "rgw_redis_driver.h" #include "rgw_redis_driver.h" #include "driver/d4n/d4n_directory.h" diff --git a/src/rgw/rgw_redis_driver.cc b/src/rgw/rgw_redis_driver.cc index b1d0db6fb3b4c..feca1e8d984f7 100644 --- a/src/rgw/rgw_redis_driver.cc +++ b/src/rgw/rgw_redis_driver.cc @@ -75,47 +75,6 @@ int RedisDriver::remove_partition_info(Partition& info) return partitions.erase(key); } -/* -uint64_t RedisDriver::get_free_space(const DoutPrefixProvider* dpp) -{ - int result = -1; - - if (!client.is_connected()) - find_client(dpp); - - try { - client.info([&result](cpp_redis::reply &reply) { - if (!reply.is_null()) { - int usedMem = -1; - int maxMem = -1; - - std::istringstream iss(reply.as_string()); - std::string line; - while (std::getline(iss, line)) { - size_t pos = line.find_first_of(":"); - if (pos != std::string::npos) { - if (line.substr(0, pos) == "used_memory") { - usedMem = std::stoi(line.substr(pos + 1, line.length() - pos - 2)); - } else if (line.substr(0, line.find_first_of(":")) == "maxmemory") { - maxMem = std::stoi(line.substr(pos + 1, line.length() - pos - 2)); - } - } - } - - if (usedMem > -1 && maxMem > -1) - result = maxMem - usedMem; - } - }); - - client.sync_commit(std::chrono::milliseconds(1000)); - } catch(std::exception &e) { - return -1; - } - - return result; -} -*/ - std::optional RedisDriver::get_partition_info(const DoutPrefixProvider* dpp, const std::string& name, const std::string& type) { std::string key = name + type; @@ -137,20 +96,6 @@ std::vector RedisDriver::list_partitions(const DoutPrefixProvider* dp return partitions_v; } -/* Currently an attribute but will also be part of the Entry metadata once consistency is guaranteed -Sam -int RedisDriver::update_local_weight(const DoutPrefixProvider* dpp, std::string key, int localWeight) -{ - auto iter = entries.find(key); - - if (iter != entries.end()) { - iter->second.localWeight = localWeight; - return 0; - } - - return -1; -} -*/ - int RedisDriver::initialize(CephContext* cct, const DoutPrefixProvider* dpp) { this->cct = cct; @@ -177,7 +122,7 @@ int RedisDriver::put(const DoutPrefixProvider* dpp, const std::string& key, buff { std::string entry = partition_info.location + key; - /* Every set will be treated as new */ // or maybe, if key exists, simply return? -Sam + /* Every set will be treated as new */ try { boost::system::error_code ec; response resp; @@ -200,7 +145,8 @@ int RedisDriver::put(const DoutPrefixProvider* dpp, const std::string& key, buff return -1; } - return 0; // why is offset necessarily 0? -Sam + this->free_space -= bl.length(); + return 0; } int RedisDriver::get(const DoutPrefixProvider* dpp, const std::string& key, off_t offset, uint64_t len, bufferlist& bl, rgw::sal::Attrs& attrs, optional_yield y) @@ -239,22 +185,55 @@ int RedisDriver::get(const DoutPrefixProvider* dpp, const std::string& key, off_ int RedisDriver::del(const DoutPrefixProvider* dpp, const std::string& key, optional_yield y) { std::string entry = partition_info.location + key; + response resp; try { boost::system::error_code ec; - response resp; request req; - req.push("DEL", entry); + req.push("HEXISTS", entry, "data"); redis_exec(conn, ec, req, resp, y); if (ec) return -1; - - return std::get<0>(resp).value() - 1; } catch(std::exception &e) { return -1; } + + if (std::get<0>(resp).value()) { + response data; + response ret; + + try { + boost::system::error_code ec; + request req; + req.push("HGET", entry, "data"); + + redis_exec(conn, ec, req, data, y); + + if (ec) + return -1; + } catch(std::exception &e) { + return -1; + } + + try { + boost::system::error_code ec; + request req; + req.push("DEL", entry); + + redis_exec(conn, ec, req, ret, y); + + if (!std::get<0>(ret).value() || ec) + return -1; + } catch(std::exception &e) { + return -1; + } + + this->free_space += std::get<0>(data).value().length(); + } + + return 0; } int RedisDriver::append_data(const DoutPrefixProvider* dpp, const::std::string& key, bufferlist& bl_data, optional_yield y) @@ -296,13 +275,13 @@ int RedisDriver::append_data(const DoutPrefixProvider* dpp, const::std::string& return -1; } + this->free_space -= bl_data.length(); return 0; } int RedisDriver::delete_data(const DoutPrefixProvider* dpp, const::std::string& key, optional_yield y) { std::string entry = partition_info.location + key; - response value; response resp; try { @@ -319,19 +298,38 @@ int RedisDriver::delete_data(const DoutPrefixProvider* dpp, const::std::string& } if (std::get<0>(resp).value()) { + response data; + response ret; + + try { + boost::system::error_code ec; + request req; + req.push("HGET", entry, "data"); + + redis_exec(conn, ec, req, data, y); + + if (ec) { + return -1; + } + } catch(std::exception &e) { + return -1; + } + try { boost::system::error_code ec; request req; req.push("HDEL", entry, "data"); - redis_exec(conn, ec, req, value, y); + redis_exec(conn, ec, req, ret, y); - if (!std::get<0>(value).value() || ec) { + if (!std::get<0>(ret).value() || ec) { return -1; } } catch(std::exception &e) { return -1; } + + this->free_space += std::get<0>(data).value().length(); } return 0; @@ -349,7 +347,7 @@ int RedisDriver::get_attrs(const DoutPrefixProvider* dpp, const std::string& key redis_exec(conn, ec, req, resp, y); - if (ec) + if (std::get<0>(resp).value().empty() || ec) return -1; for (auto const& it : std::get<0>(resp).value()) { @@ -536,16 +534,14 @@ rgw::AioResultList RedisDriver::get_async(const DoutPrefixProvider* dpp, optiona return aio->get(r_obj, redis_read_op(y, conn, ofs, len, entry), cost, id); } -#if 0 -int RedisDriver::AsyncReadOp::init(const DoutPrefixProvider *dpp, CephContext* cct, const std::string& file_path, off_t read_ofs, off_t read_len, void* arg) +int RedisDriver::put_async(const DoutPrefixProvider* dpp, const std::string& key, bufferlist& bl, uint64_t len, rgw::sal::Attrs& attrs) { + // TODO: implement + return -1; +} + +void RedisDriver::shutdown() { // call cancel() on the connection's executor boost::asio::dispatch(conn->get_executor(), [c = conn] { c->cancel(); }); } -#endif - -int RedisDriver::put_async(const DoutPrefixProvider* dpp, const std::string& key, bufferlist& bl, uint64_t len, rgw::sal::Attrs& attrs) -{ - return 0; -} } } // namespace rgw::cache diff --git a/src/rgw/rgw_redis_driver.h b/src/rgw/rgw_redis_driver.h index 9b818b42c390a..954a5b2373b1a 100644 --- a/src/rgw/rgw_redis_driver.h +++ b/src/rgw/rgw_redis_driver.h @@ -32,8 +32,6 @@ class RedisDriver : public CacheDriver { remove_partition_info(partition_info); } - //int update_local_weight(const DoutPrefixProvider* dpp, std::string key, int localWeight); // may need to exist for base class -Sam - /* Partition */ virtual Partition get_current_partition_info(const DoutPrefixProvider* dpp) override { return partition_info; } virtual uint64_t get_free_space(const DoutPrefixProvider* dpp) override { return free_space; } // how to get this from redis server? -Sam @@ -42,6 +40,7 @@ class RedisDriver : public CacheDriver { virtual int initialize(CephContext* cct, const DoutPrefixProvider* dpp) override; virtual int put(const DoutPrefixProvider* dpp, const std::string& key, bufferlist& bl, uint64_t len, rgw::sal::Attrs& attrs, optional_yield y) override; + virtual int put_async(const DoutPrefixProvider* dpp, const std::string& key, bufferlist& bl, uint64_t len, rgw::sal::Attrs& attrs) override; virtual int get(const DoutPrefixProvider* dpp, const std::string& key, off_t offset, uint64_t len, bufferlist& bl, rgw::sal::Attrs& attrs, optional_yield y) override; virtual rgw::AioResultList get_async(const DoutPrefixProvider* dpp, optional_yield y, rgw::Aio* aio, const std::string& key, off_t ofs, uint64_t len, uint64_t cost, uint64_t id) override; virtual int del(const DoutPrefixProvider* dpp, const std::string& key, optional_yield y) override; @@ -53,12 +52,11 @@ class RedisDriver : public CacheDriver { virtual int delete_attrs(const DoutPrefixProvider* dpp, const std::string& key, rgw::sal::Attrs& del_attrs, optional_yield y) override; virtual int set_attr(const DoutPrefixProvider* dpp, const std::string& key, const std::string& attr_name, const std::string& attr_val, optional_yield y) override; virtual std::string get_attr(const DoutPrefixProvider* dpp, const std::string& key, const std::string& attr_name, optional_yield y) override; + void shutdown(); - virtual int put_async(const DoutPrefixProvider* dpp, const std::string& key, bufferlist& bl, uint64_t len, rgw::sal::Attrs& attrs) override; struct redis_response { boost::redis::response resp; }; - void shutdown(); struct redis_aio_handler { rgw::Aio* throttle = nullptr; @@ -66,7 +64,7 @@ class RedisDriver : public CacheDriver { std::shared_ptr s; /* Read Callback */ - void operator()(boost::system::error_code ec, long unsigned int size) const { + void operator()(boost::system::error_code ec, auto) const { r.result = -ec.value(); r.data.append(std::get<0>(s->resp).value().c_str()); throttle->put(r); diff --git a/src/rgw/rgw_ssd_driver.cc b/src/rgw/rgw_ssd_driver.cc index 8dea916d71ff7..d9ac7aee6bfbb 100644 --- a/src/rgw/rgw_ssd_driver.cc +++ b/src/rgw/rgw_ssd_driver.cc @@ -518,12 +518,19 @@ std::string SSDDriver::get_attr(const DoutPrefixProvider* dpp, const std::string int SSDDriver::set_attr(const DoutPrefixProvider* dpp, const std::string& key, const std::string& attr_name, const std::string& attr_val, optional_yield y) { + int e; std::string location = partition_info.location + key; ldpp_dout(dpp, 20) << "SSDCache: " << __func__ << "(): location=" << location << dendl; ldpp_dout(dpp, 20) << "SSDCache: " << __func__ << "(): set_attr: key: " << attr_name << " val: " << attr_val << dendl; - return setxattr(location.c_str(), attr_name.c_str(), attr_val.c_str(), attr_val.size(), 0); + int ret = setxattr(location.c_str(), attr_name.c_str(), attr_val.c_str(), attr_val.size(), 0); + if (ret < 0) { + e = errno; + ldpp_dout(dpp, 0) << "SSDCache: " << __func__ << "(): ERROR: " << cpp_strerror(e) << dendl; + } + + return ret; } int SSDDriver::delete_attr(const DoutPrefixProvider* dpp, const std::string& key, const std::string& attr_name) diff --git a/src/test/rgw/test_redis_driver.cc b/src/test/rgw/test_redis_driver.cc index a1f8783d54bdc..a57d1a2f3e923 100644 --- a/src/test/rgw/test_redis_driver.cc +++ b/src/test/rgw/test_redis_driver.cc @@ -5,6 +5,7 @@ #include "gtest/gtest.h" #include "common/ceph_argparse.h" #include "rgw_auth_registry.h" +#include "rgw_aio_throttle.h" #include "rgw_redis_driver.h" namespace net = boost::asio; @@ -184,8 +185,6 @@ TEST_F(RedisDriverFixture, DelYield) io.run(); } -// How can I test get_async? -Sam - TEST_F(RedisDriverFixture, AppendDataYield) { spawn::spawn(io, [this] (spawn::yield_context yield) { @@ -486,7 +485,6 @@ TEST_F(RedisDriverFixture, GetAttrYield) io.run(); } - int main(int argc, char *argv[]) { ::testing::InitGoogleTest(&argc, argv); -- 2.39.5