Gargi-jais11 commented on code in PR #11229:
URL: https://github.com/apache/ozone/pull/11229#discussion_r4255524431
##########
hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/util/S3Consts.java:
##########
@@ -131,6 +131,11 @@ public final class S3Consts {
/** Request header carrying the list of object attributes to return. */
public static final String OBJECT_ATTRIBUTES_HEADER =
"x-amz-object-attributes";
+ // S3 storage class values passed via the `x-amz-storage-class` request
header.
+ public static final String S3_STORAGE_CLASS_STANDARD = "STANDARD";
+ public static final String S3_STORAGE_CLASS_STANDARD_IA = "STANDARD_IA";
+ public static final String S3_STORAGE_CLASS_GLACIER = "GLACIER";
+
Review Comment:
Thanks @devmadhuu , the split into `S3StorageClass` , `S3StorageClassSpec`
and `S3StorageClassResolver` looks good to me. Keeping the S3 names separate
from what they mean in Ozone is the right boundary. It also leaves room for the
operator-defined classes @xichen01 mentioned, where the EC layout comes from
configuration rather than a constant.
A few things can make the extension point actually hold up:
1. **Spec fields should be optional** -> If every field is required, each
new property (eg: zones, latency) forces every class to take a position on it,
and the spec stops being easy to extend. It also fixes the `STANDARD` -> `HOT`
problem: `STANDARD` and `REDUCED_REDUNDANCY` can leave the storage policy unset
and inherit the bucket's policy. Otherwise an explicit `STANDARD` header lands
on SSD while a PUT with no header lands on WARM, and any S3 user can override
the bucket policy an admin set.
2. **The spec should hold a repilcation type, not `ReplicationConfig`.** ->
The reverse direction already ignores the EC layout, but forward `STANDARD_IA`
still means a fixed 3-2. If the spec says "EC", the resolver can take the
layout from the storage-config header, then the bucket's EC default, then
config. That covers @xichen01's 4-2 / 6-3 / 9-3 case on writes as well.
3. **Keep the spec an immutable value object** -> so adding a field later is
one change in one class.
I have two questions though:
- **Where does the reverse direction get the policy?**
Right now on feature branch only buckets have a storage policy.
`KeyArgs`/`KeyInfo` have none, and `OMKeyRequest.allocateBlock` uses
`OzoneStoragePolicy.getDefaultPolicy()`. Per-key policy needs a proto/metadata
Jira. Using the bucket policy would make one `SetBucketProperty` to COLD switch
every existing object in the bucket to report GLACIER.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
