Copilot commented on code in PR #11122:
URL: https://github.com/apache/ozone/pull/11122#discussion_r4006773547
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java:
##########
@@ -1815,6 +1817,46 @@ public OmSnapshotManager getOmSnapshotManager() {
return omSnapshotManager;
}
+ /**
+ * Compute deleted bytes split for a bucket.
+ *
+ * @return stats containing snapshot-trapped bytes and purgeable bytes.
+ */
+ public BucketDeletedDataCalculator.BucketDeletedBytesStats
+ calculateDeletedBytesForBucket(String volume, String bucket)
+ throws IOException {
+ return new BucketDeletedDataCalculator(this).calculate(volume, bucket);
+ }
+
+ /**
+ * Compute deleted bytes split for a bucket path in {@code volume/bucket}
+ * format.
+ */
+ public BucketDeletedDataCalculator.BucketDeletedBytesStats
+ calculateDeletedBytesForBucket(String bucketPath)
+ throws IOException {
+ String[] tokens = bucketPath.split("/", 2);
+ if (tokens.length != 2 || tokens[0].isEmpty() || tokens[1].isEmpty()) {
+ throw new IllegalArgumentException(
+ "bucketPath must be in volume/bucket format: " + bucketPath);
+ }
+ return calculateDeletedBytesForBucket(tokens[0], tokens[1]);
+ }
+
+ @Override
+ public BucketDeletedBytes getBucketDeletedBytes(String bucketPath)
+ throws IOException {
+ BucketDeletedDataCalculator.BucketDeletedBytesStats stats =
+ calculateDeletedBytesForBucket(bucketPath);
Review Comment:
This new admin RPC does not call `checkAdminUserPrivilege`, unlike
`listOpenFiles` and the quota-repair admin methods. With admin authorization
enabled, any authenticated RPC principal can query arbitrary bucket paths and
obtain another tenant's deletion/snapshot metadata; enforce the admin check
before calculating.
##########
hadoop-ozone/interface-client/src/main/proto/OmClientProtocol.proto:
##########
@@ -167,6 +167,7 @@ enum Type {
GetLifecycleServiceStatus = 150;
SetLifecycleServiceStatus = 151;
SaveLifecycleScanState = 152;
+ GetBucketDeletedBytes = 153;
Review Comment:
`GetBucketDeletedBytes` is missing from both command classifiers in
`OmUtils`: `isReadOnly` defaults to false, so the server submits this command
through the Ratis/write flow instead of `handleReadRequest`, and
`shouldSendToFollower` logs it as uncategorized (the exhaustive `TestOmUtils`
check will also fail). Add it to the appropriate branches of both switches.
##########
hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/snapshot/trapped/TestBucketDeletedDataCalculator.java:
##########
@@ -0,0 +1,91 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.om.snapshot.trapped;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+import org.apache.hadoop.hdds.client.RatisReplicationConfig;
+import org.apache.hadoop.hdds.protocol.proto.HddsProtos;
+import org.apache.hadoop.hdds.utils.db.BatchOperation;
+import org.apache.hadoop.hdds.utils.db.Table;
+import org.apache.hadoop.ozone.om.BucketManager;
+import org.apache.hadoop.ozone.om.KeyManager;
+import org.apache.hadoop.ozone.om.helpers.OmBucketInfo;
+import org.apache.hadoop.ozone.om.helpers.OmKeyInfo;
+import org.apache.hadoop.ozone.om.helpers.RepeatedOmKeyInfo;
+import org.apache.hadoop.ozone.om.request.OMRequestTestUtils;
+import org.apache.hadoop.ozone.om.snapshot.SnapshotRequestAndResponseTests;
+import org.junit.jupiter.api.Test;
+
+/**
+ * Tests for {@link BucketDeletedDataCalculator}.
+ */
+public class TestBucketDeletedDataCalculator extends
SnapshotRequestAndResponseTests {
+
+ @Test
+ public void testAosDeletedKeyReportedAsPurgeable() throws Exception {
+ String volume = getVolumeName();
+ String bucket = getBucketName();
+ long bucketId = getOmMetadataManager().getBucketId(volume, bucket);
+
+ KeyManager keyManager = mock(KeyManager.class);
+ when(keyManager.getMetadataManager()).thenReturn(getOmMetadataManager());
+ when(keyManager.getDeletedDirEntries(volume,
bucket)).thenAnswer(invocation -> {
+ Table<String, OmKeyInfo> deletedDirTable =
getOmMetadataManager().getDeletedDirTable();
+ String prefix = getOmMetadataManager().getTableBucketPrefix(
+ deletedDirTable.getName(), volume, bucket);
+ return deletedDirTable.iterator(prefix);
+ });
+ when(getOzoneManager().getKeyManager()).thenReturn(keyManager);
+
+ BucketManager bucketManager = mock(BucketManager.class);
+ String bucketDbKey = getOmMetadataManager().getBucketKey(volume, bucket);
+ OmBucketInfo bucketInfo =
getOmMetadataManager().getBucketTable().get(bucketDbKey);
+ when(bucketManager.getBucketInfo(volume, bucket)).thenReturn(bucketInfo);
+ when(getOzoneManager().getBucketManager()).thenReturn(bucketManager);
+
+ OmKeyInfo keyInfo = OMRequestTestUtils.createOmKeyInfo(
+ volume, bucket, "key-a",
+
RatisReplicationConfig.getInstance(HddsProtos.ReplicationFactor.THREE))
+ .setObjectID(101L)
+ .setUpdateID(101L)
+ .build();
+ RepeatedOmKeyInfo repeated = new RepeatedOmKeyInfo(keyInfo, bucketId);
+
+ String deletedDbKey = getOmMetadataManager().getOzoneKey(volume, bucket,
keyInfo.getKeyName());
+ try (BatchOperation batch =
getOmMetadataManager().getStore().initBatchOperation()) {
+ getOmMetadataManager().getDeletedTable().putWithBatch(batch,
deletedDbKey, repeated);
+ getOmMetadataManager().getStore().commitBatchOperation(batch);
+ }
+
+ BucketDeletedDataCalculator.BucketDeletedBytesStats stats =
+ new BucketDeletedDataCalculator(getOzoneManager())
+ .calculate(volume, bucket);
Review Comment:
The only test populates an active object-store `deletedTable` entry and
asserts the purgeable path. It never creates an active snapshot/trapped key or
an FSO deleted directory, so the core snapshot-trapped and directory traversal
branches can regress without detection. Add focused cases for those paths, plus
coverage of the RPC routing if possible.
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/trapped/BucketDeletedDataCalculator.java:
##########
@@ -0,0 +1,289 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.om.snapshot.trapped;
+
+import java.io.IOException;
+import java.util.ArrayDeque;
+import java.util.HashSet;
+import java.util.Iterator;
+import java.util.Queue;
+import java.util.Set;
+import java.util.UUID;
+import org.apache.hadoop.hdds.utils.db.Table;
+import org.apache.hadoop.hdds.utils.db.Table.KeyValue;
+import org.apache.hadoop.hdds.utils.db.TableIterator;
+import org.apache.hadoop.ozone.om.KeyManager;
+import org.apache.hadoop.ozone.om.OMMetadataManager;
+import org.apache.hadoop.ozone.om.OmMetadataManagerImpl;
+import org.apache.hadoop.ozone.om.OmSnapshot;
+import org.apache.hadoop.ozone.om.OmSnapshotManager;
+import org.apache.hadoop.ozone.om.OzoneManager;
+import org.apache.hadoop.ozone.om.SnapshotChainManager;
+import org.apache.hadoop.ozone.om.helpers.OmKeyInfo;
+import org.apache.hadoop.ozone.om.helpers.RepeatedOmKeyInfo;
+import org.apache.hadoop.ozone.om.helpers.SnapshotInfo;
+import org.apache.hadoop.ozone.om.snapshot.SnapshotUtils;
+import org.apache.hadoop.ozone.om.snapshot.filter.ReclaimableDirFilter;
+import org.apache.hadoop.ozone.om.snapshot.filter.ReclaimableKeyFilter;
+import org.apache.ratis.util.function.UncheckedAutoCloseableSupplier;
+
+/**
+ * Computes bucket-level deleted bytes split into purgeable and
snapshot-trapped.
+ * <p>
+ * This calculator follows KDS/DDS reclaimability semantics across active OM DB
+ * and all active snapshots in the bucket chain.
+ */
+public class BucketDeletedDataCalculator {
+
+ private final OzoneManager ozoneManager;
+ private final OmSnapshotManager omSnapshotManager;
+ private final SnapshotChainManager snapshotChainManager;
+
+ public BucketDeletedDataCalculator(OzoneManager ozoneManager) {
+ this.ozoneManager = ozoneManager;
+ this.omSnapshotManager = ozoneManager.getOmSnapshotManager();
+ this.snapshotChainManager =
+ ((OmMetadataManagerImpl) ozoneManager.getMetadataManager())
+ .getSnapshotChainManager();
+ }
+
+ public BucketDeletedBytesStats calculate(String volume, String bucket)
+ throws IOException {
+ BucketDeletedBytesStats totals = new BucketDeletedBytesStats();
+ processStore(volume, bucket, null,
+ requireKeyManager(ozoneManager.getKeyManager(), "active store"),
totals);
+
+ Iterator<UUID> iterator = snapshotChainManager.iterator(true);
+ while (iterator.hasNext()) {
+ UUID snapshotId = iterator.next();
+ SnapshotInfo snapshotInfo = SnapshotUtils.getSnapshotInfo(
+ ozoneManager, snapshotChainManager, snapshotId);
+ if (snapshotInfo == null
+ || snapshotInfo.getSnapshotStatus() !=
SnapshotInfo.SnapshotStatus.SNAPSHOT_ACTIVE
+ || !snapshotInfo.getVolumeName().equals(volume)
+ || !snapshotInfo.getBucketName().equals(bucket)
+ || !OmSnapshotManager.areSnapshotChangesFlushedToDB(
Review Comment:
The calculator treats entries from every active snapshot as immediately
purgeable whenever the reclaimability filter returns true, but the actual
`KeyDeletingService` skips snapshot key processing until
`isDeepCleanedDeletedDir()` is true (see `KeyDeletingService.java:687-695`).
During that window this output can report purgeable bytes that the service will
not purge yet; either gate this state consistently or represent the pending
directory deep-clean state explicitly.
--
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]
