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 a968dd9618ed..9bd041c615c4 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 @@ -61,7 +61,12 @@ public enum OzoneManagerVersion implements ComponentVersion { S3_BUCKET_TAGGING_API(13, "OzoneManager version that supports S3 bucket tagging APIs, such as " + "PutBucketTagging, GetBucketTagging, and DeleteBucketTagging"), - + + GET_FILE_STATUS_REJECTS_OBS(14, + "OzoneManager version that rejects getFileStatus on OBJECT_STORE " + + "buckets server-side, so file system clients no longer need the " + + "client-side InfoBucket layout check"), + 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/client/src/main/java/org/apache/hadoop/ozone/client/protocol/ClientProtocol.java b/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/protocol/ClientProtocol.java index 84b49e1b57e0..3a31a111d6e4 100644 --- a/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/protocol/ClientProtocol.java +++ b/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/protocol/ClientProtocol.java @@ -31,6 +31,7 @@ import org.apache.hadoop.io.Text; import org.apache.hadoop.ozone.OzoneAcl; import org.apache.hadoop.ozone.OzoneFsServerDefaults; +import org.apache.hadoop.ozone.OzoneManagerVersion; import org.apache.hadoop.ozone.client.BucketArgs; import org.apache.hadoop.ozone.client.OzoneBucket; import org.apache.hadoop.ozone.client.OzoneKey; @@ -1063,6 +1064,14 @@ TenantUserList listUsersInTenant(String tenantId, String prefix) */ OzoneFsServerDefaults getServerDefaults() throws IOException; + /** + * Returns the negotiated Ozone Manager version for the connected cluster. + * In an HA cluster this is the minimum version across all OMs, so callers + * can safely gate client behavior on new server-side features. + * @return the effective Ozone Manager version. + */ + OzoneManagerVersion getOmVersion(); + /** * Get KMS client provider. * @return KMS client provider. diff --git a/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java b/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java index 9c121bde4c89..92f4df2b7c9f 100644 --- a/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java +++ b/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java @@ -2845,6 +2845,11 @@ public KeyProvider call() throws Exception { } } + @Override + public OzoneManagerVersion getOmVersion() { + return omVersion; + } + @Override public OzoneFsServerDefaults getServerDefaults() throws IOException { long now = Time.monotonicNow(); 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 a0d09264adbe..197d100d86c1 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 @@ -1835,16 +1835,74 @@ void testBucketDefaultsShouldBeInheritedToFileForEC() assertEquals(ReplicationType.EC.name(), key.getReplicationConfig().getReplicationType().name()); } + @Test + void testGetFileStatusUsesSingleOmRpc() throws Exception { + String keyName = "single-rpc-" + RandomStringUtils.secure().nextAlphabetic(5); + Path filePath = new Path(bucketPath, keyName); + ContractTestUtils.touch(fs, filePath); + + OMMetrics metrics = getOMMetrics(); + long bucketInfosBefore = metrics.getNumBucketInfos(); + long getFileStatusBefore = metrics.getNumGetFileStatus(); + + FileStatus status = fs.getFileStatus(filePath); + assertTrue(status.isFile()); + + assertEquals(bucketInfosBefore, metrics.getNumBucketInfos(), + "getFileStatus must not trigger InfoBucket"); + assertEquals(getFileStatusBefore + 1, metrics.getNumGetFileStatus()); + + long getFileStatusAfterFirst = metrics.getNumGetFileStatus(); + fs.getFileStatus(filePath); + assertEquals(bucketInfosBefore, metrics.getNumBucketInfos()); + assertEquals(getFileStatusAfterFirst + 1, metrics.getNumGetFileStatus()); + } + + @Test + void testGetFileStatusRejectsObsBucket() throws Exception { + OzoneBucket obsBucket = + DataTestUtil.createVolumeAndBucket(client, BucketLayout.OBJECT_STORE); + Path obsBucketPath = new Path( + new Path(OZONE_URI_DELIMITER, obsBucket.getVolumeName()), + obsBucket.getName()); + String keyName = "obs-key-" + RandomStringUtils.secure().nextAlphabetic(5); + DataTestUtil.createKey(obsBucket, keyName, + "data".getBytes(StandardCharsets.UTF_8)); + Path keyPath = new Path(obsBucketPath, keyName); + + OMMetrics metrics = getOMMetrics(); + long bucketInfosBefore = metrics.getNumBucketInfos(); + + IllegalArgumentException exception = assertThrows(IllegalArgumentException.class, + () -> fs.getFileStatus(keyPath)); + assertThat(exception.getMessage()).contains(obsBucket.getName()); + assertThat(exception.getMessage()).contains("OBJECT_STORE"); + assertEquals(bucketInfosBefore, metrics.getNumBucketInfos(), + "getFileStatus must not trigger InfoBucket"); + } + @Test void testGetFileStatus() throws Exception { String volumeNameLocal = getRandomNonExistVolumeName(); String bucketNameLocal = RandomStringUtils.secure().nextNumeric(5); Path volume = new Path("/" + volumeNameLocal); fs.mkdirs(volume); - assertThrows(OMException.class, - () -> fs.getFileStatus(new Path(volume, bucketNameLocal))); - // Cleanup - fs.delete(volume, false); + try { + FileNotFoundException exception = assertThrows(FileNotFoundException.class, + () -> fs.getFileStatus(new Path(volume, bucketNameLocal))); + assertThat(exception.getMessage()).contains("Bucket doesn't exist"); + } finally { + fs.delete(volume, false); + } + } + + @Test + void testGetFileStatusMissingFile() throws Exception { + Path missingFile = new Path(bucketPath, "missing-file-" + + RandomStringUtils.secure().nextAlphanumeric(5)); + FileNotFoundException exception = assertThrows(FileNotFoundException.class, + () -> fs.getFileStatus(missingFile)); + assertThat(exception.getMessage()).contains("No such file or directory"); } @Test diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestOfsGetFileStatusBenchmark.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestOfsGetFileStatusBenchmark.java new file mode 100644 index 000000000000..37987e2b84c0 --- /dev/null +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestOfsGetFileStatusBenchmark.java @@ -0,0 +1,356 @@ +/* + * 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 OFS {@code getFileStatus} on non-snapshot paths (HDDS-15925). + * + *

Runs a repeated-access getFileStatus workload against one + * {@link MiniOzoneCluster} using the default client configuration, and reports + * the InfoBucket RPCs, getFileStatus RPCs, per-call latency (mean/p50/p90/p99/ + * max) and throughput measured by OM. The workload touches {@link #NUM_BUCKETS} + * buckets {@link #ACCESSES_PER_BUCKET} times each in a shuffled order, the way a + * real workload repeatedly stats files across many buckets. + * + *

The identical test runs on both this branch and the baseline + * ({@code master}) code. The before/after comparison is what OM reports for the + * same workload: + *

+ * The getFileStatus RPC count is identical either way, showing the optimization + * removes only the redundant InfoBucket RPC and changes no behaviour; the + * latency percentiles show the effect of dropping that RPC per call. + * + *

A second test runs the same workload single-threaded and then across + * {@link #CONCURRENT_THREADS} client threads sharing one {@code FileSystem}, and + * logs the two side by side, confirming the InfoBucket RPC removal holds under + * concurrency while throughput scales with the added client threads. + */ +@Tag("benchmark") +@TestInstance(TestInstance.Lifecycle.PER_CLASS) +public class TestOfsGetFileStatusBenchmark { + + private static final Logger LOG = + LoggerFactory.getLogger(TestOfsGetFileStatusBenchmark.class); + + /** Distinct buckets in the working set. */ + private static final int NUM_BUCKETS = 200; + /** getFileStatus 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 getFileStatus in the concurrent comparison. */ + private static final int CONCURRENT_THREADS = 10; + + private MiniOzoneCluster cluster; + private OzoneClient client; + private OzoneConfiguration conf; + private String rootPath; + + private final List 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)); + + // One volume, NUM_BUCKETS FSO buckets, one file per bucket. The access + // sequence lists each file ACCESSES_PER_BUCKET times, then is shuffled with + // a fixed seed so hits and misses interleave the way a real workload would. + 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(); + 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.touch(setupFs, file); + for (int a = 0; a < ACCESSES_PER_BUCKET; a++) { + accessSequence.add(file); + } + } + } + Collections.shuffle(accessSequence, new Random(SHUFFLE_SEED)); + } + + @AfterAll + void shutdown() { + IOUtils.closeQuietly(client); + if (cluster != null) { + cluster.shutdown(); + } + } + + @Test + void benchmarkGetFileStatusWorkflow() throws Exception { + // Prime OM-side state and the JVM so the measured run pays no cold-start. + runWorkload(1); + + Result measured = runWorkload(1); + + assertWorkloadInvariants(measured); + logResult("single-threaded", measured); + } + + @Test + void benchmarkConcurrentVsSingleThreaded() throws Exception { + // Warm the JVM/OM, then measure the same workload single-threaded and again + // across CONCURRENT_THREADS client threads sharing one FileSystem. The + // InfoBucket RPC removal holds under concurrency, so the getFileStatus RPC + // count and InfoBucket RPC count match the single-threaded run. + 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); + LOG.info(String.format("%n" + + "single-threaded vs %d concurrent threads (optimized)%n" + + " InfoBucket RPCs : %d -> %d%n" + + " getFileStatus RPCs : %d -> %d%n" + + " latency mean (ms) : %.3f -> %.3f%n" + + " latency p99 (ms) : %.3f -> %.3f%n" + + " throughput (ops/s) : %.1f -> %.1f (%.2fx)%n", + CONCURRENT_THREADS, + single.infoBucketRpcs, concurrent.infoBucketRpcs, + single.getFileStatusRpcs, concurrent.getFileStatusRpcs, + millis(single.mean()), millis(concurrent.mean()), + millis(single.percentile(0.99)), millis(concurrent.percentile(0.99)), + single.opsPerSecond(), concurrent.opsPerSecond(), + concurrent.opsPerSecond() / single.opsPerSecond())); + } + + /** + * Invariants that hold on both the baseline and this branch, so the identical + * benchmark passes in either worktree: every getFileStatus still issues its + * getFileStatus RPC, and the InfoBucket RPCs lie between the best case (zero — + * the server-side path removes the RPC) and the baseline worst case (one per + * call). + */ + private void assertWorkloadInvariants(Result r) { + assertThat(r.getFileStatusRpcs).isEqualTo(TOTAL_CALLS); + assertThat(r.infoBucketRpcs) + .isBetween(0L, (long) TOTAL_CALLS); + } + + private void logResult(String label, Result r) { + double infoBucketEliminated = + (TOTAL_CALLS - r.infoBucketRpcs) / (double) TOTAL_CALLS; + LOG.info(String.format("%n" + + "OFS getFileStatus RPC benchmark (HDDS-15925) — %s%n" + + " buckets=%d, accesses/bucket=%d, total getFileStatus=%d, " + + "threads=%d%n" + + " ------------------------------------------------------------%n" + + " InfoBucket RPCs : %d%n" + + " getFileStatus RPCs : %d%n" + + " InfoBucket RPCs per call : %.2f%n" + + " InfoBucket RPCs eliminated : %.0f%%%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 getFileStatus/s%n", + label, NUM_BUCKETS, ACCESSES_PER_BUCKET, TOTAL_CALLS, r.threads, + r.infoBucketRpcs, r.getFileStatusRpcs, + r.infoBucketRpcs / (double) TOTAL_CALLS, + infoBucketEliminated * 100, + 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]; + try (FileSystem fs = FileSystem.newInstance(URI.create(rootPath), runConf)) { + long bucketInfosBefore = metrics.getNumBucketInfos(); + long getFileStatusBefore = metrics.getNumGetFileStatus(); + long elapsedNanos = threads == 1 + ? runSequential(fs, latencyNanos) + : runConcurrent(fs, threads, latencyNanos); + return new Result(threads, + metrics.getNumBucketInfos() - bucketInfosBefore, + metrics.getNumGetFileStatus() - getFileStatusBefore, + 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.getFileStatus(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 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.getFileStatus(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 infoBucketRpcs; + private final long getFileStatusRpcs; + private final long elapsedNanos; + private final long[] sortedLatencyNanos; + + Result(int threads, long infoBucketRpcs, long getFileStatusRpcs, + long elapsedNanos, long[] latencyNanos) { + this.threads = threads; + this.infoBucketRpcs = infoBucketRpcs; + this.getFileStatusRpcs = getFileStatusRpcs; + 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/TestOmAcls.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOmAcls.java index 90d6b128e745..30190c75d5bf 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOmAcls.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOmAcls.java @@ -46,6 +46,7 @@ import org.apache.hadoop.ozone.client.OzoneClient; import org.apache.hadoop.ozone.om.exceptions.OMException; import org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes; +import org.apache.hadoop.ozone.om.helpers.BucketLayout; import org.apache.hadoop.ozone.security.acl.IAccessAuthorizer; import org.apache.hadoop.ozone.security.acl.IOzoneObj; import org.apache.hadoop.ozone.security.acl.OzoneObj; @@ -193,7 +194,8 @@ public void testReadKeyPermissionDenied() throws Exception { @Test public void testGetFileStatusPermissionDenied() throws Exception { - OzoneBucket bucket = DataTestUtil.createVolumeAndBucket(client); + OzoneBucket bucket = DataTestUtil.createVolumeAndBucket(client, + BucketLayout.FILE_SYSTEM_OPTIMIZED); DataTestUtil.createKey(bucket, "testKey", "testcontent".getBytes(StandardCharsets.UTF_8)); authorizer.keyAclAllow = false; diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/snapshot/OmSnapshotTests.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/snapshot/OmSnapshotTests.java index fe8f4b24e7e1..516a87386a8a 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/snapshot/OmSnapshotTests.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/snapshot/OmSnapshotTests.java @@ -65,6 +65,7 @@ import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assertions.fail; +import static org.junit.jupiter.api.Assumptions.assumeFalse; import static org.junit.jupiter.api.Assumptions.assumeTrue; import com.google.common.collect.Lists; @@ -527,6 +528,7 @@ public void checkKey() throws Exception { KeyInfoWithVolumeContext fileInfo = writeClient.getKeyInfo(keyArgs, false); assertEquals(fileInfo.getKeyInfo().getKeyName(), snapshotKeyPrefix + key1); + assumeFalse(bucketLayout.equals(BucketLayout.OBJECT_STORE)); OzoneFileStatus ozoneFileStatus = writeClient.getFileStatus(keyArgs); assertEquals(ozoneFileStatus.getKeyInfo().getKeyName(), snapshotKeyPrefix + key1); 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 434d05132bf5..d6db1cb35d81 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 @@ -53,6 +53,7 @@ import org.apache.hadoop.ozone.om.helpers.OmBucketInfo; import org.apache.hadoop.ozone.om.helpers.OmKeyArgs; import org.apache.hadoop.ozone.om.helpers.OmKeyInfo; +import org.apache.hadoop.ozone.om.helpers.OzoneFSUtils; import org.apache.hadoop.ozone.om.helpers.OzoneFileStatus; import org.apache.hadoop.ozone.om.helpers.OzoneFileStatusLight; import org.apache.hadoop.ozone.om.helpers.S3VolumeContext; @@ -307,6 +308,18 @@ public OzoneFileStatus getFileStatus(OmKeyArgs args) throws IOException { args = bucket.update(args); 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(getResourceType(args), 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 bb5d7fb289dd..547ce05e09be 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 @@ -26,9 +26,11 @@ import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.mockStatic; +import static org.mockito.Mockito.never; import static org.mockito.Mockito.reset; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; @@ -47,6 +49,7 @@ import org.apache.hadoop.ozone.audit.AuditLogger; import org.apache.hadoop.ozone.om.exceptions.OMException; import org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes; +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.OzoneFileStatus; @@ -703,4 +706,64 @@ private RequestContext getContext() { return context; } } + + @Test + public void getFileStatusRejectsObjectStoreLayout() 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.getFileStatus(keyArgs)); + assertEquals(ResultCodes.NOT_SUPPORTED_OPERATION, exception.getResult()); + assertTrue(exception.getMessage().contains("obs-bucket")); + assertTrue(exception.getMessage().contains("OBJECT_STORE")); + verify(keyManager, never()).getFileStatus(any(), anyString()); + } + + @Test + public void getFileStatusAllowsLegacyLayout() 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)); + OzoneFileStatus expectedStatus = new OzoneFileStatus(); + when(keyManager.getFileStatus(any(OmKeyArgs.class), anyString())) + .thenReturn(expectedStatus); + + 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(); + + OzoneFileStatus status = reader.getFileStatus(keyArgs); + assertEquals(expectedStatus, status); + verify(keyManager).getFileStatus(any(OmKeyArgs.class), anyString()); + } } 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 261f3a83761b..9b1ed021267f 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 @@ -76,6 +76,7 @@ import org.apache.hadoop.ozone.OmUtils; import org.apache.hadoop.ozone.OzoneConfigKeys; import org.apache.hadoop.ozone.OzoneFsServerDefaults; +import org.apache.hadoop.ozone.OzoneManagerVersion; import org.apache.hadoop.ozone.client.BucketArgs; import org.apache.hadoop.ozone.client.ObjectStore; import org.apache.hadoop.ozone.client.OzoneBucket; @@ -345,6 +346,12 @@ private OzoneBucket getBucket(String volumeStr, String bucketStr, } // Try get bucket again bucket = proxy.getBucketDetails(volumeStr, bucketStr); + + BucketLayout resolvedBucketLayout = + OzoneClientUtils.resolveLinkBucketLayout(bucket, objectStore, + new HashSet<>()); + + OzoneFSUtils.validateBucketLayout(bucket.getName(), resolvedBucketLayout); } else { throw ex; } @@ -690,19 +697,40 @@ public FileStatusAdapter getFileStatus(String path, URI uri, * Return FileStatusAdapter based on OFSPath being a * valid bucket path or valid snapshot path. * Throws exception in case of failure. + * + *

Non-snapshot paths call OM GetFileStatus directly (HDDS-15925) without a + * prior InfoBucket RPC. OBJECT_STORE buckets are rejected by OM GetFileStatus. + * Mutating OFS operations still validate layout via {@link #getBucket(OFSPath, boolean)}. + * + *

The direct call is only safe once the OM performs the server-side + * OBJECT_STORE rejection. During a rolling upgrade a new client can talk to an + * older OM that lacks that check, so when the negotiated OM version predates + * {@link OzoneManagerVersion#GET_FILE_STATUS_REJECTS_OBS} we fall back to the + * pre-HDDS-15925 path that fetches the bucket and validates its layout + * client-side. */ private FileStatusAdapter getFileStatusForKeyOrSnapshot( OFSPath ofsPath, URI uri, Path qualifiedPath, String userName, boolean headOp) throws IOException { String key = ofsPath.getKeyName(); try { - OzoneBucket bucket = getBucket(ofsPath, false); if (ofsPath.isSnapshotPath()) { + OzoneBucket bucket = getBucket(ofsPath, false); OzoneVolume volume = objectStore.getVolume(ofsPath.getVolumeName()); return getFileStatusAdapterWithSnapshotIndicator( volume, bucket, uri); } else { - OzoneFileStatus status = bucket.getFileStatus(key, headOp); + OzoneFileStatus status; + if (proxy.getOmVersion() + .compareTo(OzoneManagerVersion.GET_FILE_STATUS_REJECTS_OBS) >= 0) { + status = proxy.getOzoneFileStatus(ofsPath.getVolumeName(), + ofsPath.getBucketName(), key, headOp); + } else { + // Older OM has no server-side OBJECT_STORE check; validate the bucket + // layout on the client, matching pre-HDDS-15925 behavior. + OzoneBucket bucket = getBucket(ofsPath, false); + status = bucket.getFileStatus(key, headOp); + } return toFileStatusAdapter(status, userName, uri, qualifiedPath, ofsPath.getNonKeyPath()); } @@ -711,6 +739,12 @@ private FileStatusAdapter getFileStatusForKeyOrSnapshot( throw new FileNotFoundException(key + ": No such file or directory!"); } else if (e.getResult() == OMException.ResultCodes.BUCKET_NOT_FOUND) { throw new FileNotFoundException(key + ": Bucket doesn't exist!"); + } else if (e.getResult() + == OMException.ResultCodes.NOT_SUPPORTED_OPERATION) { + // OM rejects getFileStatus on an OBJECT_STORE bucket (no file system + // semantics). Surface it as IllegalArgumentException, matching the + // pre-HDDS-15925 client-side layout check. + throw new IllegalArgumentException(e.getMessage()); } throw e; } @@ -748,6 +782,12 @@ public Collection getTrashRoots(boolean allUsers, Iterator bucketIter = volume.listBuckets(""); while (bucketIter.hasNext()) { OzoneBucket bucket = bucketIter.next(); + // OBJECT_STORE buckets have no file system semantics, so no trash + // root. Skip them; probing would fail getFileStatus with + // IllegalArgumentException and abort the whole scan. + if (BucketLayout.OBJECT_STORE.equals(bucket.getBucketLayout())) { + continue; + } Path bucketPath = new Path(volumePath, bucket.getName()); Path trashRoot = new Path(bucketPath, FileSystem.TRASH_PREFIX); if (allUsers) { diff --git a/hadoop-ozone/ozonefs-common/src/test/java/org/apache/hadoop/fs/ozone/TestBasicRootedOzoneClientAdapterHeadOp.java b/hadoop-ozone/ozonefs-common/src/test/java/org/apache/hadoop/fs/ozone/TestBasicRootedOzoneClientAdapterHeadOp.java index 8f67adef1346..25e067556d21 100644 --- a/hadoop-ozone/ozonefs-common/src/test/java/org/apache/hadoop/fs/ozone/TestBasicRootedOzoneClientAdapterHeadOp.java +++ b/hadoop-ozone/ozonefs-common/src/test/java/org/apache/hadoop/fs/ozone/TestBasicRootedOzoneClientAdapterHeadOp.java @@ -27,6 +27,7 @@ import static org.mockito.Mockito.CALLS_REAL_METHODS; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -40,9 +41,11 @@ import org.apache.hadoop.hdds.client.RatisReplicationConfig; import org.apache.hadoop.hdds.protocol.proto.HddsProtos; import org.apache.hadoop.ozone.OFSPath; +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.OzoneVolume; +import org.apache.hadoop.ozone.client.protocol.ClientProtocol; import org.apache.hadoop.ozone.om.exceptions.OMException; import org.apache.hadoop.ozone.om.helpers.OmKeyInfo; import org.apache.hadoop.ozone.om.helpers.OzoneFileStatus; @@ -62,15 +65,22 @@ public class TestBasicRootedOzoneClientAdapterHeadOp { private BasicRootedOzoneClientAdapterImpl adapter; private OzoneBucket bucket; + private ClientProtocol proxy; @BeforeEach public void setUp() throws Exception { adapter = mock(BasicRootedOzoneClientAdapterImpl.class, CALLS_REAL_METHODS); bucket = mock(OzoneBucket.class); + proxy = mock(ClientProtocol.class); + when(proxy.getOmVersion()) + .thenReturn(OzoneManagerVersion.GET_FILE_STATUS_REJECTS_OBS); doReturn(bucket).when(adapter).getBucket(any(OFSPath.class), eq(false)); - // Inject a mock object store so the volume/snapshot dispatch branches can - // run without a live OM connection. + Field proxyField = + BasicRootedOzoneClientAdapterImpl.class.getDeclaredField("proxy"); + proxyField.setAccessible(true); + proxyField.set(adapter, proxy); + OzoneVolume volume = mock(OzoneVolume.class); when(volume.getName()).thenReturn("vol"); when(volume.getOwner()).thenReturn("user"); @@ -101,25 +111,45 @@ private static OzoneFileStatus fileStatus(boolean isDir) { @Test public void keyPathThreadsHeadOp() throws IOException { - when(bucket.getFileStatus(anyString(), anyBoolean())) + when(proxy.getOzoneFileStatus(anyString(), anyString(), anyString(), anyBoolean())) .thenReturn(fileStatus(false)); assertFalse(adapter.getFileStatus("/vol/bucket/key", URI_OFS, WORKING_DIR, "user", true).isDir()); ArgumentCaptor headOp = ArgumentCaptor.forClass(Boolean.class); - verify(bucket).getFileStatus(anyString(), headOp.capture()); + verify(proxy).getOzoneFileStatus(eq("vol"), eq("bucket"), eq("key"), + headOp.capture()); assertTrue(headOp.getValue()); } @Test public void fourArgOverloadDoesNotUseHeadOp() throws IOException { - when(bucket.getFileStatus(anyString(), anyBoolean())) + when(proxy.getOzoneFileStatus(anyString(), anyString(), anyString(), anyBoolean())) .thenReturn(fileStatus(true)); assertTrue(adapter.getFileStatus("/vol/bucket/key", URI_OFS, WORKING_DIR, "user").isDir()); - verify(bucket).getFileStatus(anyString(), eq(false)); + verify(proxy).getOzoneFileStatus(eq("vol"), eq("bucket"), eq("key"), eq(false)); + } + + @Test + public void olderOmFallsBackToClientSideBucketCheck() throws IOException { + // An OM older than GET_FILE_STATUS_REJECTS_OBS has no server-side + // OBJECT_STORE rejection, so the adapter must use the pre-HDDS-15925 path: + // fetch the bucket (which validates layout client-side) and call + // OzoneBucket#getFileStatus, without issuing a direct OM GetFileStatus. + when(proxy.getOmVersion()) + .thenReturn(OzoneManagerVersion.S3_BUCKET_TAGGING_API); + when(bucket.getFileStatus(anyString(), anyBoolean())) + .thenReturn(fileStatus(false)); + + assertFalse(adapter.getFileStatus("/vol/bucket/key", URI_OFS, WORKING_DIR, + "user", true).isDir()); + + verify(bucket).getFileStatus(eq("key"), eq(true)); + verify(proxy, never()) + .getOzoneFileStatus(anyString(), anyString(), anyString(), anyBoolean()); } @Test @@ -130,7 +160,7 @@ public void rootPathReturnsDirectory() throws IOException { @Test public void fileNotFoundMappedToFileNotFoundException() throws IOException { - when(bucket.getFileStatus(anyString(), anyBoolean())) + when(proxy.getOzoneFileStatus(anyString(), anyString(), anyString(), anyBoolean())) .thenThrow(new OMException("missing", OMException.ResultCodes.FILE_NOT_FOUND)); assertThrows(FileNotFoundException.class, @@ -140,7 +170,7 @@ public void fileNotFoundMappedToFileNotFoundException() throws IOException { @Test public void otherOMExceptionPropagates() throws IOException { - when(bucket.getFileStatus(anyString(), anyBoolean())) + when(proxy.getOzoneFileStatus(anyString(), anyString(), anyString(), anyBoolean())) .thenThrow(new OMException("boom", OMException.ResultCodes.INTERNAL_ERROR)); assertThrows(OMException.class, @@ -150,7 +180,7 @@ public void otherOMExceptionPropagates() throws IOException { @Test public void bucketNotFoundMappedToFileNotFoundException() throws IOException { - when(bucket.getFileStatus(anyString(), anyBoolean())) + when(proxy.getOzoneFileStatus(anyString(), anyString(), anyString(), anyBoolean())) .thenThrow(new OMException("no bucket", OMException.ResultCodes.BUCKET_NOT_FOUND)); assertThrows(FileNotFoundException.class, @@ -169,7 +199,6 @@ public void snapshotIndicatorPathReturnsDirectory() throws IOException { when(bucket.getVolumeName()).thenReturn("vol"); when(bucket.getName()).thenReturn("bucket"); when(bucket.getCreationTime()).thenReturn(Instant.EPOCH); - // keyName == ".snapshot" is the snapshot indicator path. assertTrue(adapter.getFileStatus("/vol/bucket/.snapshot", URI_OFS, WORKING_DIR, "user", true).isDir()); } diff --git a/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/client/ClientProtocolStub.java b/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/client/ClientProtocolStub.java index 40e384d2c547..816922f5dac4 100644 --- a/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/client/ClientProtocolStub.java +++ b/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/client/ClientProtocolStub.java @@ -34,6 +34,7 @@ import org.apache.hadoop.io.Text; import org.apache.hadoop.ozone.OzoneAcl; import org.apache.hadoop.ozone.OzoneFsServerDefaults; +import org.apache.hadoop.ozone.OzoneManagerVersion; import org.apache.hadoop.ozone.client.io.OzoneDataStreamOutput; import org.apache.hadoop.ozone.client.io.OzoneInputStream; import org.apache.hadoop.ozone.client.io.OzoneOutputStream; @@ -617,6 +618,11 @@ public TenantStateList listTenant() throws IOException { return null; } + @Override + public OzoneManagerVersion getOmVersion() { + return OzoneManagerVersion.CURRENT; + } + @Override public OzoneFsServerDefaults getServerDefaults() throws IOException { return null;