This is an automated email from the ASF dual-hosted git repository.
jojochuang pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/ozone.git
The following commit(s) were added to refs/heads/master by this push:
new 7196f98d860 HDDS-15951. Reduce ofs getFileChecksum to a single OM RPC
(#11380)
7196f98d860 is described below
commit 7196f98d8601827755bb338bd9aa8d24a7586e95
Author: Andrey Yarovoy <[email protected]>
AuthorDate: Thu Oct 8 12:54:44 2026 -0400
HDDS-15951. Reduce ofs getFileChecksum to a single OM RPC (#11380)
Generated-by: Claude Code (claude-opus)
---
.../apache/hadoop/ozone/OzoneManagerVersion.java | 5 +
.../ozone/shell/keys/ChecksumKeyHandler.java | 5 +-
.../hadoop/ozone/client/OzoneClientUtils.java | 23 +-
.../client/checksum/BaseFileChecksumHelper.java | 19 +-
.../client/checksum/ChecksumHelperFactory.java | 10 +-
.../client/checksum/ECFileChecksumHelper.java | 6 +-
.../checksum/ReplicatedFileChecksumHelper.java | 6 +-
.../hadoop/ozone/client/TestOzoneClientUtils.java | 12 +-
.../client/checksum/FileChecksumBenchmark.java | 9 +-
.../client/checksum/TestFileChecksumHelper.java | 26 +-
.../ozone/AbstractRootedOzoneFileSystemTest.java | 61 ++++
.../ozone/TestOfsGetFileChecksumRpcBenchmark.java | 314 +++++++++++++++++++++
.../om/snapshot/TestOzoneManagerSnapshotAcl.java | 30 +-
.../java/org/apache/hadoop/ozone/om/OMMetrics.java | 5 +
.../apache/hadoop/ozone/om/OmMetadataReader.java | 12 +
.../hadoop/ozone/om/TestOMMetadataReader.java | 60 ++++
.../fs/ozone/BasicOzoneClientAdapterImpl.java | 6 +-
.../ozone/BasicRootedOzoneClientAdapterImpl.java | 46 ++-
...sicRootedOzoneClientAdapterGetFileChecksum.java | 199 +++++++++++++
19 files changed, 775 insertions(+), 79 deletions(-)
diff --git
a/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneManagerVersion.java
b/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneManagerVersion.java
index 9bd041c615c..f33274dde83 100644
---
a/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneManagerVersion.java
+++
b/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneManagerVersion.java
@@ -67,6 +67,11 @@ public enum OzoneManagerVersion implements ComponentVersion {
+ "buckets server-side, so file system clients no longer need the "
+ "client-side InfoBucket layout check"),
+ LOOKUP_FILE_REJECTS_OBS(15,
+ "OzoneManager version that rejects lookupFile on OBJECT_STORE buckets "
+ + "server-side, so file system clients no longer need the "
+ + "client-side InfoBucket layout check for getFileChecksum"),
+
FUTURE_VERSION(-1, "Used internally in the client when the server side is "
+ " newer and an unknown server version has arrived to the client.");
diff --git
a/hadoop-ozone/cli-shell/src/main/java/org/apache/hadoop/ozone/shell/keys/ChecksumKeyHandler.java
b/hadoop-ozone/cli-shell/src/main/java/org/apache/hadoop/ozone/shell/keys/ChecksumKeyHandler.java
index c36c1af9910..cecbb8cc020 100644
---
a/hadoop-ozone/cli-shell/src/main/java/org/apache/hadoop/ozone/shell/keys/ChecksumKeyHandler.java
+++
b/hadoop-ozone/cli-shell/src/main/java/org/apache/hadoop/ozone/shell/keys/ChecksumKeyHandler.java
@@ -59,8 +59,9 @@ protected void execute(OzoneClient client, OzoneAddress
address)
protected FileChecksum getFileChecksum(OzoneVolume vol, OzoneBucket bucket,
String keyName, long dataSize, ClientProtocol clientProxy)
throws IOException {
- return getFileChecksumWithCombineMode(vol, bucket, keyName, dataSize, mode,
- clientProxy);
+ // LookupKey, not LookupFile: the CLI must keep working on OBJECT_STORE
buckets.
+ return getFileChecksumWithCombineMode(vol.getName(), bucket.getName(),
keyName, dataSize, mode,
+ clientProxy, false);
}
/**
diff --git
a/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/OzoneClientUtils.java
b/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/OzoneClientUtils.java
index 0d5504bb167..47a13a04722 100644
---
a/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/OzoneClientUtils.java
+++
b/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/OzoneClientUtils.java
@@ -44,6 +44,7 @@
import org.apache.hadoop.ozone.om.helpers.BucketLayout;
import org.apache.hadoop.ozone.om.helpers.OmKeyArgs;
import org.apache.hadoop.ozone.om.helpers.OmKeyInfo;
+import org.apache.hadoop.ozone.om.protocol.OzoneManagerProtocol;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -220,23 +221,31 @@ decision of finding the replication config( either from
bucket defaults
.parse(clientReplicationType, clientReplication, clientSideConfig);
}
- public static FileChecksum getFileChecksumWithCombineMode(OzoneVolume volume,
- OzoneBucket bucket, String keyName, long length,
+ /**
+ * Computes the checksum of a key.
+ *
+ * @param useLookupFile when true the key is read with {@code lookupFile},
which rejects OBJECT_STORE buckets
+ * server-side; when false {@code lookupKey} is used, which accepts any
bucket layout
+ */
+ @SuppressWarnings("checkstyle:ParameterNumber")
+ public static FileChecksum getFileChecksumWithCombineMode(String volumeName,
+ String bucketName, String keyName, long length,
OzoneClientConfig.ChecksumCombineMode combineMode,
- ClientProtocol rpcClient) throws IOException {
+ ClientProtocol rpcClient, boolean useLookupFile) throws IOException {
Preconditions.checkArgument(length >= 0);
if (keyName.isEmpty()) {
return null;
}
- OmKeyArgs keyArgs = new OmKeyArgs.Builder().setVolumeName(volume.getName())
- .setBucketName(bucket.getName()).setKeyName(keyName)
+ OmKeyArgs keyArgs = new OmKeyArgs.Builder().setVolumeName(volumeName)
+ .setBucketName(bucketName).setKeyName(keyName)
.setSortDatanodesInPipeline(true)
.setLatestVersionLocation(true).build();
- OmKeyInfo keyInfo = rpcClient.getOzoneManagerClient().lookupKey(keyArgs);
+ OzoneManagerProtocol omClient = rpcClient.getOzoneManagerClient();
+ OmKeyInfo keyInfo = useLookupFile ? omClient.lookupFile(keyArgs) :
omClient.lookupKey(keyArgs);
BaseFileChecksumHelper helper = ChecksumHelperFactory
.getChecksumHelper(keyInfo.getReplicationConfig().getReplicationType(),
- volume, bucket, keyName, length, combineMode, rpcClient, keyInfo);
+ volumeName, bucketName, keyName, length, combineMode, rpcClient,
keyInfo);
helper.compute();
return helper.getFileChecksum();
}
diff --git
a/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/checksum/BaseFileChecksumHelper.java
b/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/checksum/BaseFileChecksumHelper.java
index da85d4d3e17..7c38b2b5049 100644
---
a/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/checksum/BaseFileChecksumHelper.java
+++
b/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/checksum/BaseFileChecksumHelper.java
@@ -30,8 +30,6 @@
import org.apache.hadoop.hdds.scm.XceiverClientFactory;
import org.apache.hadoop.io.DataOutputBuffer;
import org.apache.hadoop.io.MD5Hash;
-import org.apache.hadoop.ozone.client.OzoneBucket;
-import org.apache.hadoop.ozone.client.OzoneVolume;
import org.apache.hadoop.ozone.client.protocol.ClientProtocol;
import org.apache.hadoop.ozone.client.rpc.RpcClient;
import org.apache.hadoop.ozone.om.helpers.OmKeyArgs;
@@ -50,8 +48,8 @@ public abstract class BaseFileChecksumHelper {
LoggerFactory.getLogger(BaseFileChecksumHelper.class);
private OmKeyInfo keyInfo;
- private OzoneVolume volume;
- private OzoneBucket bucket;
+ private String volumeName;
+ private String bucketName;
private String keyName;
private final long length;
@@ -68,12 +66,12 @@ public abstract class BaseFileChecksumHelper {
private long crcPerBlock = 0;
// initialization
- public BaseFileChecksumHelper(OzoneVolume volume, OzoneBucket bucket,
+ public BaseFileChecksumHelper(String volumeName, String bucketName,
String keyName, long length,
OzoneClientConfig.ChecksumCombineMode checksumCombineMode,
ClientProtocol rpcClient, OmKeyInfo keyInfo) throws IOException {
- this.volume = volume;
- this.bucket = bucket;
+ this.volumeName = volumeName;
+ this.bucketName = bucketName;
this.keyName = keyName;
this.length = length;
this.combineMode = checksumCombineMode;
@@ -87,8 +85,7 @@ public BaseFileChecksumHelper(OzoneVolume volume, OzoneBucket
bucket,
}
protected String getSrc() {
- return "Volume: " + volume.getName() + " Bucket: " + bucket.getName() + " "
- + keyName;
+ return "Volume: " + volumeName + " Bucket: " + bucketName + " " + keyName;
}
protected long getLength() {
@@ -268,8 +265,8 @@ private void fetchBlocks() throws IOException {
OzoneManagerProtocol ozoneManagerClient =
getRpcClient().getOzoneManagerClient();
OmKeyArgs keyArgs =
- new OmKeyArgs.Builder().setVolumeName(volume.getName())
- .setBucketName(bucket.getName()).setKeyName(keyName)
+ new OmKeyArgs.Builder().setVolumeName(volumeName)
+ .setBucketName(bucketName).setKeyName(keyName)
.setSortDatanodesInPipeline(true)
.setLatestVersionLocation(true).build();
keyInfo = ozoneManagerClient.lookupKey(keyArgs);
diff --git
a/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/checksum/ChecksumHelperFactory.java
b/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/checksum/ChecksumHelperFactory.java
index 1c85c9f91d1..333231b27d2 100644
---
a/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/checksum/ChecksumHelperFactory.java
+++
b/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/checksum/ChecksumHelperFactory.java
@@ -20,8 +20,6 @@
import java.io.IOException;
import org.apache.hadoop.hdds.protocol.proto.HddsProtos;
import org.apache.hadoop.hdds.scm.OzoneClientConfig;
-import org.apache.hadoop.ozone.client.OzoneBucket;
-import org.apache.hadoop.ozone.client.OzoneVolume;
import org.apache.hadoop.ozone.client.protocol.ClientProtocol;
import org.apache.hadoop.ozone.om.helpers.OmKeyInfo;
@@ -35,16 +33,16 @@ private ChecksumHelperFactory() {
@SuppressWarnings("checkstyle:ParameterNumber")
public static BaseFileChecksumHelper getChecksumHelper(
- HddsProtos.ReplicationType replicationType, OzoneVolume volume,
- OzoneBucket bucket, String keyName, long length,
+ HddsProtos.ReplicationType replicationType, String volumeName,
+ String bucketName, String keyName, long length,
OzoneClientConfig.ChecksumCombineMode combineMode,
ClientProtocol rpcClient, OmKeyInfo keyInfo) throws IOException {
if (replicationType == HddsProtos.ReplicationType.EC) {
- return new ECFileChecksumHelper(volume, bucket, keyName, length,
+ return new ECFileChecksumHelper(volumeName, bucketName, keyName, length,
combineMode, rpcClient, keyInfo);
} else {
- return new ReplicatedFileChecksumHelper(volume, bucket, keyName, length,
+ return new ReplicatedFileChecksumHelper(volumeName, bucketName, keyName,
length,
combineMode, rpcClient, keyInfo);
}
}
diff --git
a/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/checksum/ECFileChecksumHelper.java
b/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/checksum/ECFileChecksumHelper.java
index faaa1e7725c..3d8f89614e9 100644
---
a/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/checksum/ECFileChecksumHelper.java
+++
b/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/checksum/ECFileChecksumHelper.java
@@ -37,8 +37,6 @@
import org.apache.hadoop.hdds.scm.pipeline.PipelineID;
import org.apache.hadoop.hdds.scm.storage.ContainerProtocolCalls;
import org.apache.hadoop.hdds.security.token.OzoneBlockTokenIdentifier;
-import org.apache.hadoop.ozone.client.OzoneBucket;
-import org.apache.hadoop.ozone.client.OzoneVolume;
import org.apache.hadoop.ozone.client.protocol.ClientProtocol;
import org.apache.hadoop.ozone.om.helpers.OmKeyInfo;
import org.apache.hadoop.ozone.om.helpers.OmKeyLocationInfo;
@@ -49,11 +47,11 @@
*/
public class ECFileChecksumHelper extends BaseFileChecksumHelper {
- public ECFileChecksumHelper(OzoneVolume volume, OzoneBucket bucket,
+ public ECFileChecksumHelper(String volumeName, String bucketName,
String keyName, long length, OzoneClientConfig.ChecksumCombineMode
checksumCombineMode, ClientProtocol rpcClient, OmKeyInfo keyInfo)
throws IOException {
- super(volume, bucket, keyName, length, checksumCombineMode, rpcClient,
+ super(volumeName, bucketName, keyName, length, checksumCombineMode,
rpcClient,
keyInfo);
}
diff --git
a/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/checksum/ReplicatedFileChecksumHelper.java
b/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/checksum/ReplicatedFileChecksumHelper.java
index 42a07690a3f..7dcc7fc94f6 100644
---
a/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/checksum/ReplicatedFileChecksumHelper.java
+++
b/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/checksum/ReplicatedFileChecksumHelper.java
@@ -26,8 +26,6 @@
import org.apache.hadoop.hdds.scm.pipeline.Pipeline;
import org.apache.hadoop.hdds.scm.storage.ContainerProtocolCalls;
import org.apache.hadoop.hdds.security.token.OzoneBlockTokenIdentifier;
-import org.apache.hadoop.ozone.client.OzoneBucket;
-import org.apache.hadoop.ozone.client.OzoneVolume;
import org.apache.hadoop.ozone.client.protocol.ClientProtocol;
import org.apache.hadoop.ozone.om.helpers.OmKeyInfo;
import org.apache.hadoop.ozone.om.helpers.OmKeyLocationInfo;
@@ -38,11 +36,11 @@
*/
public class ReplicatedFileChecksumHelper extends BaseFileChecksumHelper {
- public ReplicatedFileChecksumHelper(OzoneVolume volume, OzoneBucket bucket,
+ public ReplicatedFileChecksumHelper(String volumeName, String bucketName,
String keyName, long length,
OzoneClientConfig.ChecksumCombineMode checksumCombineMode,
ClientProtocol rpcClient, OmKeyInfo keyInfo) throws IOException {
- super(volume, bucket, keyName, length, checksumCombineMode, rpcClient,
+ super(volumeName, bucketName, keyName, length, checksumCombineMode,
rpcClient,
keyInfo);
}
diff --git
a/hadoop-ozone/client/src/test/java/org/apache/hadoop/ozone/client/TestOzoneClientUtils.java
b/hadoop-ozone/client/src/test/java/org/apache/hadoop/ozone/client/TestOzoneClientUtils.java
index c2154fe880b..b4923c536ad 100644
---
a/hadoop-ozone/client/src/test/java/org/apache/hadoop/ozone/client/TestOzoneClientUtils.java
+++
b/hadoop-ozone/client/src/test/java/org/apache/hadoop/ozone/client/TestOzoneClientUtils.java
@@ -48,26 +48,22 @@ public class TestOzoneClientUtils {
@Test()
public void testNegativeLength() throws IOException {
- OzoneVolume volume = mock(OzoneVolume.class);
- OzoneBucket bucket = mock(OzoneBucket.class);
String keyName = "dummy";
ClientProtocol clientProtocol = mock(ClientProtocol.class);
assertThrows(IllegalArgumentException.class, () ->
- OzoneClientUtils.getFileChecksumWithCombineMode(volume, bucket,
keyName,
+ OzoneClientUtils.getFileChecksumWithCombineMode("vol", "bucket",
keyName,
-1, OzoneClientConfig.ChecksumCombineMode.MD5MD5CRC,
- clientProtocol));
+ clientProtocol, false));
}
@Test
public void testEmptyKeyName() throws IOException {
- OzoneVolume volume = mock(OzoneVolume.class);
- OzoneBucket bucket = mock(OzoneBucket.class);
String keyName = "";
ClientProtocol clientProtocol = mock(ClientProtocol.class);
FileChecksum checksum =
- OzoneClientUtils.getFileChecksumWithCombineMode(volume, bucket,
keyName,
+ OzoneClientUtils.getFileChecksumWithCombineMode("vol", "bucket",
keyName,
1, OzoneClientConfig.ChecksumCombineMode.MD5MD5CRC,
- clientProtocol);
+ clientProtocol, false);
assertNull(checksum);
}
diff --git
a/hadoop-ozone/client/src/test/java/org/apache/hadoop/ozone/client/checksum/FileChecksumBenchmark.java
b/hadoop-ozone/client/src/test/java/org/apache/hadoop/ozone/client/checksum/FileChecksumBenchmark.java
index 6af4af3d839..1996a846173 100644
---
a/hadoop-ozone/client/src/test/java/org/apache/hadoop/ozone/client/checksum/FileChecksumBenchmark.java
+++
b/hadoop-ozone/client/src/test/java/org/apache/hadoop/ozone/client/checksum/FileChecksumBenchmark.java
@@ -49,8 +49,6 @@
import org.apache.hadoop.hdds.scm.XceiverClientSpi;
import org.apache.hadoop.hdds.scm.pipeline.Pipeline;
import org.apache.hadoop.hdds.scm.pipeline.PipelineID;
-import org.apache.hadoop.ozone.client.OzoneBucket;
-import org.apache.hadoop.ozone.client.OzoneVolume;
import org.apache.hadoop.ozone.client.rpc.RpcClient;
import org.apache.hadoop.ozone.om.helpers.OmKeyArgs;
import org.apache.hadoop.ozone.om.helpers.OmKeyInfo;
@@ -323,11 +321,6 @@ private static BenchmarkResult measure(int latencyMs,
ECReplicationConfig repCon
when(mockRpcClient.getOzoneManagerClient()).thenReturn(mockOm);
when(mockRpcClient.getXceiverClientManager()).thenReturn(xceiverFactory);
- OzoneVolume mockVolume = mock(OzoneVolume.class);
- when(mockVolume.getName()).thenReturn("vol");
- OzoneBucket mockBucket = mock(OzoneBucket.class);
- when(mockBucket.getName()).thenReturn("bucket");
-
OzoneClientConfig.ChecksumCombineMode combineMode =
OzoneClientConfig.ChecksumCombineMode.COMPOSITE_CRC;
@@ -344,7 +337,7 @@ private static BenchmarkResult measure(int latencyMs,
ECReplicationConfig repCon
.setLatestVersionLocation(true)
.build());
new ECFileChecksumHelper(
- mockVolume, mockBucket, keyName, FILE_SIZE, combineMode,
+ "vol", "bucket", keyName, FILE_SIZE, combineMode,
mockRpcClient, keyInfo)
.compute();
} catch (IOException e) {
diff --git
a/hadoop-ozone/client/src/test/java/org/apache/hadoop/ozone/client/checksum/TestFileChecksumHelper.java
b/hadoop-ozone/client/src/test/java/org/apache/hadoop/ozone/client/checksum/TestFileChecksumHelper.java
index 42243ae2d38..fa02ebb91b6 100644
---
a/hadoop-ozone/client/src/test/java/org/apache/hadoop/ozone/client/checksum/TestFileChecksumHelper.java
+++
b/hadoop-ozone/client/src/test/java/org/apache/hadoop/ozone/client/checksum/TestFileChecksumHelper.java
@@ -141,13 +141,13 @@ private OmKeyInfo omKeyInfo(ReplicationType type,
FileChecksum cachedChecksum, L
.build();
}
- private BaseFileChecksumHelper checksumHelper(ReplicationType type,
OzoneVolume mockVolume, OzoneBucket mockBucket,
+ private BaseFileChecksumHelper checksumHelper(ReplicationType type, String
volumeName, String bucketName,
int length, OzoneClientConfig.ChecksumCombineMode combineMode, RpcClient
mockRpcClient, OmKeyInfo keyInfo)
throws IOException {
return type == ReplicationType.RATIS ? new ReplicatedFileChecksumHelper(
- mockVolume, mockBucket, "dummy", length, combineMode, mockRpcClient,
keyInfo)
+ volumeName, bucketName, "dummy", length, combineMode, mockRpcClient,
keyInfo)
: new ECFileChecksumHelper(
- mockVolume, mockBucket, "dummy", length, combineMode, mockRpcClient,
keyInfo);
+ volumeName, bucketName, "dummy", length, combineMode, mockRpcClient,
keyInfo);
}
private Pipeline pipeline(ReplicationType type, List<DatanodeDetails>
datanodeDetails) {
@@ -169,20 +169,15 @@ public void testEmptyBlock(ReplicationType helperType)
throws IOException {
// test the file checksum of a file with an empty block.
RpcClient mockRpcClient = mock(RpcClient.class);
OmKeyInfo omKeyInfo = omKeyInfo(helperType, noCachedChecksum, new
ArrayList<>());
- OzoneVolume mockVolume = mock(OzoneVolume.class);
- OzoneBucket mockBucket = mock(OzoneBucket.class);
OzoneClientConfig.ChecksumCombineMode combineMode =
OzoneClientConfig.ChecksumCombineMode.MD5MD5CRC;
OzoneManagerProtocol om = mock(OzoneManagerProtocol.class);
when(mockRpcClient.getOzoneManagerClient()).thenReturn(om);
when(om.lookupKey(any())).thenReturn(omKeyInfo);
- when(mockVolume.getName()).thenReturn("vol1");
- when(mockBucket.getName()).thenReturn("bucket1");
-
BaseFileChecksumHelper helper =
- checksumHelper(helperType, mockVolume, mockBucket, 10, combineMode,
mockRpcClient, omKeyInfo);
+ checksumHelper(helperType, "vol1", "bucket1", 10, combineMode,
mockRpcClient, omKeyInfo);
helper.compute();
FileChecksum fileChecksum = helper.getFileChecksum();
assertInstanceOf(MD5MD5CRC32GzipFileChecksum.class, fileChecksum);
@@ -191,7 +186,7 @@ public void testEmptyBlock(ReplicationType helperType)
throws IOException {
// test negative length
helper =
- checksumHelper(helperType, mockVolume, mockBucket, -1, combineMode,
mockRpcClient, omKeyInfo);
+ checksumHelper(helperType, "vol1", "bucket1", -1, combineMode,
mockRpcClient, omKeyInfo);
helper.compute();
assertNull(helper.getKeyLocationInfoList());
}
@@ -231,16 +226,11 @@ public XceiverClientReply sendCommandAsync(
when(mockRpcClient.getOzoneManagerClient()).thenReturn(om);
when(om.lookupKey(any())).thenReturn(omKeyInfo);
- OzoneVolume mockVolume = mock(OzoneVolume.class);
- when(mockVolume.getName()).thenReturn("vol1");
- OzoneBucket mockBucket = mock(OzoneBucket.class);
- when(mockBucket.getName()).thenReturn("bucket1");
-
OzoneClientConfig.ChecksumCombineMode combineMode =
OzoneClientConfig.ChecksumCombineMode.MD5MD5CRC;
BaseFileChecksumHelper helper = checksumHelper(
- helperType, mockVolume, mockBucket, 10, combineMode, mockRpcClient,
omKeyInfo);
+ helperType, "vol1", "bucket1", 10, combineMode, mockRpcClient,
omKeyInfo);
helper.compute();
FileChecksum fileChecksum = helper.getFileChecksum();
@@ -254,7 +244,7 @@ public XceiverClientReply sendCommandAsync(
thenReturn(omKeyInfoWithChecksum);
helper = checksumHelper(
- helperType, mockVolume, mockBucket, 10, combineMode, mockRpcClient,
omKeyInfo);
+ helperType, "vol1", "bucket1", 10, combineMode, mockRpcClient,
omKeyInfo);
helper.compute();
fileChecksum = helper.getFileChecksum();
@@ -349,7 +339,7 @@ public void testPutKeyChecksum() throws IOException {
OmKeyInfo keyInfo = rpcClient.getKeyInfo(
volume.getName(), bucket.getName(), keyName, false);
ReplicatedFileChecksumHelper helper = new ReplicatedFileChecksumHelper(
- volume, bucket, keyName, 10, combineMode, rpcClient, keyInfo);
+ volume.getName(), bucket.getName(), keyName, 10, combineMode,
rpcClient, keyInfo);
helper.compute();
FileChecksum fileChecksum = helper.getFileChecksum();
diff --git
a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/AbstractRootedOzoneFileSystemTest.java
b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/AbstractRootedOzoneFileSystemTest.java
index 197d100d86c..0b31625e6f8 100644
---
a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/AbstractRootedOzoneFileSystemTest.java
+++
b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/AbstractRootedOzoneFileSystemTest.java
@@ -1881,6 +1881,67 @@ void testGetFileStatusRejectsObsBucket() throws
Exception {
"getFileStatus must not trigger InfoBucket");
}
+ @Test
+ void testGetFileChecksumUsesSingleOmRpc() throws Exception {
+ Path filePath = new Path(bucketPath,
+ "checksum-rpc-" + RandomStringUtils.secure().nextAlphanumeric(5));
+ ContractTestUtils.createFile(fs, filePath, true,
"data".getBytes(StandardCharsets.UTF_8));
+
+ OMMetrics metrics = getOMMetrics();
+ long bucketInfosBefore = metrics.getNumBucketInfos();
+ long volumeInfosBefore = metrics.getNumVolumeInfos();
+ long keyLookupsBefore = metrics.getNumKeyLookups();
+
+ assertNotNull(fs.getFileChecksum(filePath));
+
+ assertEquals(bucketInfosBefore, metrics.getNumBucketInfos(),
+ "getFileChecksum must not trigger InfoBucket");
+ assertEquals(volumeInfosBefore, metrics.getNumVolumeInfos(),
+ "getFileChecksum must not trigger InfoVolume");
+ assertEquals(keyLookupsBefore, metrics.getNumKeyLookups(),
+ "getFileChecksum must use LookupFile, not LookupKey");
+ }
+
+ @Test
+ void testGetFileChecksumRejectsObsBucket() throws Exception {
+ OzoneBucket obsBucket =
+ DataTestUtil.createVolumeAndBucket(client, BucketLayout.OBJECT_STORE);
+ String keyName = "obs-key-" + RandomStringUtils.secure().nextAlphabetic(5);
+ DataTestUtil.createKey(obsBucket, keyName,
+ "data".getBytes(StandardCharsets.UTF_8));
+ Path keyPath = new Path(new Path(
+ new Path(OZONE_URI_DELIMITER, obsBucket.getVolumeName()),
+ obsBucket.getName()), keyName);
+
+ OMMetrics metrics = getOMMetrics();
+ long bucketInfosBefore = metrics.getNumBucketInfos();
+
+ IllegalArgumentException exception =
assertThrows(IllegalArgumentException.class,
+ () -> fs.getFileChecksum(keyPath));
+ assertThat(exception.getMessage()).contains(obsBucket.getName());
+ assertThat(exception.getMessage()).contains("OBJECT_STORE");
+ assertEquals(bucketInfosBefore, metrics.getNumBucketInfos(),
+ "getFileChecksum must not trigger InfoBucket");
+ }
+
+ @Test
+ void testGetFileChecksumOnDirectory() throws Exception {
+ Path dir = new Path(bucketPath,
+ "checksum-dir-" + RandomStringUtils.secure().nextAlphanumeric(5));
+ fs.mkdirs(dir);
+
+ assertThrows(FileNotFoundException.class, () -> fs.getFileChecksum(dir));
+ }
+
+ @Test
+ void testGetFileChecksumOnNonExistentFile() throws Exception {
+ Path missing = new Path(bucketPath,
+ "checksum-missing-" + RandomStringUtils.secure().nextAlphanumeric(5));
+
+ assertThrows(FileNotFoundException.class,
+ () -> fs.getFileChecksum(missing));
+ }
+
@Test
void testGetFileStatus() throws Exception {
String volumeNameLocal = getRandomNonExistVolumeName();
diff --git
a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestOfsGetFileChecksumRpcBenchmark.java
b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestOfsGetFileChecksumRpcBenchmark.java
new file mode 100644
index 00000000000..f515a5a7dad
--- /dev/null
+++
b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestOfsGetFileChecksumRpcBenchmark.java
@@ -0,0 +1,314 @@
+/*
+ * 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.fs.ozone;
+
+import static
org.apache.hadoop.fs.CommonConfigurationKeysPublic.FS_DEFAULT_NAME_KEY;
+import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_OM_ADDRESS_KEY;
+import static org.assertj.core.api.Assertions.assertThat;
+
+import java.io.IOException;
+import java.net.URI;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.List;
+import java.util.Random;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.TimeoutException;
+import java.util.concurrent.atomic.AtomicReference;
+import org.apache.hadoop.fs.FileSystem;
+import org.apache.hadoop.fs.Path;
+import org.apache.hadoop.fs.contract.ContractTestUtils;
+import org.apache.hadoop.hdds.conf.OzoneConfiguration;
+import org.apache.hadoop.hdds.utils.IOUtils;
+import org.apache.hadoop.ozone.MiniOzoneCluster;
+import org.apache.hadoop.ozone.OzoneConsts;
+import org.apache.hadoop.ozone.client.BucketArgs;
+import org.apache.hadoop.ozone.client.ObjectStore;
+import org.apache.hadoop.ozone.client.OzoneClient;
+import org.apache.hadoop.ozone.client.OzoneVolume;
+import org.apache.hadoop.ozone.om.OMConfigKeys;
+import org.apache.hadoop.ozone.om.OMMetrics;
+import org.apache.hadoop.ozone.om.helpers.BucketLayout;
+import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Tag;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.TestInstance;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Benchmark for the OFS getFileChecksum OM RPC count (HDDS-15951).
+ *
+ * <p>Adapted from the benchmark in PR #11369 so the same workload runs
unchanged on the baseline
+ * ({@code master}: InfoVolume + InfoBucket + LookupKey) and on the
server-side fix (a single LookupFile).
+ * The only changes are in the accounting: key reads are counted as LookupKey
+ LookupFile, because the fix
+ * moves the read from one to the other, and the InfoBucket invariant no
longer assumes at least one RPC per
+ * bucket, because the fix removes that RPC entirely instead of caching it.
+ *
+ * <p>{@link #NUM_BUCKETS} buckets are touched {@link #ACCESSES_PER_BUCKET}
times each from a shuffled access
+ * sequence, which is the shape PR #11369's per-bucket layout cache was built
for (90% of calls would hit it).
+ * Keeping that shape makes the three arms directly comparable. PR #11369
drops InfoVolume unconditionally and
+ * caches InfoBucket, so it costs (2 - hitRatio) RPCs per call: 1.10 at this
workload's 90% hit ratio, rising to
+ * 2.00 for a cold or scanning client. The server-side fix costs exactly 1.00
regardless of hit ratio.
+ */
+@Tag("benchmark")
+@TestInstance(TestInstance.Lifecycle.PER_CLASS)
+public class TestOfsGetFileChecksumRpcBenchmark {
+
+ private static final Logger LOG =
LoggerFactory.getLogger(TestOfsGetFileChecksumRpcBenchmark.class);
+
+ /** Distinct buckets in the working set. */
+ private static final int NUM_BUCKETS = 200;
+ /** getFileChecksum calls per bucket. */
+ private static final int ACCESSES_PER_BUCKET = 10;
+ private static final int TOTAL_CALLS = NUM_BUCKETS * ACCESSES_PER_BUCKET;
+ private static final long SHUFFLE_SEED = 20250831L;
+ /** Client threads issuing getFileChecksum in the concurrent comparison. */
+ private static final int CONCURRENT_THREADS = 10;
+ /** Size of each file, so every call computes a real checksum over one small
block. */
+ private static final int FILE_SIZE = 4 * 1024;
+
+ private MiniOzoneCluster cluster;
+ private OzoneClient client;
+ private OzoneConfiguration conf;
+ private String rootPath;
+
+ private final List<Path> accessSequence = new ArrayList<>(TOTAL_CALLS);
+
+ @BeforeAll
+ void init() throws IOException, InterruptedException, TimeoutException {
+ conf = new OzoneConfiguration();
+ conf.set(OMConfigKeys.OZONE_DEFAULT_BUCKET_LAYOUT,
BucketLayout.FILE_SYSTEM_OPTIMIZED.name());
+ cluster = MiniOzoneCluster.newBuilder(conf)
+ .setNumDatanodes(3)
+ .build();
+ cluster.waitForClusterToBeReady();
+ client = cluster.newClient();
+ rootPath = String.format("%s://%s/", OzoneConsts.OZONE_OFS_URI_SCHEME,
conf.get(OZONE_OM_ADDRESS_KEY));
+
+ ObjectStore objectStore = client.getObjectStore();
+ String volName = "benchvol";
+ objectStore.createVolume(volName);
+ OzoneVolume volume = objectStore.getVolume(volName);
+ BucketArgs fsoArgs =
BucketArgs.newBuilder().setBucketLayout(BucketLayout.FILE_SYSTEM_OPTIMIZED).build();
+ byte[] data = ContractTestUtils.dataset(FILE_SIZE, 'a', 26);
+ try (FileSystem setupFs = FileSystem.newInstance(URI.create(rootPath),
conf)) {
+ for (int i = 0; i < NUM_BUCKETS; i++) {
+ String buckName = "benchbucket-" + i;
+ volume.createBucket(buckName, fsoArgs);
+ Path file = new Path("/" + volName + "/" + buckName + "/file");
+ ContractTestUtils.createFile(setupFs, file, true, data);
+ for (int a = 0; a < ACCESSES_PER_BUCKET; a++) {
+ accessSequence.add(file);
+ }
+ }
+ assertThat(setupFs.getFileChecksum(accessSequence.get(0))).isNotNull();
+ }
+ Collections.shuffle(accessSequence, new Random(SHUFFLE_SEED));
+ }
+
+ @AfterAll
+ void shutdown() {
+ IOUtils.closeQuietly(client);
+ if (cluster != null) {
+ cluster.shutdown();
+ }
+ }
+
+ @Test
+ void benchmarkGetFileChecksum() throws Exception {
+ // Prime OM-side state and the JVM so the measured run pays no cold-start.
+ runWorkload(1);
+
+ Result single = runWorkload(1);
+ Result concurrent = runWorkload(CONCURRENT_THREADS);
+
+ assertWorkloadInvariants(single);
+ assertWorkloadInvariants(concurrent);
+
+ logResult("single-threaded", single);
+ logResult(CONCURRENT_THREADS + " concurrent threads", concurrent);
+ }
+
+ /**
+ * Invariants that hold on both the baseline and the fixed code, so the
identical benchmark passes in either
+ * worktree: every getFileChecksum still issues exactly one key-read RPC,
whether that is LookupKey (baseline)
+ * or LookupFile (fixed), and the volume and bucket RPCs stay within one per
call.
+ */
+ private void assertWorkloadInvariants(Result r) {
+ assertThat(r.lookupKeyRpcs + r.lookupFileRpcs).isEqualTo(TOTAL_CALLS);
+ assertThat(r.bucketInfoRpcs).isBetween(0L, (long) TOTAL_CALLS);
+ assertThat(r.volumeInfoRpcs).isBetween(0L, (long) TOTAL_CALLS);
+ }
+
+ private void logResult(String label, Result r) {
+ long omRpcs = r.volumeInfoRpcs + r.bucketInfoRpcs + r.lookupKeyRpcs +
r.lookupFileRpcs;
+ LOG.info(String.format("%n"
+ + "BENCHRESULT OFS getFileChecksum OM RPC benchmark (HDDS-15951) -
%s%n"
+ + " buckets=%d, accesses/bucket=%d, total getFileChecksum=%d,
threads=%d, file size=%d bytes%n"
+ + "
------------------------------------------------------------%n"
+ + " InfoVolume RPCs : %d%n"
+ + " InfoBucket RPCs : %d%n"
+ + " LookupKey RPCs : %d%n"
+ + " LookupFile RPCs : %d%n"
+ + " total OM RPCs : %d%n"
+ + " OM RPCs per call : %.2f%n"
+ + "
------------------------------------------------------------%n"
+ + " latency mean : %.3f ms%n"
+ + " latency p50 : %.3f ms%n"
+ + " latency p90 : %.3f ms%n"
+ + " latency p99 : %.3f ms%n"
+ + " latency max : %.3f ms%n"
+ + " throughput : %.1f getFileChecksum/s%n",
+ label, NUM_BUCKETS, ACCESSES_PER_BUCKET, TOTAL_CALLS, r.threads,
FILE_SIZE,
+ r.volumeInfoRpcs, r.bucketInfoRpcs, r.lookupKeyRpcs, r.lookupFileRpcs,
+ omRpcs, omRpcs / (double) TOTAL_CALLS,
+ millis(r.mean()), millis(r.percentile(0.50)),
+ millis(r.percentile(0.90)), millis(r.percentile(0.99)),
+ millis(r.max()), r.opsPerSecond()));
+ }
+
+ private static double millis(double nanos) {
+ return nanos / (double) TimeUnit.MILLISECONDS.toNanos(1);
+ }
+
+ private Result runWorkload(int threads) throws IOException,
InterruptedException {
+ OzoneConfiguration runConf = new OzoneConfiguration(conf);
+ runConf.set(FS_DEFAULT_NAME_KEY, rootPath);
+
+ OMMetrics metrics = cluster.getOzoneManager().getMetrics();
+ long[] latencyNanos = new long[TOTAL_CALLS];
+ // A new FileSystem instance per run, so each run starts with no
client-side state carried over.
+ try (FileSystem fs = FileSystem.newInstance(URI.create(rootPath),
runConf)) {
+ long volumeInfosBefore = metrics.getNumVolumeInfos();
+ long bucketInfosBefore = metrics.getNumBucketInfos();
+ long keyLookupsBefore = metrics.getNumKeyLookups();
+ long lookupFilesBefore = metrics.getNumLookupFile();
+ long elapsedNanos = threads == 1
+ ? runSequential(fs, latencyNanos)
+ : runConcurrent(fs, threads, latencyNanos);
+ return new Result(threads,
+ metrics.getNumVolumeInfos() - volumeInfosBefore,
+ metrics.getNumBucketInfos() - bucketInfosBefore,
+ metrics.getNumKeyLookups() - keyLookupsBefore,
+ metrics.getNumLookupFile() - lookupFilesBefore,
+ elapsedNanos, latencyNanos);
+ }
+ }
+
+ private long runSequential(FileSystem fs, long[] latencyNanos) throws
IOException {
+ long startNanos = System.nanoTime();
+ int i = 0;
+ for (Path path : accessSequence) {
+ long callStart = System.nanoTime();
+ fs.getFileChecksum(path);
+ latencyNanos[i++] = System.nanoTime() - callStart;
+ }
+ return System.nanoTime() - startNanos;
+ }
+
+ private long runConcurrent(FileSystem fs, int threads, long[] latencyNanos)
+ throws IOException, InterruptedException {
+ ExecutorService pool = Executors.newFixedThreadPool(threads);
+ CountDownLatch startGate = new CountDownLatch(1);
+ CountDownLatch doneGate = new CountDownLatch(threads);
+ AtomicReference<IOException> failure = new AtomicReference<>();
+ // Disjoint, contiguous slices of the shuffled sequence; each thread
writes only its own latency indices, so no
+ // synchronization is needed per call.
+ int chunk = (TOTAL_CALLS + threads - 1) / threads;
+ for (int t = 0; t < threads; t++) {
+ final int from = t * chunk;
+ final int to = Math.min(TOTAL_CALLS, from + chunk);
+ pool.submit(() -> {
+ try {
+ startGate.await();
+ for (int i = from; i < to; i++) {
+ long callStart = System.nanoTime();
+ fs.getFileChecksum(accessSequence.get(i));
+ latencyNanos[i] = System.nanoTime() - callStart;
+ }
+ } catch (IOException e) {
+ failure.compareAndSet(null, e);
+ } catch (InterruptedException e) {
+ Thread.currentThread().interrupt();
+ } finally {
+ doneGate.countDown();
+ }
+ });
+ }
+ long startNanos = System.nanoTime();
+ startGate.countDown();
+ doneGate.await();
+ long elapsedNanos = System.nanoTime() - startNanos;
+ pool.shutdownNow();
+ if (failure.get() != null) {
+ throw failure.get();
+ }
+ return elapsedNanos;
+ }
+
+ private static final class Result {
+ private final int threads;
+ private final long volumeInfoRpcs;
+ private final long bucketInfoRpcs;
+ private final long lookupKeyRpcs;
+ private final long lookupFileRpcs;
+ private final long elapsedNanos;
+ private final long[] sortedLatencyNanos;
+
+ @SuppressWarnings("checkstyle:ParameterNumber")
+ Result(int threads, long volumeInfoRpcs, long bucketInfoRpcs, long
lookupKeyRpcs, long lookupFileRpcs,
+ long elapsedNanos, long[] latencyNanos) {
+ this.threads = threads;
+ this.volumeInfoRpcs = volumeInfoRpcs;
+ this.bucketInfoRpcs = bucketInfoRpcs;
+ this.lookupKeyRpcs = lookupKeyRpcs;
+ this.lookupFileRpcs = lookupFileRpcs;
+ this.elapsedNanos = elapsedNanos;
+ this.sortedLatencyNanos = latencyNanos.clone();
+ Arrays.sort(this.sortedLatencyNanos);
+ }
+
+ double opsPerSecond() {
+ return TOTAL_CALLS / (elapsedNanos / (double)
TimeUnit.SECONDS.toNanos(1));
+ }
+
+ double mean() {
+ long sum = 0;
+ for (long l : sortedLatencyNanos) {
+ sum += l;
+ }
+ return sum / (double) sortedLatencyNanos.length;
+ }
+
+ double percentile(double q) {
+ int idx = (int) Math.ceil(q * sortedLatencyNanos.length) - 1;
+ idx = Math.max(0, Math.min(sortedLatencyNanos.length - 1, idx));
+ return sortedLatencyNanos[idx];
+ }
+
+ double max() {
+ return sortedLatencyNanos[sortedLatencyNanos.length - 1];
+ }
+ }
+}
diff --git
a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestOzoneManagerSnapshotAcl.java
b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestOzoneManagerSnapshotAcl.java
index 4f4508ebdc7..35de4fdd129 100644
---
a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestOzoneManagerSnapshotAcl.java
+++
b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestOzoneManagerSnapshotAcl.java
@@ -277,8 +277,12 @@ public void testLookupFileWithAllowedUser(BucketLayout
bucketLayout)
final OmKeyArgs snapshotKeyArgs = getOmKeyArgs(true);
// WHEN-THEN
- assertDoesNotThrow(
- () -> ozoneManager.lookupFile(snapshotKeyArgs));
+ if (bucketLayout == BucketLayout.OBJECT_STORE) {
+ assertLookupFileNotSupported(snapshotKeyArgs);
+ } else {
+ assertDoesNotThrow(
+ () -> ozoneManager.lookupFile(snapshotKeyArgs));
+ }
}
@ParameterizedTest
@@ -292,6 +296,16 @@ public void testLookupFileWithNotAllowedUser(BucketLayout
bucketLayout)
// when reading from snapshot, read disallowed.
UserGroupInformation.setLoginUser(UGI2);
+
+ if (bucketLayout == BucketLayout.OBJECT_STORE) {
+ // HDDS-15951: the layout check runs before the ACL check, so the
+ // rejection wins over PERMISSION_DENIED on both the snapshot and the
+ // active file system.
+ assertLookupFileNotSupported(snapshotKeyArgs);
+ assertLookupFileNotSupported(keyArgs);
+ return;
+ }
+
final OMException ex = assertThrows(OMException.class,
() -> ozoneManager.lookupFile(snapshotKeyArgs));
@@ -302,6 +316,18 @@ public void testLookupFileWithNotAllowedUser(BucketLayout
bucketLayout)
assertDoesNotThrow(() -> ozoneManager.lookupFile(keyArgs));
}
+ /**
+ * HDDS-15951 moved the OBJECT_STORE layout check for LookupFile from the
file
+ * system client into OM, so OBS buckets - which have no file system
semantics
+ * - are now rejected server-side, snapshots included.
+ */
+ private static void assertLookupFileNotSupported(OmKeyArgs keyArgs) {
+ final OMException ex = assertThrows(OMException.class,
+ () -> ozoneManager.lookupFile(keyArgs));
+ assertEquals(OMException.ResultCodes.NOT_SUPPORTED_OPERATION,
+ ex.getResult());
+ }
+
@ParameterizedTest
@EnumSource(BucketLayout.class)
public void testListKeysWithAllowedUser(BucketLayout bucketLayout)
diff --git
a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OMMetrics.java
b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OMMetrics.java
index ff74e6b6e67..8ecb514a383 100644
---
a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OMMetrics.java
+++
b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OMMetrics.java
@@ -1217,6 +1217,11 @@ public long getNumKeyLookups() {
return numKeyLookup.value();
}
+ @VisibleForTesting
+ public long getNumLookupFile() {
+ return numLookupFile.value();
+ }
+
@VisibleForTesting
public long getNumKeyLookupFails() {
return numKeyLookupFails.value();
diff --git
a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmMetadataReader.java
b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmMetadataReader.java
index e93219ae64a..4435cf78b82 100644
---
a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmMetadataReader.java
+++
b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmMetadataReader.java
@@ -361,6 +361,18 @@ public OmKeyInfo lookupFile(OmKeyArgs args) throws
IOException {
args = normalizeKeyArgs(bucket.update(args), bucket);
try {
+ if (bucket.bucketLayout() != null) {
+ try {
+ OzoneFSUtils.validateBucketLayout(bucket.requestedBucket(),
+ bucket.bucketLayout());
+ } catch (IllegalArgumentException e) {
+ // Convert to an OMException so it is returned to the client as a
+ // normal (non-retryable) RPC response instead of escaping the read
+ // handler's IOException catch and triggering a client retry storm.
+ throw new OMException(e.getMessage(),
+ ResultCodes.NOT_SUPPORTED_OPERATION);
+ }
+ }
if (isAclEnabled) {
checkAcls(ResourceType.KEY, StoreType.OZONE, ACLType.READ,
bucket, args.getKeyName());
diff --git
a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestOMMetadataReader.java
b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestOMMetadataReader.java
index 5defb87760f..01a5bad1808 100644
---
a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestOMMetadataReader.java
+++
b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestOMMetadataReader.java
@@ -53,6 +53,7 @@
import org.apache.hadoop.ozone.om.helpers.BucketLayout;
import org.apache.hadoop.ozone.om.helpers.ListKeysResult;
import org.apache.hadoop.ozone.om.helpers.OmKeyArgs;
+import org.apache.hadoop.ozone.om.helpers.OmKeyInfo;
import org.apache.hadoop.ozone.om.helpers.OzoneFileStatus;
import org.apache.hadoop.ozone.om.protocolPB.grpc.GrpcClientConstants;
import
org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.S3Authentication;
@@ -840,4 +841,63 @@ public void getFileStatusAllowsLegacyLayout() throws
Exception {
assertEquals(expectedStatus, status);
verify(keyManager).getFileStatus(any(OmKeyArgs.class), anyString());
}
+
+ @Test
+ public void lookupFileRejectsObjectStoreLayout() throws Exception {
+ OzoneManager ozoneManager = mock(OzoneManager.class);
+ KeyManager keyManager = mock(KeyManager.class);
+ when(ozoneManager.getAclsEnabled()).thenReturn(false);
+
when(ozoneManager.getBucketManager()).thenReturn(mock(BucketManager.class));
+
when(ozoneManager.getVolumeManager()).thenReturn(mock(VolumeManager.class));
+
when(ozoneManager.getPerfMetrics()).thenReturn(mock(OMPerformanceMetrics.class));
+ when(ozoneManager.resolveBucketLink(any(OmKeyArgs.class)))
+ .thenReturn(new ResolvedBucket("vol", "obs-bucket", "vol",
"obs-bucket",
+ "owner", BucketLayout.OBJECT_STORE));
+
+ OmMetadataReader reader = new OmMetadataReader(keyManager,
+ mock(PrefixManager.class), ozoneManager, mock(org.slf4j.Logger.class),
+ mock(AuditLogger.class), mock(OmMetadataReaderMetrics.class), null);
+
+ OmKeyArgs keyArgs = new OmKeyArgs.Builder()
+ .setVolumeName("vol")
+ .setBucketName("obs-bucket")
+ .setKeyName("key1")
+ .build();
+
+ OMException exception = assertThrows(OMException.class,
+ () -> reader.lookupFile(keyArgs));
+ assertEquals(ResultCodes.NOT_SUPPORTED_OPERATION, exception.getResult());
+ assertTrue(exception.getMessage().contains("obs-bucket"));
+ assertTrue(exception.getMessage().contains("OBJECT_STORE"));
+ verify(keyManager, never()).lookupFile(any(), anyString());
+ }
+
+ @Test
+ public void lookupFileAllowsLegacyLayout() throws Exception {
+ OzoneManager ozoneManager = mock(OzoneManager.class);
+ KeyManager keyManager = mock(KeyManager.class);
+ when(ozoneManager.getAclsEnabled()).thenReturn(false);
+
when(ozoneManager.getBucketManager()).thenReturn(mock(BucketManager.class));
+
when(ozoneManager.getVolumeManager()).thenReturn(mock(VolumeManager.class));
+
when(ozoneManager.getPerfMetrics()).thenReturn(mock(OMPerformanceMetrics.class));
+ when(ozoneManager.resolveBucketLink(any(OmKeyArgs.class)))
+ .thenReturn(new ResolvedBucket("vol", "legacy-bucket", "vol",
+ "legacy-bucket", "owner", BucketLayout.LEGACY));
+ OmKeyInfo expectedKeyInfo = mock(OmKeyInfo.class);
+ when(keyManager.lookupFile(any(OmKeyArgs.class), anyString()))
+ .thenReturn(expectedKeyInfo);
+
+ OmMetadataReader reader = new OmMetadataReader(keyManager,
+ mock(PrefixManager.class), ozoneManager, mock(org.slf4j.Logger.class),
+ mock(AuditLogger.class), mock(OmMetadataReaderMetrics.class), null);
+
+ OmKeyArgs keyArgs = new OmKeyArgs.Builder()
+ .setVolumeName("vol")
+ .setBucketName("legacy-bucket")
+ .setKeyName("key1")
+ .build();
+
+ assertEquals(expectedKeyInfo, reader.lookupFile(keyArgs));
+ verify(keyManager).lookupFile(any(OmKeyArgs.class), anyString());
+ }
}
diff --git
a/hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicOzoneClientAdapterImpl.java
b/hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicOzoneClientAdapterImpl.java
index 39cce33637f..b733b461d48 100644
---
a/hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicOzoneClientAdapterImpl.java
+++
b/hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicOzoneClientAdapterImpl.java
@@ -672,10 +672,12 @@ public FileChecksum getFileChecksum(String keyName, long
length)
if (combineMode == null) {
return null;
}
+ // The bucket layout is validated once in the constructor, so LookupKey is
+ // enough here; there is no InfoBucket RPC to save on this path.
return OzoneClientUtils.getFileChecksumWithCombineMode(
- volume, bucket, keyName,
+ volume.getName(), bucket.getName(), keyName,
length, combineMode,
- ozoneClient.getObjectStore().getClientProxy());
+ ozoneClient.getObjectStore().getClientProxy(), false);
}
@Override
diff --git
a/hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicRootedOzoneClientAdapterImpl.java
b/hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicRootedOzoneClientAdapterImpl.java
index 9b1ed021267..c7cf1f729ac 100644
---
a/hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicRootedOzoneClientAdapterImpl.java
+++
b/hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicRootedOzoneClientAdapterImpl.java
@@ -1377,13 +1377,45 @@ public FileChecksum getFileChecksum(String keyName,
long length)
return null;
}
OFSPath ofsPath = new OFSPath(keyName, config);
- OzoneVolume volume = objectStore.getVolume(ofsPath.getVolumeName());
- OzoneBucket bucket = getBucket(ofsPath, false);
- return OzoneClientUtils.getFileChecksumWithCombineMode(
- volume, bucket, ofsPath.getKeyName(),
- length, combineMode,
- ozoneClient.getObjectStore().getClientProxy());
-
+ if (ofsPath.getBucketName().isEmpty()) {
+ // throw FileNotFoundException in this case to make Hadoop common happy
+ throw new FileNotFoundException(
+ "getFileChecksum: Invalid argument: given bucket string is empty.");
+ }
+ // OM LookupFile rejects OBJECT_STORE buckets (HDDS-15951), so the volume
and
+ // bucket no longer have to be fetched up front. During a rolling upgrade
a new
+ // client can talk to an older OM that lacks that check, so when the
negotiated
+ // OM version predates LOOKUP_FILE_REJECTS_OBS, fall back to LookupKey and
+ // validate the bucket layout client-side as before.
+ boolean omRejectsObs = proxy.getOmVersion()
+ .compareTo(OzoneManagerVersion.LOOKUP_FILE_REJECTS_OBS) >= 0;
+ if (!omRejectsObs) {
+ // Called for its layout validation side effect; the bucket itself is
not needed.
+ getBucket(ofsPath, false);
+ }
+ try {
+ return OzoneClientUtils.getFileChecksumWithCombineMode(
+ ofsPath.getVolumeName(), ofsPath.getBucketName(),
ofsPath.getKeyName(),
+ length, combineMode,
+ ozoneClient.getObjectStore().getClientProxy(), omRejectsObs);
+ } catch (OMException e) {
+ if (e.getResult() == OMException.ResultCodes.NOT_SUPPORTED_OPERATION) {
+ // OM rejects LookupFile on an OBJECT_STORE bucket (no file system
+ // semantics). Surface it as IllegalArgumentException, matching the
+ // pre-HDDS-15951 client-side layout check.
+ throw new IllegalArgumentException(e.getMessage());
+ } else if (e.getResult() == OMException.ResultCodes.NOT_A_FILE
+ || e.getResult() == OMException.ResultCodes.FILE_NOT_FOUND
+ || e.getResult() == OMException.ResultCodes.KEY_NOT_FOUND
+ || e.getResult() == OMException.ResultCodes.BUCKET_NOT_FOUND) {
+ // A checksum only exists for an existing file, so report the missing
+ // path the same way HDFS and getFileStatusForKeyOrSnapshot do.
+ // LookupFile reports a directory as NOT_A_FILE and a missing key as
+ // FILE_NOT_FOUND; the LookupKey fallback reports KEY_NOT_FOUND.
+ throw new FileNotFoundException(e.getMessage());
+ }
+ throw e;
+ }
}
@Override
diff --git
a/hadoop-ozone/ozonefs-common/src/test/java/org/apache/hadoop/fs/ozone/TestBasicRootedOzoneClientAdapterGetFileChecksum.java
b/hadoop-ozone/ozonefs-common/src/test/java/org/apache/hadoop/fs/ozone/TestBasicRootedOzoneClientAdapterGetFileChecksum.java
new file mode 100644
index 00000000000..22d1918dd1c
--- /dev/null
+++
b/hadoop-ozone/ozonefs-common/src/test/java/org/apache/hadoop/fs/ozone/TestBasicRootedOzoneClientAdapterGetFileChecksum.java
@@ -0,0 +1,199 @@
+/*
+ * 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.fs.ozone;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.anyString;
+import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.Mockito.CALLS_REAL_METHODS;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+import java.io.FileNotFoundException;
+import java.io.IOException;
+import java.lang.reflect.Field;
+import org.apache.hadoop.hdds.conf.OzoneConfiguration;
+import org.apache.hadoop.hdds.scm.OzoneClientConfig;
+import org.apache.hadoop.ozone.OzoneManagerVersion;
+import org.apache.hadoop.ozone.client.ObjectStore;
+import org.apache.hadoop.ozone.client.OzoneBucket;
+import org.apache.hadoop.ozone.client.OzoneClient;
+import org.apache.hadoop.ozone.client.protocol.ClientProtocol;
+import org.apache.hadoop.ozone.om.exceptions.OMException;
+import org.apache.hadoop.ozone.om.helpers.BucketLayout;
+import org.apache.hadoop.ozone.om.helpers.OmKeyArgs;
+import org.apache.hadoop.ozone.om.protocol.OzoneManagerProtocol;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+/**
+ * Unit tests for {@link BasicRootedOzoneClientAdapterImpl#getFileChecksum},
+ * covering the single-RPC LookupFile path added by HDDS-15951, the LookupKey
+ * fallback used when the OM predates
+ * {@link OzoneManagerVersion#LOOKUP_FILE_REJECTS_OBS}, and the mapping of OM
+ * result codes onto file system exceptions. Uses a partial mock so no OM
+ * connection is required.
+ */
+public class TestBasicRootedOzoneClientAdapterGetFileChecksum {
+
+ private static final String KEY_PATH = "/vol/bucket/key";
+
+ private BasicRootedOzoneClientAdapterImpl adapter;
+ private ClientProtocol proxy;
+ private ObjectStore objectStore;
+ private OzoneManagerProtocol omClient;
+ private OzoneBucket bucket;
+
+ @BeforeEach
+ public void setUp() throws Exception {
+ adapter = mock(BasicRootedOzoneClientAdapterImpl.class,
CALLS_REAL_METHODS);
+ proxy = mock(ClientProtocol.class);
+ objectStore = mock(ObjectStore.class);
+ omClient = mock(OzoneManagerProtocol.class);
+ bucket = mock(OzoneBucket.class);
+
+ when(proxy.getOzoneManagerClient()).thenReturn(omClient);
+ when(proxy.getBucketDetails(anyString(), anyString())).thenReturn(bucket);
+ when(bucket.getName()).thenReturn("bucket");
+ when(bucket.getBucketLayout())
+ .thenReturn(BucketLayout.FILE_SYSTEM_OPTIMIZED);
+
+ OzoneClient ozoneClient = mock(OzoneClient.class);
+ when(ozoneClient.getObjectStore()).thenReturn(objectStore);
+ when(objectStore.getClientProxy()).thenReturn(proxy);
+
+ OzoneClientConfig clientConfig = mock(OzoneClientConfig.class);
+ when(clientConfig.getChecksumCombineMode())
+ .thenReturn(OzoneClientConfig.ChecksumCombineMode.MD5MD5CRC);
+
+ setField("proxy", proxy);
+ setField("objectStore", objectStore);
+ setField("ozoneClient", ozoneClient);
+ setField("config", new OzoneConfiguration());
+ setField("clientConfig", clientConfig);
+
+ // Default to an OM that performs the server-side OBJECT_STORE rejection.
+ when(proxy.getOmVersion())
+ .thenReturn(OzoneManagerVersion.LOOKUP_FILE_REJECTS_OBS);
+ }
+
+ private void setField(String name, Object value) throws Exception {
+ Field field =
+ BasicRootedOzoneClientAdapterImpl.class.getDeclaredField(name);
+ field.setAccessible(true);
+ field.set(adapter, value);
+ }
+
+ @Test
+ public void lookupFileIsTheOnlyOmCall() throws IOException {
+ when(omClient.lookupFile(any(OmKeyArgs.class)))
+ .thenThrow(new OMException("missing",
+ OMException.ResultCodes.FILE_NOT_FOUND));
+
+ assertThrows(FileNotFoundException.class,
+ () -> adapter.getFileChecksum(KEY_PATH, 10));
+
+ verify(omClient).lookupFile(any(OmKeyArgs.class));
+ verify(omClient, never()).lookupKey(any(OmKeyArgs.class));
+ verify(proxy, never()).getBucketDetails(anyString(), anyString());
+ verify(objectStore, never()).getVolume(anyString());
+ }
+
+ @Test
+ public void notAFileMappedToFileNotFoundException() throws IOException {
+ when(omClient.lookupFile(any(OmKeyArgs.class)))
+ .thenThrow(new OMException("Can not write to directory: key",
+ OMException.ResultCodes.NOT_A_FILE));
+
+ assertThrows(FileNotFoundException.class,
+ () -> adapter.getFileChecksum(KEY_PATH, 10));
+ }
+
+ @Test
+ public void bucketNotFoundMappedToFileNotFoundException() throws IOException
{
+ when(omClient.lookupFile(any(OmKeyArgs.class)))
+ .thenThrow(new OMException("no bucket",
+ OMException.ResultCodes.BUCKET_NOT_FOUND));
+
+ assertThrows(FileNotFoundException.class,
+ () -> adapter.getFileChecksum(KEY_PATH, 10));
+ }
+
+ @Test
+ public void obsRejectionMappedToIllegalArgumentException()
+ throws IOException {
+ when(omClient.lookupFile(any(OmKeyArgs.class)))
+ .thenThrow(new OMException("Bucket: bucket has layout: OBJECT_STORE",
+ OMException.ResultCodes.NOT_SUPPORTED_OPERATION));
+
+ IllegalArgumentException ex = assertThrows(IllegalArgumentException.class,
+ () -> adapter.getFileChecksum(KEY_PATH, 10));
+ assertThat(ex.getMessage()).contains("OBJECT_STORE");
+ }
+
+ @Test
+ public void otherOMExceptionPropagates() throws IOException {
+ when(omClient.lookupFile(any(OmKeyArgs.class)))
+ .thenThrow(new OMException("boom",
+ OMException.ResultCodes.INTERNAL_ERROR));
+
+ assertThrows(OMException.class,
+ () -> adapter.getFileChecksum(KEY_PATH, 10));
+ }
+
+ @Test
+ public void olderOmFallsBackToClientSideBucketCheck() throws IOException {
+ // An OM older than LOOKUP_FILE_REJECTS_OBS has no server-side OBJECT_STORE
+ // rejection, so the adapter must keep the pre-HDDS-15951 path: fetch the
+ // bucket (which validates the layout client-side) and use LookupKey.
+ when(proxy.getOmVersion())
+ .thenReturn(OzoneManagerVersion.S3_BUCKET_TAGGING_API);
+ when(omClient.lookupKey(any(OmKeyArgs.class)))
+ .thenThrow(new OMException("missing",
+ OMException.ResultCodes.KEY_NOT_FOUND));
+
+ assertThrows(FileNotFoundException.class,
+ () -> adapter.getFileChecksum(KEY_PATH, 10));
+
+ verify(proxy).getBucketDetails(eq("vol"), eq("bucket"));
+ verify(omClient).lookupKey(any(OmKeyArgs.class));
+ verify(omClient, never()).lookupFile(any(OmKeyArgs.class));
+ // The bucket is not a link, so no InfoVolume is needed to resolve layout.
+ verify(objectStore, never()).getVolume(anyString());
+ }
+
+ @Test
+ public void olderOmRejectsObsBucketClientSide() throws IOException {
+ when(proxy.getOmVersion())
+ .thenReturn(OzoneManagerVersion.S3_BUCKET_TAGGING_API);
+ when(bucket.getBucketLayout()).thenReturn(BucketLayout.OBJECT_STORE);
+
+ IllegalArgumentException ex = assertThrows(IllegalArgumentException.class,
+ () -> adapter.getFileChecksum(KEY_PATH, 10));
+ assertThat(ex.getMessage()).contains("OBJECT_STORE");
+
+ verify(proxy).getBucketDetails(eq("vol"), eq("bucket"));
+ verify(omClient, never()).lookupKey(any(OmKeyArgs.class));
+ verify(omClient, never()).lookupFile(any(OmKeyArgs.class));
+ verify(objectStore, never()).getVolume(anyString());
+ }
+}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
