Skip to content

Commit 111bb61

Browse files
yuvalifyuvalif
authored
Merge pull request ceph#65387 from yuvalif/wip-yuval-72542
rgw/logging: allow committing empty objects
2 parents cc02dd5 + 62fed99 commit 111bb61

File tree

5 files changed

+70
-46
lines changed

5 files changed

+70
-46
lines changed

‎doc/radosgw/s3/bucketops.rst‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -919,7 +919,7 @@ Flush Bucket Logging
919919
--------------------
920920

921921
Flushes logging object for a given source bucket (if not flushed, the logging objects are written lazily to the log bucket).
922-
Returns the name of the object that was flushed. An empty name will be returned if no object needs to be flushed.
922+
Returns the name of the object that was flushed. Flushing will happen even if the logging object is empty.
923923

924924
Syntax
925925
~~~~~~

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

Lines changed: 26 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1138,7 +1138,7 @@ int RadosBucket::get_logging_object_name(std::string& obj_name,
11381138
return -EIO;
11391139
}
11401140
bufferlist bl;
1141-
const int ret = rgw_get_system_obj(store->svc()->sysobj,
1141+
if (const int ret = rgw_get_system_obj(store->svc()->sysobj,
11421142
data_pool,
11431143
obj_name_oid,
11441144
bl,
@@ -1147,13 +1147,12 @@ int RadosBucket::get_logging_object_name(std::string& obj_name,
11471147
y,
11481148
dpp,
11491149
nullptr,
1150-
nullptr);
1151-
if (ret < 0) {
1152-
if (ret == -ENOENT) {
1153-
ldpp_dout(dpp, 20) << "INFO: logging object name '" << obj_name_oid << "' not found. ret = " << ret << dendl;
1154-
return ret;
1150+
nullptr); ret < 0) {
1151+
if (ret != -ENOENT) {
1152+
ldpp_dout(dpp, 1) << "ERROR: failed to get logging object name from '" << obj_name_oid << "'. ret = " << ret << dendl;
1153+
} else {
1154+
ldpp_dout(dpp, 20) << "INFO: logging object name does not exist at '" << obj_name_oid << "'" << dendl;
11551155
}
1156-
ldpp_dout(dpp, 1) << "ERROR: failed to get logging object name from '" << obj_name_oid << "'. ret = " << ret << dendl;
11571156
return ret;
11581157
}
11591158
obj_name = bl.to_str();
@@ -1274,7 +1273,7 @@ int RadosBucket::commit_logging_object(const std::string& obj_name,
12741273
std::map<string, bufferlist> obj_attrs;
12751274
ceph::real_time mtime;
12761275
bufferlist bl_data;
1277-
if (const auto ret = rgw_get_system_obj(store->svc()->sysobj,
1276+
if (auto ret = rgw_get_system_obj(store->svc()->sysobj,
12781277
data_pool,
12791278
temp_obj_name,
12801279
bl_data,
@@ -1284,13 +1283,28 @@ int RadosBucket::commit_logging_object(const std::string& obj_name,
12841283
dpp,
12851284
&obj_attrs,
12861285
nullptr); ret < 0) {
1287-
if (ret == -ENOENT) {
1288-
ldpp_dout(dpp, 5) << "WARNING: temporary logging object '" << temp_obj_name << "' does not exists" << dendl;
1289-
} else {
1286+
if (ret != -ENOENT) {
12901287
ldpp_dout(dpp, 1) << "ERROR: failed to read logging data when committing object '" << temp_obj_name
12911288
<< ". error: " << ret << dendl;
1289+
return ret;
1290+
}
1291+
ldpp_dout(dpp, 20) << "INFO: temporary logging object '" << temp_obj_name << "' does not exist. committing it empty" << dendl;
1292+
// creating an empty object
1293+
if (ret = rgw_put_system_obj(dpp, store->svc()->sysobj,
1294+
data_pool,
1295+
temp_obj_name,
1296+
bl_data, // empty bufferlist
1297+
true, // exclusive
1298+
nullptr,
1299+
ceph::real_time::clock::now(),
1300+
y); ret < 0) {
1301+
if (ret == -EEXIST) {
1302+
ldpp_dout(dpp, 5) << "WARNING: race detected in committing an empty logging object '" << temp_obj_name << dendl;
1303+
} else {
1304+
ldpp_dout(dpp, 1) << "ERROR: failed to commit empty logging object '" << temp_obj_name << "'. error: " << ret << dendl;
1305+
}
1306+
return ret;
12921307
}
1293-
return ret;
12941308
}
12951309

12961310
uint64_t size = bl_data.length();

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

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -7812,22 +7812,18 @@ int main(int argc, const char **argv)
78127812
std::string obj_name;
78137813
RGWObjVersionTracker objv_tracker;
78147814
ret = target_bucket->get_logging_object_name(obj_name, configuration.target_prefix, null_yield, dpp(), &objv_tracker);
7815-
if (ret < 0) {
7816-
cerr << "ERROR: failed to get pending logging object name from target bucket '" << configuration.target_bucket << "'" << std::endl;
7815+
if (ret < 0 && ret != -ENOENT) {
7816+
cerr << "ERROR: failed to get pending logging object name from target bucket '" << configuration.target_bucket <<
7817+
"'. error: " << cpp_strerror(-ret) << std::endl;
78177818
return -ret;
78187819
}
78197820
std::string old_obj;
78207821
const auto region = driver->get_zone()->get_zonegroup().get_api_name();
78217822
ret = rgw::bucketlogging::rollover_logging_object(configuration, target_bucket, obj_name, dpp(), region, bucket, null_yield, true, &objv_tracker, &old_obj);
78227823
if (ret < 0) {
7823-
if (ret == -ENOENT) {
7824-
cerr << "WARNING: no pending logging object '" << obj_name << "'. nothing to flush";
7825-
ret = 0;
7826-
} else {
7827-
cerr << "ERROR: failed flush pending logging object '" << obj_name << "'";
7828-
}
7829-
cerr << " to target bucket '" << configuration.target_bucket << "'. "
7830-
<< " last committed object is '" << old_obj << "'" << std::endl;
7824+
cerr << "ERROR: failed to flush pending logging object '" << obj_name << "' to target bucket '" << configuration.target_bucket << "'. "
7825+
<< " last committed object is '" << old_obj <<
7826+
"'. error: " << cpp_strerror(-ret) << std::endl;
78317827
return -ret;
78327828
}
78337829
cout << "flushed pending logging object '" << old_obj

‎src/rgw/rgw_bucket_logging.cc‎

Lines changed: 26 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -368,25 +368,40 @@ int rollover_logging_object(const configuration& conf,
368368
"', bucket= '" << target_bucket->get_key() << "'" << dendl;
369369
return -EINVAL;
370370
}
371-
const auto old_obj = obj_name;
372-
const int ret = new_logging_object(conf, target_bucket, obj_name, dpp, region, source_bucket, y, old_obj, objv_tracker);
373-
if (ret == -ECANCELED) {
374-
ldpp_dout(dpp, 20) << "INFO: rollover already performed for object '" << old_obj << "' to logging bucket '" <<
375-
target_bucket->get_key() << "'. ret = " << ret << dendl;
376-
return 0;
377-
} else if (ret < 0) {
378-
ldpp_dout(dpp, 1) << "ERROR: failed to rollover object '" << old_obj << "' to logging bucket '" <<
379-
target_bucket->get_key() << "'. ret = " << ret << dendl;
371+
372+
auto old_obj = obj_name.empty() ? std::nullopt : std::optional<std::string>(obj_name);
373+
374+
auto handle_error = [&dpp, &old_obj, &target_bucket](int ret) {
375+
if (ret == -ECANCELED) {
376+
ldpp_dout(dpp, 20) << "INFO: rollover already performed for logging object '" << old_obj << "' to logging bucket '" <<
377+
target_bucket->get_key() << "'. ret = " << ret << dendl;
378+
return 0;
379+
}
380+
if (ret < 0) {
381+
ldpp_dout(dpp, 1) << "ERROR: failed to rollover logging object '" << old_obj << "' to logging bucket '" <<
382+
target_bucket->get_key() << "'. ret = " << ret << dendl;
383+
}
384+
return ret;
385+
};
386+
387+
if (const int ret = handle_error(new_logging_object(conf, target_bucket, obj_name, dpp, region, source_bucket, y, old_obj, objv_tracker)); ret < 0) {
380388
return ret;
381389
}
382-
if (const int ret = target_bucket->commit_logging_object(old_obj, y, dpp, conf.target_prefix, last_committed); ret < 0) {
390+
if (!old_obj) {
391+
// first time logging old == new
392+
old_obj = obj_name;
393+
if (const int ret = handle_error(new_logging_object(conf, target_bucket, obj_name, dpp, region, source_bucket, y, old_obj, objv_tracker)); ret < 0) {
394+
return ret;
395+
}
396+
}
397+
if (const int ret = target_bucket->commit_logging_object(*old_obj, y, dpp, conf.target_prefix, last_committed); ret < 0) {
383398
if (must_commit) {
384399
return ret;
385400
}
386401
ldpp_dout(dpp, 5) << "WARNING: failed to commit object '" << old_obj << "' to logging bucket '" <<
387402
target_bucket->get_key() << "'. ret = " << ret << dendl;
388403
// we still want to write the new records to the new object even if commit failed
389-
// will try to commit again next time
404+
// TODO: should add commit retry mechanism
390405
}
391406
return 0;
392407
}

‎src/rgw/rgw_rest_bucket_logging.cc‎

Lines changed: 11 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -384,23 +384,22 @@ class RGWPostBucketLoggingOp : public RGWDefaultResponseOp {
384384
std::string obj_name;
385385
RGWObjVersionTracker objv_tracker;
386386
op_ret = target_bucket->get_logging_object_name(obj_name, configuration.target_prefix, y, this, &objv_tracker);
387-
if (op_ret < 0) {
388-
ldpp_dout(this, 1) << "ERROR: failed to get pending logging object name from target bucket '" << target_bucket_id << "'" << dendl;
387+
if (op_ret < 0 && op_ret != -ENOENT) {
388+
ldpp_dout(this, 1) << "ERROR: failed to get pending logging object name from target bucket '" << target_bucket_id <<
389+
"'. error: " << op_ret << dendl;
389390
return;
390391
}
392+
if (op_ret == -ENOENT) {
393+
// no pending object - nothing to flush
394+
ldpp_dout(this, 5) << "INFO: no pending logging object in target bucket '" << target_bucket_id << "'. new object should be created" << dendl;
395+
}
391396
const auto region = driver->get_zone()->get_zonegroup().get_api_name();
392397
op_ret = rgw::bucketlogging::rollover_logging_object(configuration, target_bucket, obj_name, this, region, source_bucket, y, true, &objv_tracker, &old_obj);
393398
if (op_ret < 0) {
394-
if (op_ret == -ENOENT) {
395-
ldpp_dout(this, 5) << "WARNING: no pending logging object '" << obj_name << "'. nothing to flush"
396-
<< " to target bucket '" << target_bucket_id << "'. "
397-
<< " last committed object is '" << old_obj << "'" << dendl;
398-
op_ret = 0;
399-
} else {
400-
ldpp_dout(this, 1) << "ERROR: failed flush pending logging object '" << obj_name << "'"
401-
<< " to target bucket '" << target_bucket_id << "'. "
402-
<< " last committed object is '" << old_obj << "'" << dendl;
403-
}
399+
ldpp_dout(this, 1) << "ERROR: failed to flush pending logging object '" << obj_name << "'"
400+
<< " to target bucket '" << target_bucket_id << "'. "
401+
<< " last committed object is '" << old_obj <<
402+
"'. error: " << op_ret << dendl;
404403
return;
405404
}
406405
ldpp_dout(this, 20) << "INFO: flushed pending logging object '" << old_obj

0 commit comments

Comments
 (0)