You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by "swamirishi (via GitHub)" <gi...@apache.org> on 2025/06/03 09:35:57 UTC
[PR] HDDS-13170. Reclaimable filter should always reclaim entries when buckets and volumes have already been deleted [ozone]
swamirishi opened a new pull request, #8551:
URL: https://github.com/apache/ozone/pull/8551
## What changes were proposed in this pull request?
Currently ReclaimableFilter expects the bucket info to be present while initializing the snapshot chain but it could so happen that the bucket could have been deleted with all the entries belonging to the bucket present in the deletedTable & deleted directory table. Reclaimable filter should always return a true if the bucket or volume is not present.
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-13170
## How was this patch tested?
Additional unit tests
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-13170. ReclaimableFilter should apply if bucket or volume already deleted [ozone]
Posted by "swamirishi (via GitHub)" <gi...@apache.org>.
swamirishi commented on code in PR #8551:
URL: https://github.com/apache/ozone/pull/8551#discussion_r2127053199
##########
hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/snapshot/filter/AbstractReclaimableFilterTest.java:
##########
@@ -159,19 +161,28 @@ protected void teardown() throws IOException {
private void mockOzoneManager(BucketLayout bucketLayout) throws IOException {
OMMetadataManager metadataManager = mock(OMMetadataManager.class);
+ BucketManager bucketManager = mock(BucketManager.class);
when(ozoneManager.getMetadataManager()).thenReturn(metadataManager);
+ when(ozoneManager.getBucketManager()).thenReturn(bucketManager);
long volumeCount = 0;
- long bucketCount = 0;
for (String volume : volumes) {
when(metadataManager.getVolumeId(eq(volume))).thenReturn(volumeCount);
- for (String bucket : buckets) {
- when(ozoneManager.getBucketInfo(eq(volume), eq(bucket)))
- .thenReturn(OmBucketInfo.newBuilder().setVolumeName(volume).setBucketName(bucket)
- .setObjectID(bucketCount).setBucketLayout(bucketLayout).build());
- bucketCount++;
- }
volumeCount++;
}
+
+ when(bucketManager.getBucketInfo(anyString(), anyString())).thenAnswer(i -> {
+ String volume = i.getArgument(0, String.class);
+ String bucket = i.getArgument(1, String.class);
+ if (!volumes.contains(volume)) {
+ throw new OMException("Volume " + volume + " already exists", OMException.ResultCodes.VOLUME_NOT_FOUND);
Review Comment:
done yeah this was a miss on my part.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-13170. ReclaimableFilter should apply if bucket or volume already deleted [ozone]
Posted by "jojochuang (via GitHub)" <gi...@apache.org>.
jojochuang commented on code in PR #8551:
URL: https://github.com/apache/ozone/pull/8551#discussion_r2126902377
##########
hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/snapshot/filter/AbstractReclaimableFilterTest.java:
##########
@@ -159,19 +161,28 @@ protected void teardown() throws IOException {
private void mockOzoneManager(BucketLayout bucketLayout) throws IOException {
OMMetadataManager metadataManager = mock(OMMetadataManager.class);
+ BucketManager bucketManager = mock(BucketManager.class);
when(ozoneManager.getMetadataManager()).thenReturn(metadataManager);
+ when(ozoneManager.getBucketManager()).thenReturn(bucketManager);
long volumeCount = 0;
- long bucketCount = 0;
for (String volume : volumes) {
when(metadataManager.getVolumeId(eq(volume))).thenReturn(volumeCount);
- for (String bucket : buckets) {
- when(ozoneManager.getBucketInfo(eq(volume), eq(bucket)))
- .thenReturn(OmBucketInfo.newBuilder().setVolumeName(volume).setBucketName(bucket)
- .setObjectID(bucketCount).setBucketLayout(bucketLayout).build());
- bucketCount++;
- }
volumeCount++;
}
+
+ when(bucketManager.getBucketInfo(anyString(), anyString())).thenAnswer(i -> {
+ String volume = i.getArgument(0, String.class);
+ String bucket = i.getArgument(1, String.class);
+ if (!volumes.contains(volume)) {
+ throw new OMException("Volume " + volume + " already exists", OMException.ResultCodes.VOLUME_NOT_FOUND);
Review Comment:
Looks like the exception message needs an update. See Gemini's review comment: https://github.com/jojochuang/ozone/pull/568#discussion_r2125748884
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-13170. ReclaimableFilter should apply if bucket or volume already deleted [ozone]
Posted by "jojochuang (via GitHub)" <gi...@apache.org>.
jojochuang commented on code in PR #8551:
URL: https://github.com/apache/ozone/pull/8551#discussion_r2126902377
##########
hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/snapshot/filter/AbstractReclaimableFilterTest.java:
##########
@@ -159,19 +161,28 @@ protected void teardown() throws IOException {
private void mockOzoneManager(BucketLayout bucketLayout) throws IOException {
OMMetadataManager metadataManager = mock(OMMetadataManager.class);
+ BucketManager bucketManager = mock(BucketManager.class);
when(ozoneManager.getMetadataManager()).thenReturn(metadataManager);
+ when(ozoneManager.getBucketManager()).thenReturn(bucketManager);
long volumeCount = 0;
- long bucketCount = 0;
for (String volume : volumes) {
when(metadataManager.getVolumeId(eq(volume))).thenReturn(volumeCount);
- for (String bucket : buckets) {
- when(ozoneManager.getBucketInfo(eq(volume), eq(bucket)))
- .thenReturn(OmBucketInfo.newBuilder().setVolumeName(volume).setBucketName(bucket)
- .setObjectID(bucketCount).setBucketLayout(bucketLayout).build());
- bucketCount++;
- }
volumeCount++;
}
+
+ when(bucketManager.getBucketInfo(anyString(), anyString())).thenAnswer(i -> {
+ String volume = i.getArgument(0, String.class);
+ String bucket = i.getArgument(1, String.class);
+ if (!volumes.contains(volume)) {
+ throw new OMException("Volume " + volume + " already exists", OMException.ResultCodes.VOLUME_NOT_FOUND);
Review Comment:
Looks like the exception message needs to update to " not found". See Gemini's review comment: https://github.com/jojochuang/ozone/pull/568#discussion_r2125748884
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-13170. Reclaimable filter should always reclaim entries when buckets and volumes have already been deleted [ozone]
Posted by "swamirishi (via GitHub)" <gi...@apache.org>.
swamirishi commented on PR #8551:
URL: https://github.com/apache/ozone/pull/8551#issuecomment-2942538257
Thank you for reviewing the patch @jojochuang
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-13170. Reclaimable filter should always reclaim entries when buckets and volumes have already been deleted [ozone]
Posted by "jojochuang (via GitHub)" <gi...@apache.org>.
jojochuang commented on code in PR #8551:
URL: https://github.com/apache/ozone/pull/8551#discussion_r2127546189
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/filter/ReclaimableFilter.java:
##########
@@ -167,11 +168,21 @@ private void initializePreviousSnapshotsFromChain(String volume, String bucket)
previousOmSnapshots.add(null);
previousSnapshotInfos.add(null);
}
-
- // NOTE: Getting volumeId and bucket from active OM.
- // This would be wrong on volume & bucket renames support.
- bucketInfo = ozoneManager.getBucketInfo(volume, bucket);
+ }
+ // NOTE: Getting volumeId and bucket from active OM.
+ // This would be wrong on volume & bucket renames support.
Review Comment:
this is not supported and is a design decision, so I think we can safely ignore it.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-13170. Reclaimable filter should always reclaim entries when buckets and volumes have already been deleted [ozone]
Posted by "swamirishi (via GitHub)" <gi...@apache.org>.
swamirishi merged PR #8551:
URL: https://github.com/apache/ozone/pull/8551
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org