rgw/cloud-restore [PART1] : Add Restore support from Glacier/Tape cloud endpoints by soumyakoduri · Pull Request #61745 · ceph/ceph

soumyakoduri · 2025-02-10T18:59:24Z

This PR is the first in a series of enhancements to the restore feature, adding support for S3 Glacier cloud services.

Design doc: https://docs.google.com/document/d/1rzLJAzHK6cLuzJswgoplgugNOFCKi8NrjPOGfgVrIFg/edit?tab=t.0#heading=h.sgrmb31roboc

Unlike regular S3 cloud services, restoring objects from S3/Tape or AWS Glacier services would require special handling. We need to first restore the object using Glacier RestoreObject API and then download it using GET.
https://docs.aws.amazon.com/cli/latest/reference/s3api/restore-object.html

This PR handles below support -

Add new tier-type & options related to S3 Glacier
A new cloud tier-type s3-glacier is added to handle S3 Glacier endpoints along with below tier-config options -
glacier_restore_days - lifetime of the restored copy on the Glacier endpoint ; default: 1 day
glacier_restore_tier_type - Retrieval tier at which the restore will be processed.
Only "Standard" (default) and "Expedited" options are supported.

In addition, a new option restore_storage_class is added to configure the storage class the objects need to be restored to. Default value: STANDARD

Add Restore support from Glacier/Tape cloud endpoints
This PR handles "Expedited" tier retrieval type. That means the cases where in restore would be quick and the object can be downloaded soon.

TODO: "Standard" tier-type support. Need to handle the case where in restore from cloud endpoint could take a longer time and need to be monitored periodically in the background.

TODO: Update cloud-transition doc & add tests.

Note: The first commit is already part of #61569 and will be rebased once merged.

Fixes: https://tracker.ceph.com/issues/68012
Signed-off-by: Soumya Koduri [email protected]

Checklist

Tracker (select at least one)
- References tracker ticket
- Very recent bug; references commit where it was introduced
- New feature (ticket optional)
- Doc update (no ticket needed)
- Code cleanup (no ticket needed)
Component impact
- Affects Dashboard, opened tracker ticket
- Affects Orchestrator, opened tracker ticket
- No impact that needs to be tracked
Documentation (select at least one)
- Updates relevant documentation
- No doc update is appropriate
Tests (select at least one)
- Includes unit test(s)
- Includes integration test(s)
- Includes bug reproducer
- No tests

Show available Jenkins commands

jenkins retest this please
jenkins test classic perf
jenkins test crimson perf
jenkins test signed
jenkins test make check
jenkins test make check arm64
jenkins test submodules
jenkins test dashboard
jenkins test dashboard cephadm
jenkins test api
jenkins test docs
jenkins render docs
jenkins test ceph-volume all
jenkins test ceph-volume tox
jenkins test windows
jenkins test rook e2e

thotz · 2025-02-24T12:53:54Z

src/rgw/driver/rados/rgw_obj_manifest.h

  }

+  bool is_tier_type_s3() {
+      return (tier_type == "cloud-s3" || tier_type == "cloud-s3-glacier");


IMO better to seperate them out

We would still need one common routine for all s3-* style tier types as most of the S3 endpoint options and processing would be common. And there is already one defined for just "glacier" below to use when needed specific to glacier

dang · 2025-03-03T16:51:25Z

src/rgw/rgw_sal.h


  /** Get the type of this tier */
  virtual const std::string& get_tier_type() = 0;
+  /** Is the type of this tier cloud-s3/clous-s3-glacier */


clous -> cloud

thotz · 2025-03-04T10:36:50Z

src/rgw/driver/rados/rgw_lc_tier.cc


+/* Read object or just head from remote endpoint.
+ */
+int rgw_cloud_tier_restore_object(RGWLCCloudTierCtx& tier_ctx, bool head,


head is not used in this function, rgw_cloud_tier_get_object() with a boolean value directly

thotz · 2025-03-04T11:36:15Z

src/rgw/driver/rados/rgw_rados.cc

  RGWRadosPutObj cb(dpp, cct, plugin, compressor, &processor, progress_cb, progress_data,
                    [&](map<string, bufferlist> obj_attrs) {
-                      // XXX: do we need filter() like in fetch_remote_obj() cb
+                      // XXX: do we need filter() lke in fetch_remote_obj() cb


unintented change

thotz · 2025-03-04T11:36:55Z

src/rgw/driver/rados/rgw_rados.cc

                       * for permanent restore
                       */
-                      dest_placement.storage_class = RGW_STORAGE_CLASS_STANDARD;
+  //                    dest_placement.storage_class = RGW_STORAGE_CLASS_STANDARD;


what will be the storage in this case?

It is now updated in Line:5281 above. A new option "restore_storage_class" is added (default:STANDARD) to configure restored object storage-class.

src/rgw/rgw_zone_types.h

thotz

Minor comments overall LGTM, saw two swap files related to vim in PR

soumyakoduri · 2025-03-04T18:48:47Z

Minor comments overall LGTM, saw two swap files related to vim in PR

Thanks. Addressed them.

github-actions · 2025-03-06T15:45:34Z

This pull request can no longer be automatically merged: a rebase is needed and changes have to be manually resolved

src/rgw/rgw_zone_types.h

soumyakoduri · 2025-03-10T15:52:09Z

jenkins test make check

soumyakoduri · 2025-03-11T12:49:37Z

teuthology tests - http://pulpito.front.sepia.ceph.com/soumyakoduri-2025-03-10_15:55:41-rgw-wip-skoduri-glacier-cli-distro-default-smithi/
http://pulpito.front.sepia.ceph.com/soumyakoduri-2025-03-10_15:55:01-rgw:multisite-wip-skoduri-glacier-cli-distro-default-smithi

soumyakoduri · 2025-03-11T12:49:53Z

jenkins test make check

ivancich

Whenever I see so many duplicates of hard-coded strings, I think that argues for a macro (old style) or (better) (static const or static constexpr). That avoids potential minor typo-issues and allows the compiler to do some checking.

A few that I noticed:

"cloud-s3-glacier"
"cloud-s3"
"glacier_restore_days"
"restore_storage_class"

ivancich · 2025-03-11T17:46:57Z

src/rgw/driver/rados/rgw_lc_tier.cc

+
+  if (!s.empty()){
+    const char *r_str = "ongoing-request=\"true\"";
+    char *found = strstr((char*)s.c_str(), r_str);


nit -- why is there a cast to (char*)? strstr takes a const char* as a parameter and that's what c_str() returns.

ivancich · 2025-03-11T17:55:04Z

src/rgw/rgw_zone_types.h

    o.back()->tier_type = "cloud-s3";
    o.back()->storage_class = "STANDARD";
+    o.back()->allow_read_through = false;
+    o.back()->restore_storage_class = "STANDARD";


Since you have a macro RGW_STORAGE_CLASS_STANDARD for this, maybe use it instead of hard-coding the string here and a few lines above. (I realize you're just duplicating the local coding style.)

soumyakoduri · 2025-03-14T11:01:59Z

Whenever I see so many duplicates of hard-coded strings, I think that argues for a macro (old style) or (better) (static const or static constexpr). That avoids potential minor typo-issues and allows the compiler to do some checking.

A few that I noticed:
* "cloud-s3-glacier"

* "cloud-s3"

* "glacier_restore_days"

* "restore_storage_class"

Thanks Eric. I have addressed string literals "cloud-s3-glacier" and "cloud-s3" as suggested above in c6559af . Please review.

Unlike regular S3 cloud services, restoring objects from S3/Tape or AWS Glacier services would require special handling. We need to first restore the object using Glacier `RestoreObject` API and then download it using `GET`. https://docs.aws.amazon.com/cli/latest/reference/s3api/restore-object.html A new cloud tier-type `s3-glacier` is added to handle S3 Glacier endpoints along with below tier-config options - `glacier_restore_days` - lifetime of the restored copy on the Glacier endpoint ; default: 1 day `glacier_restore_tier_type` - Retrieval tier at which the restore will be processed. Only "Standard" (default) and "Expedited" options are supported. In addition, a new option `restore_storage_class` is added to configure the storage class the objects need to be restored to. Default value: STANDARD Design doc: https://docs.google.com/document/d/1rzLJAzHK6cLuzJswgoplgugNOFCKi8NrjPOGfgVrIFg/edit?tab=t.0#heading=h.sgrmb31roboc Signed-off-by: Soumya Koduri <[email protected]>

Signed-off-by: Soumya Koduri <[email protected]>

Unlike regular S3 cloud services, restoring objects from S3/Tape or AWS Glacier services would require special handling. We need to first restore the object using Glacier RestoreObject API and then download it using GET. https://docs.aws.amazon.com/cli/latest/reference/s3api/restore-object.html This PR adds that support for "Expedited" tier retrieval type. That means the restore would be quick and the object can be downloaded soon. TODO: "Standard" tier-type support. Need to handle the case where in restore from cloud endpoint could take a longer time and need to be monitored periodically in the background. Signed-off-by: Soumya Koduri <[email protected]>

soumyakoduri · 2025-03-17T01:56:29Z

jenkins test api

thotz · 2025-03-20T06:35:43Z

jenkins test api

thotz · 2025-03-20T12:58:08Z

jenkins test api

This commit fixes an undetected merge conflict between PRs ceph#61745 and ceph#60159. The dencoding problem has been introduced very recently, it is straightforward and causes failures of the make check bot everywhere, therefore -- if no objections -- I want to merge this patch without the Teuthology testing. Signed-off-by: Radoslaw Zarzynski <[email protected]>

cbodley · 2025-03-26T12:40:02Z

src/rgw/rgw_zone_types.h

-    ENCODE_START(3, 1, bl);
+    ENCODE_START(4, 1, bl);


@soumyakoduri @thotz please note the fix for this in #62515 (has this been backported?)

No. This feature is not present in squid upstream release. (Yet to be backported into downstream though.)

Yet to be backported into downstream though

okay, thanks - just wanted to make sure we don't miss the fix downstream

yes. Thank you! We weren't aware of this issue till now.

soumyakoduri requested a review from a team as a code owner February 10, 2025 18:59

github-actions bot added the rgw label Feb 10, 2025

soumyakoduri requested review from cbodley, dang, ivancich, mattbenjamin and thotz February 10, 2025 19:05

thotz reviewed Feb 24, 2025

View reviewed changes

soumyakoduri force-pushed the wip-skoduri-glacier-cli branch from 61d2bd5 to 4963de8 Compare March 1, 2025 17:44

soumyakoduri changed the title ~~rgw/cloud-restore: Add new tier-type & options related to S3 Glacier~~ rgw/cloud-restore [PART1] : Add Restore support from Glacier/Tape cloud endpoints Mar 1, 2025

soumyakoduri requested a review from adamemerson March 1, 2025 17:50

dang approved these changes Mar 3, 2025

View reviewed changes

thotz reviewed Mar 4, 2025

View reviewed changes

soumyakoduri force-pushed the wip-skoduri-glacier-cli branch from 4963de8 to 05e4bbd Compare March 5, 2025 06:48

thotz approved these changes Mar 6, 2025

View reviewed changes

github-actions bot added the needs-rebase label Mar 6, 2025

soumyakoduri force-pushed the wip-skoduri-glacier-cli branch from 05e4bbd to a3d412a Compare March 6, 2025 16:58

github-actions bot removed the needs-rebase label Mar 6, 2025

cbodley reviewed Mar 6, 2025

View reviewed changes

src/rgw/rgw_zone_types.h Outdated Show resolved Hide resolved

soumyakoduri force-pushed the wip-skoduri-glacier-cli branch from a3d412a to d4241c4 Compare March 7, 2025 08:51

ivancich reviewed Mar 11, 2025

View reviewed changes

soumyakoduri force-pushed the wip-skoduri-glacier-cli branch from d4241c4 to 6ffe699 Compare March 14, 2025 10:35

soumyakoduri added 2 commits March 15, 2025 21:43

rgw/cloud-tier: Redefining cloud tier types

38d2c11

Signed-off-by: Soumya Koduri <[email protected]>

soumyakoduri force-pushed the wip-skoduri-glacier-cli branch from 6ffe699 to 300f643 Compare March 15, 2025 16:16

thotz requested review from cbodley and ivancich March 20, 2025 06:37

thotz approved these changes Mar 20, 2025

View reviewed changes

thotz merged commit 6332769 into ceph:main Mar 21, 2025
11 checks passed

rzarzynski mentioned this pull request Mar 26, 2025

rgw: fix the version in DECODE_START() of RGWZoneGroupPlacementTier #62515

Merged

14 tasks

cbodley reviewed Mar 26, 2025

View reviewed changes

soumyakoduri mentioned this pull request Apr 7, 2025

rgw/cloud-restore [PART2] : Add Restore support from Glacier/Tape cloud endpoints #62713

Merged

14 tasks

Comments

Conversation

soumyakoduri commented Feb 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Checklist

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

thotz left a comment

Choose a reason for hiding this comment

Uh oh!

soumyakoduri commented Mar 4, 2025

Uh oh!

github-actions bot commented Mar 6, 2025

Uh oh!

Uh oh!

soumyakoduri commented Mar 10, 2025

Uh oh!

soumyakoduri commented Mar 11, 2025

Uh oh!

soumyakoduri commented Mar 11, 2025

Uh oh!

ivancich left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

soumyakoduri commented Mar 14, 2025

Uh oh!

soumyakoduri commented Mar 17, 2025

Uh oh!

thotz commented Mar 20, 2025

Uh oh!

thotz commented Mar 20, 2025

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

soumyakoduri commented Feb 10, 2025 •

edited

Loading