Skip to content

HDDS-16039. Introduce S3StorageClass mapping to OzoneStoragePolicy. - #11229

Open
devmadhuu wants to merge 1 commit into
apache:HDDS-11233from
devmadhuu:HDDS-16039
Open

devmadhuu wants to merge 1 commit into
apache:HDDS-11233from
devmadhuu:HDDS-16039

Conversation

@devmadhuu

Copy link
Copy Markdown
Contributor

What changes were proposed in this pull request?

This PR is to introduce S3StorageClass — a small enum that maps AWS S3 storage class strings (STANDARD, STANDARD_IA, GLACIER) to Ozone storage policies (HOT, WARM, COLD).

Two helper methods on the enum:

  • fromS3StorageClass(String) — parse the value of an incoming x-amz-storage-class header.
  • fromStoragePolicy(StoragePolicy) — go the other way, for building responses.

This PR is infrastructure only — no caller uses S3StorageClass yet. The S3 write path (PutObject) and read path (GetObject) will start calling it in follow-up PRs.

What is the link to the Apache JIRA

https://issues.apache.org/jira/browse/HDDS-16039

How was this patch tested?

New unit test TestS3StorageClass covers:

  • S3 string → enum for all three values, plus rejects null and unknown values.
  • Ozone StoragePolicy → enum for all three, plus rejects null.
  • Round-trip consistency across all enum values.

@devmadhuu
devmadhuu marked this pull request as ready for review September 11, 2026 11:09
throw new IllegalArgumentException("Not supported s3StorageClass: null");
}
try {
return S3StorageClass.valueOf(s3StorageClass.toUpperCase());

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @devmadhuu.
I wonder would Locale.ROOT be safer here? toUpperCase() uses the JVM's default locale.
For example, under Turkish locale, "standard_ia" becomes "STANDARD_İA", which does not match the enum constant.
The Java documentation recommends Locale.ROOT for locale-independent strings such as protocol keys.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

@Gargi-jais11 Gargi-jais11 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @devmadhuu for this PR.

Comment on lines +134 to +138
// 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";

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know whether u are aware or not but S3StorageType.java in the S3 Gateway has similar enums means: “this S3 storage class string picks how many copies / EC layout the object gets” (STANDARD → 3-way Ratis, STANDARD_IA → EC, etc.). Almost all upload/download code uses that today.

This PR adds S3StorageClass, which means: “this string picks Hot / Warm / Cold (which disk tier).”

Those are two different ideas with similar names. That will confuse people and cause wrong imports later.

We should look into these and check how they both can be used independently.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AWS's StorageClass https://aws.amazon.com/s3/storage-classes does not explicitly specify the replica types or storage media types for each StorageClass; instead, it defines concepts such as durability, SLA, zones, and latency , etc for different StorageClasses

Ozone’s S3 StorageClasses can have custom definitions; users can specify the storage medium (storage policy) and replica type through S3 StorageClass parameter, which does not conflict with AWS S3’s StorageClasses.

try {
return S3StorageClass.valueOf(s3StorageClass.toUpperCase());
} catch (IllegalArgumentException e) {
throw new IllegalArgumentException(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IllegalArgumentException is fine for a util with no callers yet but PutObject must map unknown classes to OS3Exception / InvalidStorageClass, not a 500.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants