HDDS-12784. Add bucket object ID to lifecycle configuration proto definition #9255

ChenSammi · 2025-11-06T06:12:10Z

What changes were proposed in this pull request?

save bucket object ID in LifeCycleConfiguration, capable to detect same bucket name, actually different bucket case.

What is the link to the Apache JIRA

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

How was this patch tested?

new unit test

…inition

Copilot

Pull Request Overview

This PR adds bucket object ID tracking to lifecycle configurations to prevent stale lifecycle policies from being applied after bucket recreation. The main goal is to ensure that lifecycle configurations are invalidated when a bucket is deleted and recreated with the same name but a different object ID.

Key changes:

Added bucketObjectID field to OmLifecycleConfiguration to track the bucket instance
Modified lifecycle configuration validation to check bucket object ID matching
Updated bucket deletion to clean up associated lifecycle configurations

Reviewed Changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
OmClientProtocol.proto	Added `bucketObjectID` field to LifecycleConfiguration message
OmLifecycleConfiguration.java	Added `bucketObjectID` field with getter/setter, removed `@Immutable` annotation, updated serialization/deserialization
OMLifecycleConfigurationSetRequest.java	Sets `bucketObjectID` when creating/updating lifecycle configuration
OMLifecycleConfigurationSetResponse.java	Added getter method to expose `OmLifecycleConfiguration` for testing
KeyLifecycleService.java	Added validation to check bucket object ID matches before applying lifecycle policies
OMBucketDeleteResponse.java	Added lifecycle configuration table to cleanup tables list
S3LifecycleConfiguration.java	Added comment explaining why OzoneBucket doesn't have objectID info
TestOMLifecycleConfigurationSetRequest.java	Added test assertion to verify `bucketObjectID` is set correctly

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-11-06T06:21:17Z

...-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmLifecycleConfiguration.java

  public static final int LC_MAX_RULES = 1000;
  private final String volume;
  private final String bucket;
+  private long bucketObjectID = -1L;


The field bucketObjectID should be marked as final and initialized through the constructor. The current implementation with a setter method violates immutability principles and makes the class susceptible to being modified after construction, which can lead to thread-safety issues. If mutability is required, consider documenting why the @Immutable annotation was removed and the implications for thread safety.

Suggested change

private long bucketObjectID = -1L;

private final long bucketObjectID;

ivandika3

@ChenSammi Thanks for the improvement. Left come comments.

Since the OMBucketDeleteRequest will also delete the lifecycle configuration, technically the bucket object ID mismatch should not happen. However, it's good to double check.

...manager/src/main/java/org/apache/hadoop/ozone/om/response/bucket/OMBucketDeleteResponse.java

...in/java/org/apache/hadoop/ozone/om/request/lifecycle/OMLifecycleConfigurationSetRequest.java

...zone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/KeyLifecycleService.java

...-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmLifecycleConfiguration.java

ChenSammi · 2025-11-07T02:00:01Z

@ChenSammi Thanks for the improvement. Left come comments.

Since the OMBucketDeleteRequest will also delete the lifecycle configuration, technically the bucket object ID mismatch should not happen. However, it's good to double check.

@ivandika3 , this is mainly for upgrade-downgrade-upgrade consideration. After discuss with Xi, we plan to not introduce new OM layout feature. In case of upgrade-downgrade-upgrade, to prevent a bucket with LifecycleConfiguration created during upgrade, but not finalized, then system is downgraded, this LifecycleConfiguration will be left in the DB table, then later the cluster is upgraded again. If the bucket is deleted before that, and a new bucket with same name is created, then this new bucket should not be linked to the LifecycleConfiguration in the DB table. To detect this, we need object ID to compare weather it's the same bucket or a differnt bucket.

ivandika3 · 2025-11-07T02:46:14Z

@ChenSammi Thanks for the explanation. Makes sense.

ivandika3 · 2025-11-07T07:04:30Z

Error:  Tests run: 24, Failures: 0, Errors: 4, Skipped: 0, Time elapsed: 58.96 s <<< FAILURE! -- in org.apache.hadoop.ozone.om.service.TestKeyLifecycleService$Failing
Error:  org.apache.hadoop.ozone.om.service.TestKeyLifecycleService$Failing.testKeyDeletedOrRenamed(BucketLayout, boolean)[1] -- Time elapsed: 10.40 s <<< ERROR!

Seems some tests are timeout.

ivandika3

Thanks for the update. LGTM +1.

xichen01 · 2025-11-13T17:05:06Z

...-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmLifecycleConfiguration.java

        .setObjectID(getObjectID())
        .setUpdateID(getUpdateID());

+    if (bucketObjectID != null) {


Perhaps we can set bucketObjectID to -1 or some other special value, so that the bucketObjectID of the version containing the Lifecycle feature will definitely have one valid bucket ObjectID (it is not -1), ultimately achieving the same effect.

This conflicts with my review #9255 (comment) , IMO it's better to explicitly set to null.

xichen01 · 2025-11-13T17:13:28Z

...ne/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/S3LifecycleConfiguration.java

    }
  }

+  // OzoneBucket doesn't have objectID info.


This seems to be fine; OM itself knows the bucket's ObjectID.

Yeah, this is the reason that OmLifecycleConfiguration object created by this function doesn't have bucket's objectID set because it doesn't know the ID from OzoneBucket. I put this comment here to help reminder in case I forget later.

xichen01 · 2025-11-13T17:16:15Z

...in/java/org/apache/hadoop/ozone/om/request/lifecycle/OMLifecycleConfigurationSetRequest.java

+      omLifecycleConfiguration.valid();
+      auditMap = omLifecycleConfiguration.toAuditMap();
+
+      omLifecycleConfiguration.setUpdateID(transactionLogIndex);


Seems the OmLifecycleConfiguration is not a @Immutable, since it can be modified, maybe we should remove the @immutable on the OmLifecycleConfiguration

We can remove the @immutable in next path, where I plan to add an option to enable/disable move to trash support.

OmLifecycleConfiguration inherits mutability from its parent class WithObjectID. Still, we should strive to eliminate mutability, not introduce further. Please prefer creating modified copy via the builder.

ChenSammi · 2025-11-17T04:04:25Z

Thanks @ivandika3 and @xichen01 for the review.

HDDS-12784. Add bucket object ID to lifecycle configuration proto def…

cc5ae3a

…inition

ChenSammi requested review from Copilot, ivandika3 and xichen01 November 6, 2025 06:12

Copilot AI reviewed Nov 6, 2025

View reviewed changes

ivandika3 added the s3-lifecycle HDDS-8342 label Nov 7, 2025

ivandika3 reviewed Nov 7, 2025

View reviewed changes

address comments and fix failed UT

bb28c49

ivandika3 approved these changes Nov 12, 2025

View reviewed changes

xichen01 reviewed Nov 13, 2025

View reviewed changes

ChenSammi merged commit 3b5cb65 into apache:HDDS-8342 Nov 17, 2025
83 of 84 checks passed

	private long bucketObjectID = -1L;
	private final long bucketObjectID;

HDDS-12784. Add bucket object ID to lifecycle configuration proto definition #9255

HDDS-12784. Add bucket object ID to lifecycle configuration proto definition #9255

Uh oh!

Conversation

ChenSammi commented Nov 6, 2025

What changes were proposed in this pull request?

What is the link to the Apache JIRA

How was this patch tested?

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Nov 6, 2025

Choose a reason for hiding this comment

Uh oh!

ivandika3 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ChenSammi commented Nov 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ivandika3 commented Nov 7, 2025

Uh oh!

ivandika3 commented Nov 7, 2025

Uh oh!

ivandika3 left a comment

Choose a reason for hiding this comment

Uh oh!

xichen01 Nov 13, 2025

Choose a reason for hiding this comment

Uh oh!

ivandika3 Nov 14, 2025

Choose a reason for hiding this comment

Uh oh!

xichen01 Nov 13, 2025

Choose a reason for hiding this comment

Uh oh!

ChenSammi Nov 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

xichen01 Nov 13, 2025

Choose a reason for hiding this comment

Uh oh!

ChenSammi Nov 14, 2025

Choose a reason for hiding this comment

Uh oh!

adoroszlai Nov 14, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ChenSammi commented Nov 17, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

ChenSammi commented Nov 7, 2025 •

edited

Loading

ChenSammi Nov 14, 2025 •

edited

Loading