From: Yuval Lifshitz Date: Tue, 3 Jun 2025 10:30:46 +0000 (+0000) Subject: rgw/logging: return the last object name that was actually comitted X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=efe4473000e1a4510549c9e39ce984e8f3bb1db6;p=ceph.git rgw/logging: return the last object name that was actually comitted when comitting a pending object that was never created we should not reply the object name as the name of the comitted object. instead, we should return the name of the object that was actuaslly comitted. Fixes: https://tracker.ceph.com/issues/71219 Signed-off-by: Yuval Lifshitz --- diff --git a/src/rgw/driver/rados/rgw_sal_rados.cc b/src/rgw/driver/rados/rgw_sal_rados.cc index 05fdc7cf7015..99bb6b071369 100644 --- a/src/rgw/driver/rados/rgw_sal_rados.cc +++ b/src/rgw/driver/rados/rgw_sal_rados.cc @@ -1071,6 +1071,56 @@ int RadosBucket::remove_topics(RGWObjVersionTracker* objv_tracker, objv_tracker, y); } + +#define RGW_ATTR_COMMITTED_LOGGING_OBJ RGW_ATTR_PREFIX "committed-logging-obj" + +int get_committed_logging_object(RadosStore* store, + const std::string& obj_name_oid, + const rgw_pool& data_pool, + optional_yield y, + const DoutPrefixProvider *dpp, + std::string& last_committed) { + librados::IoCtx io_ctx; + if (const int ret = rgw_init_ioctx(dpp, store->getRados()->get_rados_handle(), data_pool, io_ctx); ret < 0) { + return -EIO; + } + if (!io_ctx.is_valid()) { + return -EIO; + } + bufferlist bl; + librados::ObjectReadOperation op; + int rval; + op.getxattr(RGW_ATTR_COMMITTED_LOGGING_OBJ, &bl, &rval); + if (const int ret = rgw_rados_operate(dpp, io_ctx, obj_name_oid, std::move(op), nullptr, y); ret < 0) { + return ret; + } + last_committed = bl.to_str(); + return 0; +} + +int set_committed_logging_object(RadosStore* store, + const std::string& obj_name_oid, + const rgw_pool& data_pool, + optional_yield y, + const DoutPrefixProvider *dpp, + const std::string& last_committed) { + librados::IoCtx io_ctx; + if (const int ret = rgw_init_ioctx(dpp, store->getRados()->get_rados_handle(), data_pool, io_ctx); ret < 0) { + ldpp_dout(dpp, 1) << "ERROR: failed to get IO context when setting last committed logging object name from data pool:" << data_pool.to_str() << dendl; + return -EIO; + } + if (!io_ctx.is_valid()) { + ldpp_dout(dpp, 1) << "ERROR: invalid IO context when setting last committed logging object name" << dendl; + return -EIO; + } + + bufferlist bl; + bl.append(last_committed); + librados::ObjectWriteOperation op; + op.setxattr(RGW_ATTR_COMMITTED_LOGGING_OBJ, std::move(bl)); + return rgw_rados_operate(dpp, io_ctx, obj_name_oid, std::move(op), y); +} + int RadosBucket::get_logging_object_name(std::string& obj_name, const std::string& prefix, optional_yield y, @@ -1129,7 +1179,7 @@ int RadosBucket::set_logging_object_name(const std::string& obj_name, objv_tracker, ceph::real_time::clock::now(), y, - nullptr); + no_change_attrs()); if (ret == -EEXIST) { ldpp_dout(dpp, 20) << "INFO: race detected in initializing '" << obj_name_oid << "' with logging object name:'" << obj_name << "'. ret = " << ret << dendl; } else if (ret == -ECANCELED) { @@ -1183,7 +1233,11 @@ int RadosBucket::remove_logging_object(const std::string& obj_name, optional_yie y); } -int RadosBucket::commit_logging_object(const std::string& obj_name, optional_yield y, const DoutPrefixProvider *dpp) { +int RadosBucket::commit_logging_object(const std::string& obj_name, + optional_yield y, + const DoutPrefixProvider *dpp, + const std::string& prefix, + std::string* last_committed) { rgw_pool data_pool; const rgw_obj head_obj{get_key(), obj_name}; const auto placement_rule = get_placement_rule(); @@ -1193,6 +1247,24 @@ int RadosBucket::commit_logging_object(const std::string& obj_name, optional_yie "' when committing logging object" << dendl; return -EIO; } + const auto obj_name_oid = bucketlogging::object_name_oid(this, prefix); + if (last_committed) { + if (const int ret = get_committed_logging_object(store, + obj_name_oid, + data_pool, + y, + dpp, + *last_committed); ret < 0) { + if (ret != -ENODATA) { + ldpp_dout(dpp, 1) << "ERROR: failed to get last committed logging object name from bucket '" << get_key() << + "' when committing logging object. ret = " << ret << dendl; + return ret; + } + ldpp_dout(dpp, 5) << "WARNING: no last committed logging object name found for bucket '" << get_key() << + "' when committing logging object" << dendl; + last_committed->clear(); + } + } const auto temp_obj_name = to_temp_object_name(this, obj_name); std::map obj_attrs; @@ -1207,13 +1279,14 @@ int RadosBucket::commit_logging_object(const std::string& obj_name, optional_yie y, dpp, &obj_attrs, - nullptr); ret < 0 && ret != -ENOENT) { - ldpp_dout(dpp, 1) << "ERROR: failed to read logging data when committing object '" << temp_obj_name - << ". error: " << ret << dendl; + nullptr); ret < 0) { + if (ret == -ENOENT) { + ldpp_dout(dpp, 5) << "WARNING: temporary logging object '" << temp_obj_name << "' does not exists" << dendl; + } else { + ldpp_dout(dpp, 1) << "ERROR: failed to read logging data when committing object '" << temp_obj_name + << ". error: " << ret << dendl; + } return ret; - } else if (ret == -ENOENT) { - ldpp_dout(dpp, 1) << "WARNING: temporary logging object '" << temp_obj_name << "' does not exists" << dendl; - return 0; } uint64_t size = bl_data.length(); @@ -1279,9 +1352,24 @@ int RadosBucket::commit_logging_object(const std::string& obj_name, optional_yie "' to bucket '" << get_key() <<"'. error: " << ret << dendl; return ret; } + ldpp_dout(dpp, 20) << "INFO: committed logging object '" << temp_obj_name << "' with size of " << size << " bytes, to bucket '" << get_key() << "' as '" << obj_name << "'" << dendl; + + if (const int ret = set_committed_logging_object(store, + obj_name_oid, + data_pool, + y, + dpp, + obj_name); ret < 0) { + ldpp_dout(dpp, 5) << "WARNING: object was committed, but we failed to set last committed logging object name. ret = " << ret << dendl; + } else { + ldpp_dout(dpp, 20) << "INFO: last committed logging object name was set to '" << obj_name << "'" << dendl; + } + if (last_committed) { + *last_committed = obj_name; + } return 0; } diff --git a/src/rgw/driver/rados/rgw_sal_rados.h b/src/rgw/driver/rados/rgw_sal_rados.h index 68db1b8bf096..66c73a1199f0 100644 --- a/src/rgw/driver/rados/rgw_sal_rados.h +++ b/src/rgw/driver/rados/rgw_sal_rados.h @@ -803,7 +803,7 @@ class RadosBucket : public StoreBucket { optional_yield y, const DoutPrefixProvider *dpp, RGWObjVersionTracker* objv_tracker) override; - int commit_logging_object(const std::string& obj_name, optional_yield y, const DoutPrefixProvider *dpp) override; + int commit_logging_object(const std::string& obj_name, optional_yield y, const DoutPrefixProvider *dpp, const std::string& prefix, std::string* last_committed) override; int remove_logging_object(const std::string& obj_name, optional_yield y, const DoutPrefixProvider *dpp) override; int write_logging_object(const std::string& obj_name, const std::string& record, optional_yield y, const DoutPrefixProvider *dpp, bool async_completion) override; diff --git a/src/rgw/radosgw-admin/radosgw-admin.cc b/src/rgw/radosgw-admin/radosgw-admin.cc index 43b023490395..ed292bb49b83 100644 --- a/src/rgw/radosgw-admin/radosgw-admin.cc +++ b/src/rgw/radosgw-admin/radosgw-admin.cc @@ -7832,12 +7832,18 @@ int main(int argc, const char **argv) cerr << "ERROR: failed to get pending logging object name from target bucket '" << configuration.target_bucket << "'" << std::endl; return -ret; } - const auto old_obj = obj_name; + std::string old_obj; const auto region = driver->get_zone()->get_zonegroup().get_api_name(); - ret = rgw::bucketlogging::rollover_logging_object(configuration, target_bucket, obj_name, dpp(), region, bucket, null_yield, true, &objv_tracker); + ret = rgw::bucketlogging::rollover_logging_object(configuration, target_bucket, obj_name, dpp(), region, bucket, null_yield, true, &objv_tracker, &old_obj); if (ret < 0) { - cerr << "ERROR: failed to flush pending logging object '" << old_obj - << "' to target bucket '" << configuration.target_bucket << "'" << std::endl; + if (ret == -ENOENT) { + cerr << "WARNING: no pending logging object '" << obj_name << "'. nothing to flush"; + ret = 0; + } else { + cerr << "ERROR: failed flush pending logging object '" << obj_name << "'"; + } + cerr << " to target bucket '" << configuration.target_bucket << "'. " + << " last committed object is '" << old_obj << "'" << std::endl; return -ret; } cout << "flushed pending logging object '" << old_obj diff --git a/src/rgw/rgw_bucket_logging.cc b/src/rgw/rgw_bucket_logging.cc index 94ba1187d443..b1f159762452 100644 --- a/src/rgw/rgw_bucket_logging.cc +++ b/src/rgw/rgw_bucket_logging.cc @@ -301,7 +301,8 @@ int commit_logging_object(const configuration& conf, const DoutPrefixProvider *dpp, rgw::sal::Driver* driver, const std::string& tenant_name, - optional_yield y) { + optional_yield y, + std::string* last_committed) { std::string target_bucket_name; std::string target_tenant_name; int ret = rgw_parse_url_bucket(conf.target_bucket, tenant_name, target_tenant_name, target_bucket_name); @@ -319,20 +320,21 @@ int commit_logging_object(const configuration& conf, << ret << dendl; return ret; } - return commit_logging_object(conf, target_bucket, dpp, y); + return commit_logging_object(conf, target_bucket, dpp, y, last_committed); } int commit_logging_object(const configuration& conf, const std::unique_ptr& target_bucket, const DoutPrefixProvider *dpp, - optional_yield y) { + optional_yield y, + std::string* last_committed) { std::string obj_name; if (const int ret = target_bucket->get_logging_object_name(obj_name, conf.target_prefix, y, dpp, nullptr); ret < 0) { ldpp_dout(dpp, 1) << "ERROR: failed to get name of logging object of bucket '" << target_bucket->get_key() << "'. ret = " << ret << dendl; return ret; } - if (const int ret = target_bucket->commit_logging_object(obj_name, y, dpp); ret <0 ) { + if (const int ret = target_bucket->commit_logging_object(obj_name, y, dpp, conf.target_prefix, last_committed); ret < 0) { ldpp_dout(dpp, 1) << "ERROR: failed to commit logging object '" << obj_name << "' of bucket '" << target_bucket->get_key() << "'. ret = " << ret << dendl; return ret; @@ -348,7 +350,8 @@ int rollover_logging_object(const configuration& conf, const std::unique_ptr& source_bucket, optional_yield y, bool must_commit, - RGWObjVersionTracker* objv_tracker) { + RGWObjVersionTracker* objv_tracker, + std::string* last_committed) { std::string target_bucket_name; std::string target_tenant_name; std::ignore = rgw_parse_url_bucket(conf.target_bucket, target_bucket->get_tenant(), target_tenant_name, target_bucket_name); @@ -368,7 +371,7 @@ int rollover_logging_object(const configuration& conf, target_bucket->get_key() << "'. ret = " << ret << dendl; return ret; } - if (const int ret = target_bucket->commit_logging_object(old_obj, y, dpp); ret < 0) { + if (const int ret = target_bucket->commit_logging_object(old_obj, y, dpp, conf.target_prefix, last_committed); ret < 0) { if (must_commit) { return ret; } @@ -476,7 +479,7 @@ int log_record(rgw::sal::Driver* driver, if (ceph::coarse_real_time::clock::now() > time_to_commit) { ldpp_dout(dpp, 20) << "INFO: logging object '" << obj_name << "' exceeded its time, will be committed to bucket '" << target_bucket_id << "'" << dendl; - if (ret = rollover_logging_object(conf, target_bucket, obj_name, dpp, region, s->bucket, y, false, &objv_tracker); ret < 0) { + if (ret = rollover_logging_object(conf, target_bucket, obj_name, dpp, region, s->bucket, y, false, &objv_tracker, nullptr); ret < 0) { return ret; } } else { @@ -622,7 +625,7 @@ int log_record(rgw::sal::Driver* driver, if (ret == -EFBIG) { ldpp_dout(dpp, 5) << "WARNING: logging object '" << obj_name << "' is full, will be committed to bucket '" << target_bucket->get_key() << "'" << dendl; - if (ret = rollover_logging_object(conf, target_bucket, obj_name, dpp, region, s->bucket, y, true, nullptr); ret < 0 ) { + if (ret = rollover_logging_object(conf, target_bucket, obj_name, dpp, region, s->bucket, y, true, nullptr, nullptr); ret < 0 ) { return ret; } if (ret = target_bucket->write_logging_object(obj_name, @@ -853,7 +856,7 @@ int source_bucket_cleanup(const DoutPrefixProvider* dpp, return 0; } const auto& info = bucket->get_info(); - if (const int ret = commit_logging_object(*conf, dpp, driver, info.bucket.tenant, y); ret < 0) { + if (const int ret = commit_logging_object(*conf, dpp, driver, info.bucket.tenant, y, nullptr); ret < 0) { ldpp_dout(dpp, 5) << "WARNING: could not commit pending logging object of bucket '" << bucket->get_key() << "' during cleanup. ret = " << ret << dendl; } else { diff --git a/src/rgw/rgw_bucket_logging.h b/src/rgw/rgw_bucket_logging.h index 0b7a9c75423b..f27f296f53e0 100644 --- a/src/rgw/rgw_bucket_logging.h +++ b/src/rgw/rgw_bucket_logging.h @@ -172,6 +172,7 @@ int log_record(rgw::sal::Driver* driver, // commit the pending log objec to the log bucket // and create a new pending log object // if "must_commit" is "false" the function will return success even if the pending log object was not committed +// if "last_committed" is not null, it will be set to the name of the last committed object int rollover_logging_object(const configuration& conf, const std::unique_ptr& bucket, std::string& obj_name, @@ -180,24 +181,29 @@ int rollover_logging_object(const configuration& conf, const std::unique_ptr& source_bucket, optional_yield y, bool must_commit, - RGWObjVersionTracker* objv_tracker); + RGWObjVersionTracker* objv_tracker, + std::string* last_committed); // commit the pending log object to the log bucket // use this for cleanup, when new pending object is not needed // and target bucket is known +// if "last_committed" is not null, it will be set to the name of the last committed object int commit_logging_object(const configuration& conf, const std::unique_ptr& target_bucket, const DoutPrefixProvider *dpp, - optional_yield y); + optional_yield y, + std::string* last_committed); // commit the pending log object to the log bucket // use this for cleanup, when new pending object is not needed // and target bucket shoud be loaded based on the configuration +// if "last_committed" is not null, it will be set to the name of the last committed object int commit_logging_object(const configuration& conf, const DoutPrefixProvider *dpp, rgw::sal::Driver* driver, const std::string& tenant_name, - optional_yield y); + optional_yield y, + std::string* last_committed); // return the oid of the object holding the name of the temporary logging object // bucket - log bucket diff --git a/src/rgw/rgw_rest_bucket_logging.cc b/src/rgw/rgw_rest_bucket_logging.cc index 2d0533088128..09c1a2200524 100644 --- a/src/rgw/rgw_rest_bucket_logging.cc +++ b/src/rgw/rgw_rest_bucket_logging.cc @@ -304,7 +304,7 @@ class RGWPutBucketLoggingOp : public RGWDefaultResponseOp { } } else if (*old_conf != configuration) { // conf changed - do cleanup - if (const auto ret = commit_logging_object(*old_conf, target_bucket, this, y); ret < 0) { + if (const auto ret = commit_logging_object(*old_conf, target_bucket, this, y, nullptr); ret < 0) { ldpp_dout(this, 1) << "WARNING: could not commit pending logging object when updating logging configuration of bucket '" << src_bucket->get_key() << "', ret = " << ret << dendl; } else { @@ -385,12 +385,19 @@ class RGWPostBucketLoggingOp : public RGWDefaultResponseOp { ldpp_dout(this, 1) << "ERROR: failed to get pending logging object name from target bucket '" << target_bucket_id << "'" << dendl; return; } - old_obj = obj_name; const auto region = driver->get_zone()->get_zonegroup().get_api_name(); - op_ret = rgw::bucketlogging::rollover_logging_object(configuration, target_bucket, obj_name, this, region, source_bucket, null_yield, true, &objv_tracker); + op_ret = rgw::bucketlogging::rollover_logging_object(configuration, target_bucket, obj_name, this, region, source_bucket, null_yield, true, &objv_tracker, &old_obj); if (op_ret < 0) { - ldpp_dout(this, 1) << "ERROR: failed to flush pending logging object '" << old_obj - << "' to target bucket '" << target_bucket_id << "'" << dendl; + if (op_ret == -ENOENT) { + ldpp_dout(this, 5) << "WARNING: no pending logging object '" << obj_name << "'. nothing to flush" + << " to target bucket '" << target_bucket_id << "'. " + << " last committed object is '" << old_obj << "'" << dendl; + op_ret = 0; + } else { + ldpp_dout(this, 1) << "ERROR: failed flush pending logging object '" << obj_name << "'" + << " to target bucket '" << target_bucket_id << "'. " + << " last committed object is '" << old_obj << "'" << dendl; + } return; } ldpp_dout(this, 20) << "INFO: flushed pending logging object '" << old_obj diff --git a/src/rgw/rgw_sal.h b/src/rgw/rgw_sal.h index 46ec35654464..a50fc65d7fc6 100644 --- a/src/rgw/rgw_sal.h +++ b/src/rgw/rgw_sal.h @@ -1038,8 +1038,10 @@ class Bucket { optional_yield y, const DoutPrefixProvider *dpp, RGWObjVersionTracker* objv_tracker) = 0; - /** Move the pending bucket logging object into the bucket */ - virtual int commit_logging_object(const std::string& obj_name, optional_yield y, const DoutPrefixProvider *dpp) = 0; + /** Move the pending bucket logging object into the bucket + if "last_committed" is not null, it will be set to the name of the last committed object + * */ + virtual int commit_logging_object(const std::string& obj_name, optional_yield y, const DoutPrefixProvider *dpp, const std::string& prefix, std::string* last_committed) = 0; //** Remove the pending bucket logging object */ virtual int remove_logging_object(const std::string& obj_name, optional_yield y, const DoutPrefixProvider *dpp) = 0; /** Write a record to the pending bucket logging object */ diff --git a/src/rgw/rgw_sal_filter.h b/src/rgw/rgw_sal_filter.h index a3a759b9cd3d..4b0b97fe2208 100644 --- a/src/rgw/rgw_sal_filter.h +++ b/src/rgw/rgw_sal_filter.h @@ -693,8 +693,8 @@ public: RGWObjVersionTracker* objv_tracker) override { return next->remove_logging_object_name(prefix, y, dpp, objv_tracker); } - int commit_logging_object(const std::string& obj_name, optional_yield y, const DoutPrefixProvider *dpp)override { - return next->commit_logging_object(obj_name, y, dpp); + int commit_logging_object(const std::string& obj_name, optional_yield y, const DoutPrefixProvider *dpp, const std::string& prefix, std::string* last_committed) override { + return next->commit_logging_object(obj_name, y, dpp, prefix, last_committed); } int remove_logging_object(const std::string& obj_name, optional_yield y, const DoutPrefixProvider *dpp) override { return next->remove_logging_object(obj_name, y, dpp); diff --git a/src/rgw/rgw_sal_store.h b/src/rgw/rgw_sal_store.h index 45ee69617884..da98eb41f997 100644 --- a/src/rgw/rgw_sal_store.h +++ b/src/rgw/rgw_sal_store.h @@ -266,7 +266,7 @@ class StoreBucket : public Bucket { optional_yield y, const DoutPrefixProvider *dpp, RGWObjVersionTracker* objv_tracker) override { return 0; } - int commit_logging_object(const std::string& obj_name, optional_yield y, const DoutPrefixProvider *dpp) override { return 0; } + int commit_logging_object(const std::string& obj_name, optional_yield y, const DoutPrefixProvider *dpp, const std::string& prefix, std::string* last_committed) override { return 0; } int remove_logging_object(const std::string& obj_name, optional_yield y, const DoutPrefixProvider *dpp) override { return 0; } int write_logging_object(const std::string& obj_name, const std::string& record, optional_yield y, const DoutPrefixProvider *dpp, bool async_completion) override { return 0;