Skip to content

Commit efe4473

Browse files
yuvalifyuvalif
committed
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 <[email protected]>
1 parent fb2b037 commit efe4473

File tree

9 files changed

+147
-35
lines changed

9 files changed

+147
-35
lines changed

‎src/rgw/driver/rados/rgw_sal_rados.cc‎

Lines changed: 96 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1071,6 +1071,56 @@ int RadosBucket::remove_topics(RGWObjVersionTracker* objv_tracker,
10711071
objv_tracker, y);
10721072
}
10731073

1074+
1075+
#define RGW_ATTR_COMMITTED_LOGGING_OBJ RGW_ATTR_PREFIX "committed-logging-obj"
1076+
1077+
int get_committed_logging_object(RadosStore* store,
1078+
const std::string& obj_name_oid,
1079+
const rgw_pool& data_pool,
1080+
optional_yield y,
1081+
const DoutPrefixProvider *dpp,
1082+
std::string& last_committed) {
1083+
librados::IoCtx io_ctx;
1084+
if (const int ret = rgw_init_ioctx(dpp, store->getRados()->get_rados_handle(), data_pool, io_ctx); ret < 0) {
1085+
return -EIO;
1086+
}
1087+
if (!io_ctx.is_valid()) {
1088+
return -EIO;
1089+
}
1090+
bufferlist bl;
1091+
librados::ObjectReadOperation op;
1092+
int rval;
1093+
op.getxattr(RGW_ATTR_COMMITTED_LOGGING_OBJ, &bl, &rval);
1094+
if (const int ret = rgw_rados_operate(dpp, io_ctx, obj_name_oid, std::move(op), nullptr, y); ret < 0) {
1095+
return ret;
1096+
}
1097+
last_committed = bl.to_str();
1098+
return 0;
1099+
}
1100+
1101+
int set_committed_logging_object(RadosStore* store,
1102+
const std::string& obj_name_oid,
1103+
const rgw_pool& data_pool,
1104+
optional_yield y,
1105+
const DoutPrefixProvider *dpp,
1106+
const std::string& last_committed) {
1107+
librados::IoCtx io_ctx;
1108+
if (const int ret = rgw_init_ioctx(dpp, store->getRados()->get_rados_handle(), data_pool, io_ctx); ret < 0) {
1109+
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;
1110+
return -EIO;
1111+
}
1112+
if (!io_ctx.is_valid()) {
1113+
ldpp_dout(dpp, 1) << "ERROR: invalid IO context when setting last committed logging object name" << dendl;
1114+
return -EIO;
1115+
}
1116+
1117+
bufferlist bl;
1118+
bl.append(last_committed);
1119+
librados::ObjectWriteOperation op;
1120+
op.setxattr(RGW_ATTR_COMMITTED_LOGGING_OBJ, std::move(bl));
1121+
return rgw_rados_operate(dpp, io_ctx, obj_name_oid, std::move(op), y);
1122+
}
1123+
10741124
int RadosBucket::get_logging_object_name(std::string& obj_name,
10751125
const std::string& prefix,
10761126
optional_yield y,
@@ -1129,7 +1179,7 @@ int RadosBucket::set_logging_object_name(const std::string& obj_name,
11291179
objv_tracker,
11301180
ceph::real_time::clock::now(),
11311181
y,
1132-
nullptr);
1182+
no_change_attrs());
11331183
if (ret == -EEXIST) {
11341184
ldpp_dout(dpp, 20) << "INFO: race detected in initializing '" << obj_name_oid << "' with logging object name:'" << obj_name << "'. ret = " << ret << dendl;
11351185
} else if (ret == -ECANCELED) {
@@ -1183,7 +1233,11 @@ int RadosBucket::remove_logging_object(const std::string& obj_name, optional_yie
11831233
y);
11841234
}
11851235

1186-
int RadosBucket::commit_logging_object(const std::string& obj_name, optional_yield y, const DoutPrefixProvider *dpp) {
1236+
int RadosBucket::commit_logging_object(const std::string& obj_name,
1237+
optional_yield y,
1238+
const DoutPrefixProvider *dpp,
1239+
const std::string& prefix,
1240+
std::string* last_committed) {
11871241
rgw_pool data_pool;
11881242
const rgw_obj head_obj{get_key(), obj_name};
11891243
const auto placement_rule = get_placement_rule();
@@ -1193,6 +1247,24 @@ int RadosBucket::commit_logging_object(const std::string& obj_name, optional_yie
11931247
"' when committing logging object" << dendl;
11941248
return -EIO;
11951249
}
1250+
const auto obj_name_oid = bucketlogging::object_name_oid(this, prefix);
1251+
if (last_committed) {
1252+
if (const int ret = get_committed_logging_object(store,
1253+
obj_name_oid,
1254+
data_pool,
1255+
y,
1256+
dpp,
1257+
*last_committed); ret < 0) {
1258+
if (ret != -ENODATA) {
1259+
ldpp_dout(dpp, 1) << "ERROR: failed to get last committed logging object name from bucket '" << get_key() <<
1260+
"' when committing logging object. ret = " << ret << dendl;
1261+
return ret;
1262+
}
1263+
ldpp_dout(dpp, 5) << "WARNING: no last committed logging object name found for bucket '" << get_key() <<
1264+
"' when committing logging object" << dendl;
1265+
last_committed->clear();
1266+
}
1267+
}
11961268

11971269
const auto temp_obj_name = to_temp_object_name(this, obj_name);
11981270
std::map<string, bufferlist> obj_attrs;
@@ -1207,13 +1279,14 @@ int RadosBucket::commit_logging_object(const std::string& obj_name, optional_yie
12071279
y,
12081280
dpp,
12091281
&obj_attrs,
1210-
nullptr); ret < 0 && ret != -ENOENT) {
1211-
ldpp_dout(dpp, 1) << "ERROR: failed to read logging data when committing object '" << temp_obj_name
1212-
<< ". error: " << ret << dendl;
1282+
nullptr); ret < 0) {
1283+
if (ret == -ENOENT) {
1284+
ldpp_dout(dpp, 5) << "WARNING: temporary logging object '" << temp_obj_name << "' does not exists" << dendl;
1285+
} else {
1286+
ldpp_dout(dpp, 1) << "ERROR: failed to read logging data when committing object '" << temp_obj_name
1287+
<< ". error: " << ret << dendl;
1288+
}
12131289
return ret;
1214-
} else if (ret == -ENOENT) {
1215-
ldpp_dout(dpp, 1) << "WARNING: temporary logging object '" << temp_obj_name << "' does not exists" << dendl;
1216-
return 0;
12171290
}
12181291

12191292
uint64_t size = bl_data.length();
@@ -1279,9 +1352,24 @@ int RadosBucket::commit_logging_object(const std::string& obj_name, optional_yie
12791352
"' to bucket '" << get_key() <<"'. error: " << ret << dendl;
12801353
return ret;
12811354
}
1355+
12821356
ldpp_dout(dpp, 20) << "INFO: committed logging object '" << temp_obj_name <<
12831357
"' with size of " << size << " bytes, to bucket '" << get_key() << "' as '" <<
12841358
obj_name << "'" << dendl;
1359+
1360+
if (const int ret = set_committed_logging_object(store,
1361+
obj_name_oid,
1362+
data_pool,
1363+
y,
1364+
dpp,
1365+
obj_name); ret < 0) {
1366+
ldpp_dout(dpp, 5) << "WARNING: object was committed, but we failed to set last committed logging object name. ret = " << ret << dendl;
1367+
} else {
1368+
ldpp_dout(dpp, 20) << "INFO: last committed logging object name was set to '" << obj_name << "'" << dendl;
1369+
}
1370+
if (last_committed) {
1371+
*last_committed = obj_name;
1372+
}
12851373
return 0;
12861374
}
12871375

‎src/rgw/driver/rados/rgw_sal_rados.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -803,7 +803,7 @@ class RadosBucket : public StoreBucket {
803803
optional_yield y,
804804
const DoutPrefixProvider *dpp,
805805
RGWObjVersionTracker* objv_tracker) override;
806-
int commit_logging_object(const std::string& obj_name, optional_yield y, const DoutPrefixProvider *dpp) override;
806+
int commit_logging_object(const std::string& obj_name, optional_yield y, const DoutPrefixProvider *dpp, const std::string& prefix, std::string* last_committed) override;
807807
int remove_logging_object(const std::string& obj_name, optional_yield y, const DoutPrefixProvider *dpp) override;
808808
int write_logging_object(const std::string& obj_name, const std::string& record, optional_yield y, const DoutPrefixProvider *dpp, bool async_completion) override;
809809

‎src/rgw/radosgw-admin/radosgw-admin.cc‎

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7832,12 +7832,18 @@ int main(int argc, const char **argv)
78327832
cerr << "ERROR: failed to get pending logging object name from target bucket '" << configuration.target_bucket << "'" << std::endl;
78337833
return -ret;
78347834
}
7835-
const auto old_obj = obj_name;
7835+
std::string old_obj;
78367836
const auto region = driver->get_zone()->get_zonegroup().get_api_name();
7837-
ret = rgw::bucketlogging::rollover_logging_object(configuration, target_bucket, obj_name, dpp(), region, bucket, null_yield, true, &objv_tracker);
7837+
ret = rgw::bucketlogging::rollover_logging_object(configuration, target_bucket, obj_name, dpp(), region, bucket, null_yield, true, &objv_tracker, &old_obj);
78387838
if (ret < 0) {
7839-
cerr << "ERROR: failed to flush pending logging object '" << old_obj
7840-
<< "' to target bucket '" << configuration.target_bucket << "'" << std::endl;
7839+
if (ret == -ENOENT) {
7840+
cerr << "WARNING: no pending logging object '" << obj_name << "'. nothing to flush";
7841+
ret = 0;
7842+
} else {
7843+
cerr << "ERROR: failed flush pending logging object '" << obj_name << "'";
7844+
}
7845+
cerr << " to target bucket '" << configuration.target_bucket << "'. "
7846+
<< " last committed object is '" << old_obj << "'" << std::endl;
78417847
return -ret;
78427848
}
78437849
cout << "flushed pending logging object '" << old_obj

‎src/rgw/rgw_bucket_logging.cc‎

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -301,7 +301,8 @@ int commit_logging_object(const configuration& conf,
301301
const DoutPrefixProvider *dpp,
302302
rgw::sal::Driver* driver,
303303
const std::string& tenant_name,
304-
optional_yield y) {
304+
optional_yield y,
305+
std::string* last_committed) {
305306
std::string target_bucket_name;
306307
std::string target_tenant_name;
307308
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,
319320
<< ret << dendl;
320321
return ret;
321322
}
322-
return commit_logging_object(conf, target_bucket, dpp, y);
323+
return commit_logging_object(conf, target_bucket, dpp, y, last_committed);
323324
}
324325

325326
int commit_logging_object(const configuration& conf,
326327
const std::unique_ptr<rgw::sal::Bucket>& target_bucket,
327328
const DoutPrefixProvider *dpp,
328-
optional_yield y) {
329+
optional_yield y,
330+
std::string* last_committed) {
329331
std::string obj_name;
330332
if (const int ret = target_bucket->get_logging_object_name(obj_name, conf.target_prefix, y, dpp, nullptr); ret < 0) {
331333
ldpp_dout(dpp, 1) << "ERROR: failed to get name of logging object of bucket '" <<
332334
target_bucket->get_key() << "'. ret = " << ret << dendl;
333335
return ret;
334336
}
335-
if (const int ret = target_bucket->commit_logging_object(obj_name, y, dpp); ret <0 ) {
337+
if (const int ret = target_bucket->commit_logging_object(obj_name, y, dpp, conf.target_prefix, last_committed); ret < 0) {
336338
ldpp_dout(dpp, 1) << "ERROR: failed to commit logging object '" << obj_name << "' of bucket '" <<
337339
target_bucket->get_key() << "'. ret = " << ret << dendl;
338340
return ret;
@@ -348,7 +350,8 @@ int rollover_logging_object(const configuration& conf,
348350
const std::unique_ptr<rgw::sal::Bucket>& source_bucket,
349351
optional_yield y,
350352
bool must_commit,
351-
RGWObjVersionTracker* objv_tracker) {
353+
RGWObjVersionTracker* objv_tracker,
354+
std::string* last_committed) {
352355
std::string target_bucket_name;
353356
std::string target_tenant_name;
354357
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,
368371
target_bucket->get_key() << "'. ret = " << ret << dendl;
369372
return ret;
370373
}
371-
if (const int ret = target_bucket->commit_logging_object(old_obj, y, dpp); ret < 0) {
374+
if (const int ret = target_bucket->commit_logging_object(old_obj, y, dpp, conf.target_prefix, last_committed); ret < 0) {
372375
if (must_commit) {
373376
return ret;
374377
}
@@ -476,7 +479,7 @@ int log_record(rgw::sal::Driver* driver,
476479
if (ceph::coarse_real_time::clock::now() > time_to_commit) {
477480
ldpp_dout(dpp, 20) << "INFO: logging object '" << obj_name << "' exceeded its time, will be committed to bucket '" <<
478481
target_bucket_id << "'" << dendl;
479-
if (ret = rollover_logging_object(conf, target_bucket, obj_name, dpp, region, s->bucket, y, false, &objv_tracker); ret < 0) {
482+
if (ret = rollover_logging_object(conf, target_bucket, obj_name, dpp, region, s->bucket, y, false, &objv_tracker, nullptr); ret < 0) {
480483
return ret;
481484
}
482485
} else {
@@ -622,7 +625,7 @@ int log_record(rgw::sal::Driver* driver,
622625
if (ret == -EFBIG) {
623626
ldpp_dout(dpp, 5) << "WARNING: logging object '" << obj_name << "' is full, will be committed to bucket '" <<
624627
target_bucket->get_key() << "'" << dendl;
625-
if (ret = rollover_logging_object(conf, target_bucket, obj_name, dpp, region, s->bucket, y, true, nullptr); ret < 0 ) {
628+
if (ret = rollover_logging_object(conf, target_bucket, obj_name, dpp, region, s->bucket, y, true, nullptr, nullptr); ret < 0 ) {
626629
return ret;
627630
}
628631
if (ret = target_bucket->write_logging_object(obj_name,
@@ -853,7 +856,7 @@ int source_bucket_cleanup(const DoutPrefixProvider* dpp,
853856
return 0;
854857
}
855858
const auto& info = bucket->get_info();
856-
if (const int ret = commit_logging_object(*conf, dpp, driver, info.bucket.tenant, y); ret < 0) {
859+
if (const int ret = commit_logging_object(*conf, dpp, driver, info.bucket.tenant, y, nullptr); ret < 0) {
857860
ldpp_dout(dpp, 5) << "WARNING: could not commit pending logging object of bucket '" <<
858861
bucket->get_key() << "' during cleanup. ret = " << ret << dendl;
859862
} else {

‎src/rgw/rgw_bucket_logging.h‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -172,6 +172,7 @@ int log_record(rgw::sal::Driver* driver,
172172
// commit the pending log objec to the log bucket
173173
// and create a new pending log object
174174
// if "must_commit" is "false" the function will return success even if the pending log object was not committed
175+
// if "last_committed" is not null, it will be set to the name of the last committed object
175176
int rollover_logging_object(const configuration& conf,
176177
const std::unique_ptr<rgw::sal::Bucket>& bucket,
177178
std::string& obj_name,
@@ -180,24 +181,29 @@ int rollover_logging_object(const configuration& conf,
180181
const std::unique_ptr<rgw::sal::Bucket>& source_bucket,
181182
optional_yield y,
182183
bool must_commit,
183-
RGWObjVersionTracker* objv_tracker);
184+
RGWObjVersionTracker* objv_tracker,
185+
std::string* last_committed);
184186

185187
// commit the pending log object to the log bucket
186188
// use this for cleanup, when new pending object is not needed
187189
// and target bucket is known
190+
// if "last_committed" is not null, it will be set to the name of the last committed object
188191
int commit_logging_object(const configuration& conf,
189192
const std::unique_ptr<rgw::sal::Bucket>& target_bucket,
190193
const DoutPrefixProvider *dpp,
191-
optional_yield y);
194+
optional_yield y,
195+
std::string* last_committed);
192196

193197
// commit the pending log object to the log bucket
194198
// use this for cleanup, when new pending object is not needed
195199
// and target bucket shoud be loaded based on the configuration
200+
// if "last_committed" is not null, it will be set to the name of the last committed object
196201
int commit_logging_object(const configuration& conf,
197202
const DoutPrefixProvider *dpp,
198203
rgw::sal::Driver* driver,
199204
const std::string& tenant_name,
200-
optional_yield y);
205+
optional_yield y,
206+
std::string* last_committed);
201207

202208
// return the oid of the object holding the name of the temporary logging object
203209
// bucket - log bucket

‎src/rgw/rgw_rest_bucket_logging.cc‎

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -304,7 +304,7 @@ class RGWPutBucketLoggingOp : public RGWDefaultResponseOp {
304304
}
305305
} else if (*old_conf != configuration) {
306306
// conf changed - do cleanup
307-
if (const auto ret = commit_logging_object(*old_conf, target_bucket, this, y); ret < 0) {
307+
if (const auto ret = commit_logging_object(*old_conf, target_bucket, this, y, nullptr); ret < 0) {
308308
ldpp_dout(this, 1) << "WARNING: could not commit pending logging object when updating logging configuration of bucket '" <<
309309
src_bucket->get_key() << "', ret = " << ret << dendl;
310310
} else {
@@ -385,12 +385,19 @@ class RGWPostBucketLoggingOp : public RGWDefaultResponseOp {
385385
ldpp_dout(this, 1) << "ERROR: failed to get pending logging object name from target bucket '" << target_bucket_id << "'" << dendl;
386386
return;
387387
}
388-
old_obj = obj_name;
389388
const auto region = driver->get_zone()->get_zonegroup().get_api_name();
390-
op_ret = rgw::bucketlogging::rollover_logging_object(configuration, target_bucket, obj_name, this, region, source_bucket, null_yield, true, &objv_tracker);
389+
op_ret = rgw::bucketlogging::rollover_logging_object(configuration, target_bucket, obj_name, this, region, source_bucket, null_yield, true, &objv_tracker, &old_obj);
391390
if (op_ret < 0) {
392-
ldpp_dout(this, 1) << "ERROR: failed to flush pending logging object '" << old_obj
393-
<< "' to target bucket '" << target_bucket_id << "'" << dendl;
391+
if (op_ret == -ENOENT) {
392+
ldpp_dout(this, 5) << "WARNING: no pending logging object '" << obj_name << "'. nothing to flush"
393+
<< " to target bucket '" << target_bucket_id << "'. "
394+
<< " last committed object is '" << old_obj << "'" << dendl;
395+
op_ret = 0;
396+
} else {
397+
ldpp_dout(this, 1) << "ERROR: failed flush pending logging object '" << obj_name << "'"
398+
<< " to target bucket '" << target_bucket_id << "'. "
399+
<< " last committed object is '" << old_obj << "'" << dendl;
400+
}
394401
return;
395402
}
396403
ldpp_dout(this, 20) << "INFO: flushed pending logging object '" << old_obj

‎src/rgw/rgw_sal.h‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1038,8 +1038,10 @@ class Bucket {
10381038
optional_yield y,
10391039
const DoutPrefixProvider *dpp,
10401040
RGWObjVersionTracker* objv_tracker) = 0;
1041-
/** Move the pending bucket logging object into the bucket */
1042-
virtual int commit_logging_object(const std::string& obj_name, optional_yield y, const DoutPrefixProvider *dpp) = 0;
1041+
/** Move the pending bucket logging object into the bucket
1042+
if "last_committed" is not null, it will be set to the name of the last committed object
1043+
* */
1044+
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;
10431045
//** Remove the pending bucket logging object */
10441046
virtual int remove_logging_object(const std::string& obj_name, optional_yield y, const DoutPrefixProvider *dpp) = 0;
10451047
/** Write a record to the pending bucket logging object */

‎src/rgw/rgw_sal_filter.h‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -693,8 +693,8 @@ class FilterBucket : public Bucket {
693693
RGWObjVersionTracker* objv_tracker) override {
694694
return next->remove_logging_object_name(prefix, y, dpp, objv_tracker);
695695
}
696-
int commit_logging_object(const std::string& obj_name, optional_yield y, const DoutPrefixProvider *dpp)override {
697-
return next->commit_logging_object(obj_name, y, dpp);
696+
int commit_logging_object(const std::string& obj_name, optional_yield y, const DoutPrefixProvider *dpp, const std::string& prefix, std::string* last_committed) override {
697+
return next->commit_logging_object(obj_name, y, dpp, prefix, last_committed);
698698
}
699699
int remove_logging_object(const std::string& obj_name, optional_yield y, const DoutPrefixProvider *dpp) override {
700700
return next->remove_logging_object(obj_name, y, dpp);

‎src/rgw/rgw_sal_store.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -266,7 +266,7 @@ class StoreBucket : public Bucket {
266266
optional_yield y,
267267
const DoutPrefixProvider *dpp,
268268
RGWObjVersionTracker* objv_tracker) override { return 0; }
269-
int commit_logging_object(const std::string& obj_name, optional_yield y, const DoutPrefixProvider *dpp) override { return 0; }
269+
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; }
270270
int remove_logging_object(const std::string& obj_name, optional_yield y, const DoutPrefixProvider *dpp) override { return 0; }
271271
int write_logging_object(const std::string& obj_name, const std::string& record, optional_yield y, const DoutPrefixProvider *dpp, bool async_completion) override {
272272
return 0;

0 commit comments

Comments
 (0)