Skip to content

Commit 5005343

Browse files
committed
rgw/logging: fix partitioned key format
Fixes: https://tracker.ceph.com/issues/71537 Signed-off-by: Yuval Lifshitz <[email protected]>
1 parent c71d7ee commit 5005343

File tree

5 files changed

+33
-22
lines changed

5 files changed

+33
-22
lines changed

doc/radosgw/bucket_logging.rst

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -153,13 +153,13 @@ has the following format:
153153

154154
::
155155

156-
<prefix><bucket owner>/<source region>/[tenant:]<bucket name>/<year>/<month>/<day>/<year-month-day-hour-minute-second>-<16 bytes unique-id>
156+
<prefix><source bucket owner>/<zone group>/[tenant:]<source bucket name>/<year>/<month>/<day>/<year-month-day-hour-minute-second>-<16 bytes unique-id>
157157

158158
For example:
159159

160160
::
161161

162-
fish/testid//all-log/2024/08/06/2024-08-06-10-11-18-0000000000000002
162+
fish/testid/default/fish-bucket/2024/08/06/2024-08-06-10-11-18-0000000000000002
163163

164164
Log Records
165165
~~~~~~~~~~~

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7833,7 +7833,8 @@ int main(int argc, const char **argv)
78337833
return -ret;
78347834
}
78357835
const auto old_obj = obj_name;
7836-
ret = rgw::bucketlogging::rollover_logging_object(configuration, target_bucket, obj_name, dpp(), null_yield, true, &objv_tracker);
7836+
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);
78377838
if (ret < 0) {
78387839
cerr << "ERROR: failed to flush pending logging object '" << old_obj
78397840
<< "' to target bucket '" << configuration.target_bucket << "'" << std::endl;

src/rgw/rgw_bucket_logging.cc

Lines changed: 22 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -242,6 +242,8 @@ int new_logging_object(const configuration& conf,
242242
const std::unique_ptr<rgw::sal::Bucket>& target_bucket,
243243
std::string& obj_name,
244244
const DoutPrefixProvider *dpp,
245+
const std::string& region,
246+
const std::unique_ptr<rgw::sal::Bucket>& source_bucket,
245247
optional_yield y,
246248
std::optional<std::string> old_name,
247249
RGWObjVersionTracker* objv_tracker) {
@@ -253,20 +255,21 @@ int new_logging_object(const configuration& conf,
253255

254256
switch (conf.obj_key_format) {
255257
case KeyFormat::Simple:
258+
// [DestinationPrefix][YYYY]-[MM]-[DD]-[hh]-[mm]-[ss]-[UniqueString]
256259
obj_name = fmt::format("{}{:%Y-%m-%d-%H-%M-%S}-{}",
257260
conf.target_prefix,
258261
t,
259262
unique);
260263
break;
261264
case KeyFormat::Partitioned:
262265
{
263-
// TODO: use date_source
264-
const auto source_region = ""; // TODO
266+
// TODO: support both EventTime and DeliveryTime
267+
// [DestinationPrefix][SourceAccountId]/[SourceRegion]/[SourceBucket]/[YYYY]/[MM]/[DD]/[YYYY]-[MM]-[DD]-[hh]-[mm]-[ss]-[UniqueString]
265268
obj_name = fmt::format("{}{}/{}/{}/{:%Y/%m/%d}/{:%Y-%m-%d-%H-%M-%S}-{}",
266269
conf.target_prefix,
267-
to_string(target_bucket->get_owner()),
268-
source_region,
269-
full_bucket_name(target_bucket),
270+
to_string(source_bucket->get_owner()),
271+
region,
272+
full_bucket_name(source_bucket),
270273
t,
271274
t,
272275
unique);
@@ -341,6 +344,8 @@ int rollover_logging_object(const configuration& conf,
341344
const std::unique_ptr<rgw::sal::Bucket>& target_bucket,
342345
std::string& obj_name,
343346
const DoutPrefixProvider *dpp,
347+
const std::string& region,
348+
const std::unique_ptr<rgw::sal::Bucket>& source_bucket,
344349
optional_yield y,
345350
bool must_commit,
346351
RGWObjVersionTracker* objv_tracker) {
@@ -353,7 +358,7 @@ int rollover_logging_object(const configuration& conf,
353358
return -EINVAL;
354359
}
355360
const auto old_obj = obj_name;
356-
const int ret = new_logging_object(conf, target_bucket, obj_name, dpp, y, old_obj, objv_tracker);
361+
const int ret = new_logging_object(conf, target_bucket, obj_name, dpp, region, source_bucket, y, old_obj, objv_tracker);
357362
if (ret == -ECANCELED) {
358363
ldpp_dout(dpp, 20) << "INFO: rollover already performed for object '" << old_obj << "' to logging bucket '" <<
359364
target_bucket->get_key() << "'. ret = " << ret << dendl;
@@ -462,6 +467,7 @@ int log_record(rgw::sal::Driver* driver,
462467
return ret;
463468
}
464469

470+
const auto region = driver->get_zone()->get_zonegroup().get_api_name();
465471
std::string obj_name;
466472
RGWObjVersionTracker objv_tracker;
467473
ret = target_bucket->get_logging_object_name(obj_name, conf.target_prefix, y, dpp, &objv_tracker);
@@ -470,15 +476,15 @@ int log_record(rgw::sal::Driver* driver,
470476
if (ceph::coarse_real_time::clock::now() > time_to_commit) {
471477
ldpp_dout(dpp, 20) << "INFO: logging object '" << obj_name << "' exceeded its time, will be committed to bucket '" <<
472478
target_bucket_id << "'" << dendl;
473-
if (ret = rollover_logging_object(conf, target_bucket, obj_name, dpp, y, false, &objv_tracker); ret < 0) {
479+
if (ret = rollover_logging_object(conf, target_bucket, obj_name, dpp, region, s->bucket, y, false, &objv_tracker); ret < 0) {
474480
return ret;
475481
}
476482
} else {
477483
ldpp_dout(dpp, 20) << "INFO: record will be written to current logging object '" << obj_name << "'. will be comitted at: " << time_to_commit << dendl;
478484
}
479485
} else if (ret == -ENOENT) {
480486
// try to create the temporary log object for the first time
481-
ret = new_logging_object(conf, target_bucket, obj_name, dpp, y, std::nullopt, nullptr);
487+
ret = new_logging_object(conf, target_bucket, obj_name, dpp, region, s->bucket, y, std::nullopt, nullptr);
482488
if (ret == 0) {
483489
ldpp_dout(dpp, 20) << "INFO: first time logging for bucket '" << target_bucket_id << "' and prefix '" <<
484490
conf.target_prefix << "'" << dendl;
@@ -509,9 +515,13 @@ int log_record(rgw::sal::Driver* driver,
509515
fqdn.append(".").append(s->info.domain);
510516
}
511517

518+
std::string aws_version("-");
519+
std::string auth_type("-");
520+
rgw::auth::s3::get_aws_version_and_auth_type(s, aws_version, auth_type);
512521
std::string bucket_owner;
513522
std::string bucket_name;
514-
if (log_source_bucket) {
523+
if (log_source_bucket && conf.logging_type == LoggingType::Standard) {
524+
// log source bucket for COPY operations only in standard mode
515525
if (!s->src_object || !s->src_object->get_bucket()) {
516526
ldpp_dout(dpp, 1) << "ERROR: source object or bucket is missing when logging source bucket" << dendl;
517527
return -EINVAL;
@@ -523,9 +533,6 @@ int log_record(rgw::sal::Driver* driver,
523533
bucket_name = full_bucket_name(s->bucket);
524534
}
525535

526-
std::string aws_version("-");
527-
std::string auth_type("-");
528-
rgw::auth::s3::get_aws_version_and_auth_type(s, aws_version, auth_type);
529536

530537
switch (conf.logging_type) {
531538
case LoggingType::Standard:
@@ -562,8 +569,8 @@ int log_record(rgw::sal::Driver* driver,
562569
break;
563570
case LoggingType::Journal:
564571
record = fmt::format("{} {} [{:%d/%b/%Y:%H:%M:%S %z}] {} {} {} {} {}",
565-
dash_if_empty(to_string(s->bucket->get_owner())),
566-
dash_if_empty(full_bucket_name(s->bucket)),
572+
dash_if_empty(bucket_owner),
573+
dash_if_empty(bucket_name),
567574
t,
568575
op_name,
569576
dash_if_empty_or_null(obj, obj->get_name()),
@@ -615,7 +622,7 @@ int log_record(rgw::sal::Driver* driver,
615622
if (ret == -EFBIG) {
616623
ldpp_dout(dpp, 5) << "WARNING: logging object '" << obj_name << "' is full, will be committed to bucket '" <<
617624
target_bucket->get_key() << "'" << dendl;
618-
if (ret = rollover_logging_object(conf, target_bucket, obj_name, dpp, y, true, nullptr); ret < 0 ) {
625+
if (ret = rollover_logging_object(conf, target_bucket, obj_name, dpp, region, s->bucket, y, true, nullptr); ret < 0 ) {
619626
return ret;
620627
}
621628
if (ret = target_bucket->write_logging_object(obj_name,

src/rgw/rgw_bucket_logging.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -176,6 +176,8 @@ int rollover_logging_object(const configuration& conf,
176176
const std::unique_ptr<rgw::sal::Bucket>& bucket,
177177
std::string& obj_name,
178178
const DoutPrefixProvider *dpp,
179+
const std::string& region,
180+
const std::unique_ptr<rgw::sal::Bucket>& source_bucket,
179181
optional_yield y,
180182
bool must_commit,
181183
RGWObjVersionTracker* objv_tracker);

src/rgw/rgw_rest_bucket_logging.cc

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -340,22 +340,22 @@ class RGWPostBucketLoggingOp : public RGWDefaultResponseOp {
340340
// and usd in execute()
341341
rgw::bucketlogging::configuration configuration;
342342
std::unique_ptr<rgw::sal::Bucket> target_bucket;
343+
std::unique_ptr<rgw::sal::Bucket> source_bucket;
343344

344345
int init_processing(optional_yield y) override {
345346
if (const auto ret = verify_bucket_logging_params(this, s); ret < 0) {
346347
return ret;
347348
}
348349

349-
std::unique_ptr<rgw::sal::Bucket> src_bucket;
350350
{
351351
const rgw_bucket src_bucket_id{s->bucket_tenant, s->bucket_name};
352352
if (const auto ret = driver->load_bucket(this, src_bucket_id,
353-
&src_bucket, y); ret < 0) {
353+
&source_bucket, y); ret < 0) {
354354
ldpp_dout(this, 1) << "ERROR: failed to get bucket '" << src_bucket_id << "', ret = " << ret << dendl;
355355
return ret;
356356
}
357357
}
358-
return rgw::bucketlogging::get_target_and_conf_from_source(this, driver, src_bucket.get(), s->bucket_tenant, configuration, target_bucket, y);
358+
return rgw::bucketlogging::get_target_and_conf_from_source(this, driver, source_bucket.get(), s->bucket_tenant, configuration, target_bucket, y);
359359
}
360360

361361
int verify_permission(optional_yield y) override {
@@ -385,7 +385,8 @@ class RGWPostBucketLoggingOp : public RGWDefaultResponseOp {
385385
return;
386386
}
387387
const auto old_obj = obj_name;
388-
op_ret = rgw::bucketlogging::rollover_logging_object(configuration, target_bucket, obj_name, this, null_yield, true, &objv_tracker);
388+
const auto region = driver->get_zone()->get_zonegroup().get_api_name();
389+
op_ret = rgw::bucketlogging::rollover_logging_object(configuration, target_bucket, obj_name, this, region, source_bucket, null_yield, true, &objv_tracker);
389390
if (op_ret < 0) {
390391
ldpp_dout(this, 1) << "ERROR: failed to flush pending logging object '" << old_obj
391392
<< "' to target bucket '" << target_bucket_id << "'" << dendl;

0 commit comments

Comments
 (0)