From 9ba9b27c8836451d055257f3b52fe04b4863db9f Mon Sep 17 00:00:00 2001 From: Samarah Date: Wed, 3 Jan 2024 15:52:25 +0000 Subject: [PATCH] rgw: this commit squashes commits performing cleanup in various files like d4n filter, policy, cache backend, aio and process_env. d4n/filter: Remove unnecessary erasure of attrs in read op's `prepare` method d4n: Remove `dout` definitions from header files rgw: Remove unnecessary includes in `rgw_process_env.h` rgw: Remove `cache_read_op` from `rgw_aio.h` rgw: Return POSIX error codes and perform minor cleanup for D4N and Redis files Signed-off-by: Samarah --- src/rgw/driver/d4n/d4n_directory.cc | 214 +++++++++++++++++----------- src/rgw/driver/d4n/d4n_directory.h | 2 - src/rgw/driver/d4n/d4n_policy.cc | 101 +++++++------ src/rgw/driver/d4n/d4n_policy.h | 2 - src/rgw/driver/d4n/rgw_sal_d4n.cc | 1 - src/rgw/rgw_aio.h | 3 - src/rgw/rgw_process_env.h | 2 - src/rgw/rgw_redis_driver.cc | 208 +++++++++++++++++---------- src/rgw/rgw_redis_driver.h | 5 +- src/test/rgw/test_d4n_directory.cc | 2 + src/test/rgw/test_d4n_policy.cc | 2 + src/test/rgw/test_redis_driver.cc | 2 + 12 files changed, 320 insertions(+), 224 deletions(-) diff --git a/src/rgw/driver/d4n/d4n_directory.cc b/src/rgw/driver/d4n/d4n_directory.cc index 62b071f801703..856c88be3563d 100644 --- a/src/rgw/driver/d4n/d4n_directory.cc +++ b/src/rgw/driver/d4n/d4n_directory.cc @@ -1,5 +1,6 @@ #include #include "common/async/blocked_completion.h" +#include "common/dout.h" #include "d4n_directory.h" namespace rgw { namespace d4n { @@ -43,11 +44,13 @@ void redis_exec(std::shared_ptr conn, } } -std::string ObjectDirectory::build_index(CacheObj* object) { +std::string ObjectDirectory::build_index(CacheObj* object) +{ return object->bucketName + "_" + object->objName; } -int ObjectDirectory::exist_key(CacheObj* object, optional_yield y) { +int ObjectDirectory::exist_key(CacheObj* object, optional_yield y) +{ std::string key = build_index(object); response resp; @@ -60,7 +63,7 @@ int ObjectDirectory::exist_key(CacheObj* object, optional_yield y) { if ((bool)ec) return false; - } catch(std::exception &e) {} + } catch (std::exception &e) {} return std::get<0>(resp).value(); } @@ -71,7 +74,8 @@ void ObjectDirectory::shutdown() boost::asio::dispatch(conn->get_executor(), [c = conn] { c->cancel(); }); } -int ObjectDirectory::set(CacheObj* object, optional_yield y) { +int ObjectDirectory::set(CacheObj* object, optional_yield y) +{ std::string key = build_index(object); /* Every set will be treated as new */ @@ -109,17 +113,18 @@ int ObjectDirectory::set(CacheObj* object, optional_yield y) { redis_exec(conn, ec, req, resp, y); - if (std::get<0>(resp).value() != "OK" || (bool)ec) { - return -1; + if (ec) { + return -ec.value(); } - } catch(std::exception &e) { - return -1; + } catch (std::exception &e) { + return -EINVAL; } return 0; } -int ObjectDirectory::get(CacheObj* object, optional_yield y) { +int ObjectDirectory::get(CacheObj* object, optional_yield y) +{ std::string key = build_index(object); if (exist_key(object, y)) { @@ -139,8 +144,10 @@ int ObjectDirectory::get(CacheObj* object, optional_yield y) { redis_exec(conn, ec, req, resp, y); - if (!std::get<0>(resp).value().size() || (bool)ec) { - return -1; + if (std::get<0>(resp).value().empty()) { + return -ENOENT; + } else if (ec) { + return -ec.value(); } object->objName = std::get<0>(resp).value()[0]; @@ -157,17 +164,18 @@ int ObjectDirectory::get(CacheObj* object, optional_yield y) { object->hostsList.push_back(host); } } - } catch(std::exception &e) { - return -1; + } catch (std::exception &e) { + return -EINVAL; } } else { - return -2; + return -ENOENT; } return 0; } -int ObjectDirectory::copy(CacheObj* object, std::string copyName, std::string copyBucketName, optional_yield y) { +int ObjectDirectory::copy(CacheObj* object, std::string copyName, std::string copyBucketName, optional_yield y) +{ std::string key = build_index(object); auto copyObj = CacheObj{ .objName = copyName, .bucketName = copyBucketName }; std::string copyKey = build_index(©Obj); @@ -183,8 +191,8 @@ int ObjectDirectory::copy(CacheObj* object, std::string copyName, std::string co redis_exec(conn, ec, req, resp, y); - if ((bool)ec) { - return -1; + if (ec) { + return -ec.value(); } } @@ -196,21 +204,22 @@ int ObjectDirectory::copy(CacheObj* object, std::string copyName, std::string co redis_exec(conn, ec, req, res, y); - if (std::get<0>(res).value() != "OK" || (bool)ec) { - return -1; - } + if (ec) { + return -ec.value(); + } } return std::get<0>(resp).value() - 1; - } catch(std::exception &e) { - return -1; + } catch (std::exception &e) { + return -EINVAL; } } else { - return -2; + return -ENOENT; } } -int ObjectDirectory::del(CacheObj* object, optional_yield y) { +int ObjectDirectory::del(CacheObj* object, optional_yield y) +{ std::string key = build_index(object); if (exist_key(object, y)) { @@ -222,19 +231,21 @@ int ObjectDirectory::del(CacheObj* object, optional_yield y) { redis_exec(conn, ec, req, resp, y); - if ((bool)ec) - return -1; + if (ec) { + return -ec.value(); + } return std::get<0>(resp).value() - 1; - } catch(std::exception &e) { - return -1; + } catch (std::exception &e) { + return -EINVAL; } } else { return 0; /* No delete was necessary */ } } -int ObjectDirectory::update_field(CacheObj* object, std::string field, std::string value, optional_yield y) { +int ObjectDirectory::update_field(CacheObj* object, std::string field, std::string value, optional_yield y) +{ std::string key = build_index(object); if (exist_key(object, y)) { @@ -248,8 +259,11 @@ int ObjectDirectory::update_field(CacheObj* object, std::string field, std::stri redis_exec(conn, ec, req, resp, y); - if (!std::get<0>(resp).value() || (bool)ec) - return -1; + if (!std::get<0>(resp).value()) { + return -ENOENT; + } else if (ec) { + return -ec.value(); + } } if (field == "objHosts") { @@ -261,8 +275,11 @@ int ObjectDirectory::update_field(CacheObj* object, std::string field, std::stri redis_exec(conn, ec, req, resp, y); - if (!std::get<0>(resp).value().size() || (bool)ec) - return -1; + if (std::get<0>(resp).value().empty()) { + return -ENOENT; + } else if (ec) { + return -ec.value(); + } std::get<0>(resp).value() += "_"; std::get<0>(resp).value() += value; @@ -277,25 +294,27 @@ int ObjectDirectory::update_field(CacheObj* object, std::string field, std::stri redis_exec(conn, ec, req, resp, y); - if ((bool)ec) { - return -1; + if (ec) { + return -ec.value(); } return std::get<0>(resp).value(); } - } catch(std::exception &e) { - return -1; + } catch (std::exception &e) { + return -EINVAL; } } else { - return -2; + return -ENOENT; } } -std::string BlockDirectory::build_index(CacheBlock* block) { +std::string BlockDirectory::build_index(CacheBlock* block) +{ return block->cacheObj.bucketName + "_" + block->cacheObj.objName + "_" + std::to_string(block->blockID) + "_" + std::to_string(block->size); } -int BlockDirectory::exist_key(CacheBlock* block, optional_yield y) { +int BlockDirectory::exist_key(CacheBlock* block, optional_yield y) +{ std::string key = build_index(block); response resp; @@ -308,7 +327,7 @@ int BlockDirectory::exist_key(CacheBlock* block, optional_yield y) { if ((bool)ec) return false; - } catch(std::exception &e) {} + } catch (std::exception &e) {} return std::get<0>(resp).value(); } @@ -319,7 +338,8 @@ void BlockDirectory::shutdown() boost::asio::dispatch(conn->get_executor(), [c = conn] { c->cancel(); }); } -int BlockDirectory::set(CacheBlock* block, optional_yield y) { +int BlockDirectory::set(CacheBlock* block, optional_yield y) +{ std::string key = build_index(block); /* Every set will be treated as new */ @@ -380,17 +400,18 @@ int BlockDirectory::set(CacheBlock* block, optional_yield y) { redis_exec(conn, ec, req, resp, y); - if (std::get<0>(resp).value() != "OK" || (bool)ec) { - return -1; + if (ec) { + return -ec.value(); } - } catch(std::exception &e) { - return -1; + } catch (std::exception &e) { + return -EINVAL; } return 0; } -int BlockDirectory::get(CacheBlock* block, optional_yield y) { +int BlockDirectory::get(CacheBlock* block, optional_yield y) +{ std::string key = build_index(block); if (exist_key(block, y)) { @@ -416,8 +437,10 @@ int BlockDirectory::get(CacheBlock* block, optional_yield y) { redis_exec(conn, ec, req, resp, y); - if (!std::get<0>(resp).value().size() || (bool)ec) { - return -1; + if (std::get<0>(resp).value().empty()) { + return -ENOENT; + } else if (ec) { + return -ec.value(); } block->blockID = boost::lexical_cast(std::get<0>(resp).value()[0]); @@ -451,17 +474,18 @@ int BlockDirectory::get(CacheBlock* block, optional_yield y) { block->cacheObj.hostsList.push_back(host); } } - } catch(std::exception &e) { - return -1; + } catch (std::exception &e) { + return -EINVAL; } } else { - return -2; + return -ENOENT; } return 0; } -int BlockDirectory::copy(CacheBlock* block, std::string copyName, std::string copyBucketName, optional_yield y) { +int BlockDirectory::copy(CacheBlock* block, std::string copyName, std::string copyBucketName, optional_yield y) +{ std::string key = build_index(block); auto copyBlock = CacheBlock{ .cacheObj = { .objName = copyName, .bucketName = copyBucketName }, .blockID = 0 }; std::string copyKey = build_index(©Block); @@ -477,8 +501,8 @@ int BlockDirectory::copy(CacheBlock* block, std::string copyName, std::string co redis_exec(conn, ec, req, resp, y); - if ((bool)ec) { - return -1; + if (ec) { + return -ec.value(); } } @@ -490,21 +514,22 @@ int BlockDirectory::copy(CacheBlock* block, std::string copyName, std::string co redis_exec(conn, ec, req, res, y); - if (std::get<0>(res).value() != "OK" || (bool)ec) { - return -1; - } + if (ec) { + return -ec.value(); + } } return std::get<0>(resp).value() - 1; - } catch(std::exception &e) { - return -1; + } catch (std::exception &e) { + return -EINVAL; } } else { - return -2; + return -ENOENT; } } -int BlockDirectory::del(CacheBlock* block, optional_yield y) { +int BlockDirectory::del(CacheBlock* block, optional_yield y) +{ std::string key = build_index(block); if (exist_key(block, y)) { @@ -516,19 +541,21 @@ int BlockDirectory::del(CacheBlock* block, optional_yield y) { redis_exec(conn, ec, req, resp, y); - if ((bool)ec) - return -1; + if (ec) { + return -ec.value(); + } return std::get<0>(resp).value() - 1; - } catch(std::exception &e) { - return -1; + } catch (std::exception &e) { + return -EINVAL; } } else { return 0; /* No delete was necessary */ } } -int BlockDirectory::update_field(CacheBlock* block, std::string field, std::string value, optional_yield y) { +int BlockDirectory::update_field(CacheBlock* block, std::string field, std::string value, optional_yield y) +{ std::string key = build_index(block); if (exist_key(block, y)) { @@ -542,8 +569,11 @@ int BlockDirectory::update_field(CacheBlock* block, std::string field, std::stri redis_exec(conn, ec, req, resp, y); - if (!std::get<0>(resp).value() || (bool)ec) - return -1; + if (!std::get<0>(resp).value()) { + return -ENOENT; + } else if (ec) { + return -ec.value(); + } } if (field == "blockHosts") { @@ -555,8 +585,11 @@ int BlockDirectory::update_field(CacheBlock* block, std::string field, std::stri redis_exec(conn, ec, req, resp, y); - if (!std::get<0>(resp).value().size() || (bool)ec) - return -1; + if (std::get<0>(resp).value().empty()) { + return -ENOENT; + } else if (ec) { + return -ec.value(); + } std::get<0>(resp).value() += "_"; std::get<0>(resp).value() += value; @@ -571,21 +604,22 @@ int BlockDirectory::update_field(CacheBlock* block, std::string field, std::stri redis_exec(conn, ec, req, resp, y); - if ((bool)ec) { - return -1; + if (ec) { + return -ec.value(); } return std::get<0>(resp).value(); } - } catch(std::exception &e) { - return -1; + } catch (std::exception &e) { + return -EINVAL; } } else { - return -2; + return -ENOENT; } } -int BlockDirectory::remove_host(CacheBlock* block, std::string delValue, optional_yield y) { +int BlockDirectory::remove_host(CacheBlock* block, std::string delValue, optional_yield y) +{ std::string key = build_index(block); if (exist_key(block, y)) { @@ -599,8 +633,11 @@ int BlockDirectory::remove_host(CacheBlock* block, std::string delValue, optiona redis_exec(conn, ec, req, resp, y); - if (!std::get<0>(resp).value() || (bool)ec) - return -1; + if (!std::get<0>(resp).value()) { + return -ENOENT; + } else if (ec) { + return -ec.value(); + } } { @@ -611,8 +648,11 @@ int BlockDirectory::remove_host(CacheBlock* block, std::string delValue, optiona redis_exec(conn, ec, req, resp, y); - if (!std::get<0>(resp).value().size() || (bool)ec) - return -1; + if (std::get<0>(resp).value().empty()) { + return -ENOENT; + } else if (ec) { + return -ec.value(); + } if (std::get<0>(resp).value().find("_") == std::string::npos) /* Last host, delete entirely */ return del(block, y); @@ -622,7 +662,7 @@ int BlockDirectory::remove_host(CacheBlock* block, std::string delValue, optiona if (it != std::string::npos) result.erase(result.begin() + it, result.begin() + it + delValue.size()); else - return -1; + return -ENOENT; if (result[0] == '_') result.erase(0, 1); @@ -638,17 +678,17 @@ int BlockDirectory::remove_host(CacheBlock* block, std::string delValue, optiona redis_exec(conn, ec, req, resp, y); - if ((bool)ec) { - return -1; + if (ec) { + return -ec.value(); } return std::get<0>(resp).value(); } - } catch(std::exception &e) { - return -1; + } catch (std::exception &e) { + return -EINVAL; } } else { - return -2; + return -ENOENT; } } diff --git a/src/rgw/driver/d4n/d4n_directory.h b/src/rgw/driver/d4n/d4n_directory.h index e9f62dd510721..d60731f7ed52e 100644 --- a/src/rgw/driver/d4n/d4n_directory.h +++ b/src/rgw/driver/d4n/d4n_directory.h @@ -6,8 +6,6 @@ #include #include -#define dout_subsys ceph_subsys_rgw - namespace rgw { namespace d4n { namespace net = boost::asio; diff --git a/src/rgw/driver/d4n/d4n_policy.cc b/src/rgw/driver/d4n/d4n_policy.cc index 3da99e9792cfe..7b17c4477e744 100644 --- a/src/rgw/driver/d4n/d4n_policy.cc +++ b/src/rgw/driver/d4n/d4n_policy.cc @@ -3,6 +3,7 @@ #include #include "../../../common/async/yield_context.h" #include "common/async/blocked_completion.h" +#include "common/dout.h" namespace rgw { namespace d4n { @@ -51,12 +52,13 @@ int LFUDAPolicy::set_age(int age, optional_yield y) { redis_exec(conn, ec, req, resp, y); - if (ec) - return {}; + if (ec) { + return -ec.value(); + } return std::get<0>(resp).value(); /* Returns number of fields set */ - } catch(std::exception &e) { - return -1; + } catch (std::exception &e) { + return -EINVAL; } } @@ -70,17 +72,18 @@ int LFUDAPolicy::get_age(optional_yield y) { redis_exec(conn, ec, req, resp, y); - if (ec) - return -1; - } catch(std::exception &e) { - return -1; + if (ec) { + return -ec.value(); + } + } catch (std::exception &e) { + return -EINVAL; } if (!std::get<0>(resp).value()) { if (set_age(1, y)) /* Initialize age */ return 1; else - return -1; + return -ENOENT; } try { @@ -91,12 +94,13 @@ int LFUDAPolicy::get_age(optional_yield y) { redis_exec(conn, ec, req, value, y); - if (ec) - return -1; + if (ec) { + return -ec.value(); + } return std::stoi(std::get<0>(value).value()); - } catch(std::exception &e) { - return -1; + } catch (std::exception &e) { + return -EINVAL; } } @@ -109,12 +113,13 @@ int LFUDAPolicy::set_local_weight_sum(size_t weight, optional_yield y) { redis_exec(conn, ec, req, resp, y); - if (ec) - return -1; + if (ec) { + return -ec.value(); + } return std::get<0>(resp).value(); /* Returns number of fields set */ - } catch(std::exception &e) { - return -1; + } catch (std::exception &e) { + return -EINVAL; } } @@ -128,10 +133,11 @@ int LFUDAPolicy::get_local_weight_sum(optional_yield y) { redis_exec(conn, ec, req, resp, y); - if (ec) - return -1; - } catch(std::exception &e) { - return -1; + if (ec) { + return -ec.value(); + } + } catch (std::exception &e) { + return -EINVAL; } if (!std::get<0>(resp).value()) { @@ -139,8 +145,8 @@ int LFUDAPolicy::get_local_weight_sum(optional_yield y) { for (auto& entry : entries_map) sum += entry.second->localWeight; - if (set_local_weight_sum(sum, y) < 0) { /* Initialize */ - return -1; + if (int ret = set_local_weight_sum(sum, y) < 0) { /* Initialize */ + return ret; } else { return sum; } @@ -154,12 +160,13 @@ int LFUDAPolicy::get_local_weight_sum(optional_yield y) { redis_exec(conn, ec, req, value, y); - if (ec) - return -1; + if (ec) { + return -ec.value(); + } return std::stoi(std::get<0>(value).value()); - } catch(std::exception &e) { - return -1; + } catch (std::exception &e) { + return -EINVAL; } } @@ -251,7 +258,7 @@ int LFUDAPolicy::eviction(const DoutPrefixProvider* dpp, uint64_t size, optional if (victim == nullptr) { ldpp_dout(dpp, 10) << "LFUDAPolicy::" << __func__ << "(): Could not retrieve victim block." << dendl; delete victim; - return -1; + return -ENOENT; } const std::lock_guard l(lfuda_lock); @@ -259,28 +266,32 @@ int LFUDAPolicy::eviction(const DoutPrefixProvider* dpp, uint64_t size, optional auto it = entries_map.find(key); if (it == entries_map.end()) { delete victim; - return -1; + return -ENOENT; } - int avgWeight = (get_local_weight_sum(y) / entries_map.size()); + int avgWeight = get_local_weight_sum(y); if (avgWeight < 0) { delete victim; - return -1; + return avgWeight; } + avgWeight /= entries_map.size(); + if (victim->hostsList.size() == 1 && victim->hostsList[0] == dir->cct->_conf->rgw_local_cache_address) { /* Last copy */ if (victim->globalWeight) { it->second->localWeight += victim->globalWeight; (*it->second->handle)->localWeight = it->second->localWeight; entries_heap.increase(it->second->handle); - if (cacheDriver->set_attr(dpp, key, "user.rgw.localWeight", std::to_string(it->second->localWeight), y) < 0) - return -1; + if (int ret = cacheDriver->set_attr(dpp, key, "user.rgw.localWeight", std::to_string(it->second->localWeight), y) < 0) { + delete victim; + return ret; + } victim->globalWeight = 0; - if (dir->update_field(victim, "globalWeight", std::to_string(victim->globalWeight), y) < 0) { + if (int ret = dir->update_field(victim, "globalWeight", std::to_string(victim->globalWeight), y) < 0) { delete victim; - return -1; + return ret; } } @@ -291,31 +302,31 @@ int LFUDAPolicy::eviction(const DoutPrefixProvider* dpp, uint64_t size, optional } victim->globalWeight += it->second->localWeight; - if (dir->update_field(victim, "globalWeight", std::to_string(victim->globalWeight), y) < 0) { + if (int ret = dir->update_field(victim, "globalWeight", std::to_string(victim->globalWeight), y) < 0) { delete victim; - return -1; + return ret; } - if (dir->remove_host(victim, dir->cct->_conf->rgw_local_cache_address, y) < 0) { + if (int ret = dir->remove_host(victim, dir->cct->_conf->rgw_local_cache_address, y) < 0) { delete victim; - return -1; + return ret; } delete victim; - if (cacheDriver->del(dpp, key, y) < 0) - return -1; + if (int ret = cacheDriver->del(dpp, key, y) < 0) + return ret; ldpp_dout(dpp, 10) << "LFUDAPolicy::" << __func__ << "(): Block " << key << " has been evicted." << dendl; int weight = (avgWeight * entries_map.size()) - it->second->localWeight; - if (set_local_weight_sum((weight > 0) ? weight : 0, y) < 0) - return -1; + if (int ret = set_local_weight_sum((weight > 0) ? weight : 0, y) < 0) + return ret; int age = get_age(y); age = std::max(it->second->localWeight, age); - if (set_age(age, y) < 0) - return -1; + if (int ret = set_age(age, y) < 0) + return ret; erase(dpp, key, y); freeSpace = cacheDriver->get_free_space(dpp); diff --git a/src/rgw/driver/d4n/d4n_policy.h b/src/rgw/driver/d4n/d4n_policy.h index 0339e2b62d627..a9181bdb24fd2 100644 --- a/src/rgw/driver/d4n/d4n_policy.h +++ b/src/rgw/driver/d4n/d4n_policy.h @@ -5,8 +5,6 @@ #include "rgw_sal_d4n.h" #include "rgw_cache_driver.h" -#define dout_subsys ceph_subsys_rgw - namespace rgw::sal { class D4NFilterObject; } diff --git a/src/rgw/driver/d4n/rgw_sal_d4n.cc b/src/rgw/driver/d4n/rgw_sal_d4n.cc index 13e18d7a18b6b..4c2508553c6df 100644 --- a/src/rgw/driver/d4n/rgw_sal_d4n.cc +++ b/src/rgw/driver/d4n/rgw_sal_d4n.cc @@ -409,7 +409,6 @@ int D4NFilterObject::D4NFilterReadOp::prepare(optional_yield y, const DoutPrefix attrs.erase(attr.first); } else { ldpp_dout(dpp, 20) << "D4NFilterObject::D4NFilterReadOp::" << __func__ << "(): Unexpected attribute; not locally set." << dendl; - attrs.erase(attr.first); } } user->set_info(quota_info); diff --git a/src/rgw/rgw_aio.h b/src/rgw/rgw_aio.h index 035758e42da7c..0b4ce71ca3f66 100644 --- a/src/rgw/rgw_aio.h +++ b/src/rgw/rgw_aio.h @@ -103,9 +103,6 @@ class Aio { optional_yield y, jspan_context *trace_ctx = nullptr); static OpFunc d3n_cache_op(const DoutPrefixProvider *dpp, optional_yield y, off_t read_ofs, off_t read_len, std::string& location); - - static OpFunc cache_read_op(const DoutPrefixProvider *dpp, optional_yield y, rgw::cache::CacheDriver* cache_driver, - off_t read_ofs, off_t read_len, const std::string& key); }; } // namespace rgw diff --git a/src/rgw/rgw_process_env.h b/src/rgw/rgw_process_env.h index 5ad87d23e8f29..710340f0a259b 100644 --- a/src/rgw/rgw_process_env.h +++ b/src/rgw/rgw_process_env.h @@ -4,8 +4,6 @@ #pragma once #include -#include "rgw_sal.h" -#include "rgw_auth.h" class ActiveRateLimiter; class OpsLogSink; diff --git a/src/rgw/rgw_redis_driver.cc b/src/rgw/rgw_redis_driver.cc index 8158c595a6e2c..caad53a24262d 100644 --- a/src/rgw/rgw_redis_driver.cc +++ b/src/rgw/rgw_redis_driver.cc @@ -2,8 +2,9 @@ #include #include -#include "rgw_redis_driver.h" +#include "common/dout.h" #include "common/async/blocked_completion.h" +#include "rgw_redis_driver.h" namespace rgw { namespace cache { @@ -100,11 +101,12 @@ int RedisDriver::put(const DoutPrefixProvider* dpp, const std::string& key, cons redis_exec(conn, ec, req, resp, y); - if (std::get<0>(resp).value() != "OK" || ec) { - return -1; + if (ec) { + return -ec.value(); } - } catch(std::exception &e) { - return -1; + } catch (std::exception &e) { + ldpp_dout(dpp, 10) << "RedisDriver::" << __func__ << "(): ERROR: " << e.what() << dendl; + return -EINVAL; } this->free_space -= bl.length(); @@ -124,8 +126,9 @@ int RedisDriver::get(const DoutPrefixProvider* dpp, const std::string& key, off_ redis_exec(conn, ec, req, resp, y); - if (ec) - return -1; + if (ec) { + return -ec.value(); + } for (auto const& it : std::get<0>(resp).value()) { if (it.first == "data") { @@ -137,8 +140,9 @@ int RedisDriver::get(const DoutPrefixProvider* dpp, const std::string& key, off_ bl_value.clear(); } } - } catch(std::exception &e) { - return -1; + } catch (std::exception &e) { + ldpp_dout(dpp, 10) << "RedisDriver::" << __func__ << "(): ERROR: " << e.what() << dendl; + return -EINVAL; } return 0; @@ -156,10 +160,12 @@ int RedisDriver::del(const DoutPrefixProvider* dpp, const std::string& key, opti redis_exec(conn, ec, req, resp, y); - if (ec) - return -1; - } catch(std::exception &e) { - return -1; + if (ec) { + return -ec.value(); + } + } catch (std::exception &e) { + ldpp_dout(dpp, 10) << "RedisDriver::" << __func__ << "(): ERROR: " << e.what() << dendl; + return -EINVAL; } if (std::get<0>(resp).value()) { @@ -173,10 +179,12 @@ int RedisDriver::del(const DoutPrefixProvider* dpp, const std::string& key, opti redis_exec(conn, ec, req, data, y); - if (ec) - return -1; - } catch(std::exception &e) { - return -1; + if (ec) { + return -ec.value(); + } + } catch (std::exception &e) { + ldpp_dout(dpp, 10) << "RedisDriver::" << __func__ << "(): ERROR: " << e.what() << dendl; + return -EINVAL; } try { @@ -186,10 +194,14 @@ int RedisDriver::del(const DoutPrefixProvider* dpp, const std::string& key, opti redis_exec(conn, ec, req, ret, y); - if (!std::get<0>(ret).value() || ec) - return -1; - } catch(std::exception &e) { - return -1; + if (!std::get<0>(ret).value()) { + return -ENOENT; + } else if (ec) { + return -ec.value(); + } + } catch (std::exception &e) { + ldpp_dout(dpp, 10) << "RedisDriver::" << __func__ << "(): ERROR: " << e.what() << dendl; + return -EINVAL; } this->free_space += std::get<0>(data).value().length(); @@ -211,15 +223,17 @@ int RedisDriver::append_data(const DoutPrefixProvider* dpp, const::std::string& redis_exec(conn, ec, req, exists, y); - if (ec) - return -1; - } catch(std::exception &e) { - return -1; + if (ec) { + return -ec.value(); + } + } catch (std::exception &e) { + ldpp_dout(dpp, 10) << "RedisDriver::" << __func__ << "(): ERROR: " << e.what() << dendl; + return -EINVAL; } if (!std::get<0>(exists).value()) { ldpp_dout(dpp, 10) << "RedisDriver::" << __func__ << "(): Data field was not found." << dendl; - return -1; + return -ENOENT; } try { @@ -230,12 +244,14 @@ int RedisDriver::append_data(const DoutPrefixProvider* dpp, const::std::string& redis_exec(conn, ec, req, resp, y); - if (ec) - return -1; + if (ec) { + return -ec.value(); + } value = std::get<0>(resp).value(); - } catch(std::exception &e) { - return -1; + } catch (std::exception &e) { + ldpp_dout(dpp, 10) << "RedisDriver::" << __func__ << "(): ERROR: " << e.what() << dendl; + return -EINVAL; } try { @@ -249,11 +265,12 @@ int RedisDriver::append_data(const DoutPrefixProvider* dpp, const::std::string& redis_exec(conn, ec, req, resp, y); - if (std::get<0>(resp).value() != "OK" || ec) { - return -1; + if (ec) { + return -ec.value(); } - } catch(std::exception &e) { - return -1; + } catch (std::exception &e) { + ldpp_dout(dpp, 10) << "RedisDriver::" << __func__ << "(): ERROR: " << e.what() << dendl; + return -EINVAL; } this->free_space -= bl_data.length(); @@ -272,10 +289,12 @@ int RedisDriver::delete_data(const DoutPrefixProvider* dpp, const::std::string& redis_exec(conn, ec, req, resp, y); - if (ec) - return -1; - } catch(std::exception &e) { - return -1; + if (ec) { + return -ec.value(); + } + } catch (std::exception &e) { + ldpp_dout(dpp, 10) << "RedisDriver::" << __func__ << "(): ERROR: " << e.what() << dendl; + return -EINVAL; } if (std::get<0>(resp).value()) { @@ -290,10 +309,11 @@ int RedisDriver::delete_data(const DoutPrefixProvider* dpp, const::std::string& redis_exec(conn, ec, req, data, y); if (ec) { - return -1; + return -ec.value(); } - } catch(std::exception &e) { - return -1; + } catch (std::exception &e) { + ldpp_dout(dpp, 10) << "RedisDriver::" << __func__ << "(): ERROR: " << e.what() << dendl; + return -EINVAL; } try { @@ -303,11 +323,14 @@ int RedisDriver::delete_data(const DoutPrefixProvider* dpp, const::std::string& redis_exec(conn, ec, req, ret, y); - if (!std::get<0>(ret).value() || ec) { - return -1; + if (!std::get<0>(ret).value()) { + return -ENOENT; + } else if (ec) { + return -ec.value(); } - } catch(std::exception &e) { - return -1; + } catch (std::exception &e) { + ldpp_dout(dpp, 10) << "RedisDriver::" << __func__ << "(): ERROR: " << e.what() << dendl; + return -EINVAL; } this->free_space += std::get<0>(data).value().length(); @@ -328,8 +351,9 @@ int RedisDriver::get_attrs(const DoutPrefixProvider* dpp, const std::string& key redis_exec(conn, ec, req, resp, y); - if (std::get<0>(resp).value().empty() || ec) - return -1; + if (ec) { + return -ec.value(); + } for (auto const& it : std::get<0>(resp).value()) { if (it.first != "data") { @@ -339,8 +363,9 @@ int RedisDriver::get_attrs(const DoutPrefixProvider* dpp, const std::string& key bl_value.clear(); } } - } catch(std::exception &e) { - return -1; + } catch (std::exception &e) { + ldpp_dout(dpp, 10) << "RedisDriver::" << __func__ << "(): ERROR: " << e.what() << dendl; + return -EINVAL; } return 0; @@ -349,7 +374,7 @@ int RedisDriver::get_attrs(const DoutPrefixProvider* dpp, const std::string& key int RedisDriver::set_attrs(const DoutPrefixProvider* dpp, const std::string& key, const rgw::sal::Attrs& attrs, optional_yield y) { if (attrs.empty()) - return -1; + return -EINVAL; std::string entry = partition_info.location + key; @@ -365,11 +390,12 @@ int RedisDriver::set_attrs(const DoutPrefixProvider* dpp, const std::string& key redis_exec(conn, ec, req, resp, y); - if (std::get<0>(resp).value() != "OK" || ec) { - return -1; + if (ec) { + return -ec.value(); } - } catch(std::exception &e) { - return -1; + } catch (std::exception &e) { + ldpp_dout(dpp, 10) << "RedisDriver::" << __func__ << "(): ERROR: " << e.what() << dendl; + return -EINVAL; } return 0; @@ -389,11 +415,12 @@ int RedisDriver::update_attrs(const DoutPrefixProvider* dpp, const std::string& redis_exec(conn, ec, req, resp, y); - if (std::get<0>(resp).value() != "OK" || ec) { - return -1; + if (ec) { + return -ec.value(); } - } catch(std::exception &e) { - return -1; + } catch (std::exception &e) { + ldpp_dout(dpp, 10) << "RedisDriver::" << __func__ << "(): ERROR: " << e.what() << dendl; + return -EINVAL; } return 0; @@ -413,12 +440,16 @@ int RedisDriver::delete_attrs(const DoutPrefixProvider* dpp, const std::string& redis_exec(conn, ec, req, resp, y); - if (ec) - return -1; + if (!std::get<0>(resp).value()) { + return -ENOENT; + } else if (ec) { + return -ec.value(); + } return std::get<0>(resp).value(); - } catch(std::exception &e) { - return -1; + } catch (std::exception &e) { + ldpp_dout(dpp, 10) << "RedisDriver::" << __func__ << "(): ERROR: " << e.what() << dendl; + return -EINVAL; } } @@ -427,6 +458,7 @@ int RedisDriver::get_attr(const DoutPrefixProvider* dpp, const std::string& key, std::string entry = partition_info.location + key; response value; response resp; + attr_val = ""; /* Ensure field was set */ try { @@ -437,18 +469,37 @@ int RedisDriver::get_attr(const DoutPrefixProvider* dpp, const std::string& key, redis_exec(conn, ec, req, resp, y); if (ec) { - attr_val = ""; - return -1; + return -ec.value(); + } + } catch (std::exception &e) { + ldpp_dout(dpp, 10) << "RedisDriver::" << __func__ << "(): ERROR: " << e.what() << dendl; + return -EINVAL; + } + + if (!std::get<0>(resp).value()) { + ldpp_dout(dpp, 10) << "RedisDriver::" << __func__ << "(): Attribute was not found." << dendl; + return -ENOENT; + } + + /* Retrieve existing value from cache */ + try { + boost::system::error_code ec; + request req; + req.push("HGET", entry, attr_name); + + redis_exec(conn, ec, req, value, y); + + if (ec) { + return -ec.value(); } - } catch(std::exception &e) { - attr_val = ""; - return -1; + } catch (std::exception &e) { + ldpp_dout(dpp, 10) << "RedisDriver::" << __func__ << "(): ERROR: " << e.what() << dendl; + return -EINVAL; } if (!std::get<0>(resp).value()) { ldpp_dout(dpp, 10) << "RedisDriver::" << __func__ << "(): Attribute was not found." << dendl; - attr_val = ""; - return -1; + return -ENOENT; } /* Retrieve existing value from cache */ @@ -460,12 +511,11 @@ int RedisDriver::get_attr(const DoutPrefixProvider* dpp, const std::string& key, redis_exec(conn, ec, req, value, y); if (ec) { - attr_val = ""; - return -1; + return -ec.value(); } - } catch(std::exception &e) { - attr_val = ""; - return -1; + } catch (std::exception &e) { + ldpp_dout(dpp, 10) << "RedisDriver::" << __func__ << "(): ERROR: " << e.what() << dendl; + return -EINVAL; } attr_val = std::get<0>(value).value(); @@ -485,10 +535,12 @@ int RedisDriver::set_attr(const DoutPrefixProvider* dpp, const std::string& key, redis_exec(conn, ec, req, resp, y); - if (ec) - return {}; - } catch(std::exception &e) { - return -1; + if (ec) { + return -ec.value(); + } + } catch (std::exception &e) { + ldpp_dout(dpp, 10) << "RedisDriver::" << __func__ << "(): ERROR: " << e.what() << dendl; + return -EINVAL; } return std::get<0>(resp).value(); diff --git a/src/rgw/rgw_redis_driver.h b/src/rgw/rgw_redis_driver.h index bf78496e573e8..3abfde71729ad 100644 --- a/src/rgw/rgw_redis_driver.h +++ b/src/rgw/rgw_redis_driver.h @@ -7,10 +7,7 @@ #include "rgw_common.h" #include "rgw_cache_driver.h" -#define dout_subsys ceph_subsys_rgw -#define dout_context g_ceph_context - -namespace rgw { namespace cache { +namespace rgw { namespace cache { namespace net = boost::asio; using boost::redis::config; diff --git a/src/test/rgw/test_d4n_directory.cc b/src/test/rgw/test_d4n_directory.cc index 8d00a2b023942..7328212dcffa6 100644 --- a/src/test/rgw/test_d4n_directory.cc +++ b/src/test/rgw/test_d4n_directory.cc @@ -7,6 +7,8 @@ #include "rgw_auth_registry.h" #include "driver/d4n/d4n_directory.h" +#define dout_subsys ceph_subsys_rgw + namespace net = boost::asio; using boost::redis::config; using boost::redis::connection; diff --git a/src/test/rgw/test_d4n_policy.cc b/src/test/rgw/test_d4n_policy.cc index ef9a2474fa592..659fcf93e06a0 100644 --- a/src/test/rgw/test_d4n_policy.cc +++ b/src/test/rgw/test_d4n_policy.cc @@ -8,6 +8,8 @@ #include "rgw_auth_registry.h" #include "driver/d4n/d4n_policy.h" +#define dout_subsys ceph_subsys_rgw + namespace net = boost::asio; using boost::redis::config; using boost::redis::connection; diff --git a/src/test/rgw/test_redis_driver.cc b/src/test/rgw/test_redis_driver.cc index e6a9c7c67fc2b..f031a939285fb 100644 --- a/src/test/rgw/test_redis_driver.cc +++ b/src/test/rgw/test_redis_driver.cc @@ -8,6 +8,8 @@ #include "rgw_aio_throttle.h" #include "rgw_redis_driver.h" +#define dout_subsys ceph_subsys_rgw + namespace net = boost::asio; using boost::redis::config; using boost::redis::connection; -- 2.39.5