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 e479cbb661e..2874c08e6f5 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 @@ -1847,6 +1847,58 @@ void testGetFileStatus() throws Exception { fs.delete(volume, false); } + @Test + void testGetFileStatusUsesSingleOmRpcAfterCacheWarm() throws Exception { + Path path = new Path(bucketPath, "single-rpc-stat-test"); + fs.mkdirs(path); + // Warm the bucket layout cache; ignore metrics from the first stat. + fs.getFileStatus(path); + + long getFileStatusBefore = getOMMetrics().getNumGetFileStatus(); + long bucketInfoBefore = getOMMetrics().getNumBucketInfos(); + + FileStatus status = fs.getFileStatus(path); + assertTrue(status.isDirectory()); + + assertEquals(getFileStatusBefore + 1, getOMMetrics().getNumGetFileStatus()); + assertEquals(bucketInfoBefore, getOMMetrics().getNumBucketInfos()); + } + + @Test + void testGetFileStatusOnBucketRoot() throws Exception { + FileStatus status = fs.getFileStatus(bucketPath); + assertTrue(status.isDirectory()); + } + + @Test + void testGetFileStatusOnObjectStoreBucketRejectsInvalidLayout() + throws Exception { + OzoneBucket obsBucket = + TestDataUtil.createVolumeAndBucket(client, BucketLayout.OBJECT_STORE); + Path obsBucketPath = new Path( + new Path(OZONE_URI_DELIMITER + obsBucket.getVolumeName()), + obsBucket.getName()); + try { + IllegalArgumentException exception = assertThrows( + IllegalArgumentException.class, () -> fs.getFileStatus(obsBucketPath)); + assertThat(exception.getMessage()) + .contains(BucketLayout.OBJECT_STORE.name()); + } finally { + objectStore.getVolume(obsBucket.getVolumeName()) + .deleteBucket(obsBucket.getName()); + objectStore.deleteVolume(obsBucket.getVolumeName()); + } + } + + @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 void testUnbuffer() throws IOException { String testKeyName = "testKey2"; 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 50842949af2..85a01fced0c 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 @@ -41,6 +41,7 @@ import java.util.Iterator; import java.util.List; import java.util.Objects; +import java.util.concurrent.ConcurrentHashMap; import java.util.stream.Collectors; import org.apache.commons.collections4.CollectionUtils; import org.apache.commons.lang3.StringUtils; @@ -132,6 +133,11 @@ public class BasicRootedOzoneClientAdapterImpl private BucketLayout defaultOFSBucketLayout; private final OzoneConfiguration config; private final OzoneClientConfig clientConfig; + /** + * Cache of successfully validated bucket layouts for read paths. + */ + private final ConcurrentHashMap validatedBucketLayouts = + new ConcurrentHashMap<>(); /** * Create new OzoneClientAdapter implementation. @@ -274,6 +280,47 @@ OzoneBucket getBucket(OFSPath ofsPath, boolean createIfNotExist) createIfNotExist); } + private static String bucketLayoutCacheKey(String volumeStr, String bucketStr) { + return volumeStr + OZONE_URI_DELIMITER + bucketStr; + } + + private BucketLayout validateBucketLayoutForBucket(OzoneBucket bucket) + throws IOException { + BucketLayout resolvedBucketLayout = + OzoneClientUtils.resolveLinkBucketLayout(bucket, objectStore, + new HashSet<>()); + OzoneFSUtils.validateBucketLayout(bucket.getName(), resolvedBucketLayout); + return resolvedBucketLayout; + } + + private void cacheValidatedBucketLayout(String volumeStr, String bucketStr, + BucketLayout layout) { + validatedBucketLayouts.putIfAbsent( + bucketLayoutCacheKey(volumeStr, bucketStr), layout); + } + + private void validateAndCacheBucketLayout(String volumeStr, String bucketStr, + OzoneBucket bucket) throws IOException { + BucketLayout layout = validateBucketLayoutForBucket(bucket); + cacheValidatedBucketLayout(volumeStr, bucketStr, layout); + } + + private void resolveAndValidateBucketLayout(String volumeStr, String bucketStr) + throws IOException { + OzoneBucket bucket = proxy.getBucketDetails(volumeStr, bucketStr); + validateAndCacheBucketLayout(volumeStr, bucketStr, bucket); + } + + private void ensureBucketLayoutValid(OFSPath ofsPath) throws IOException { + String volumeStr = ofsPath.getVolumeName(); + String bucketStr = ofsPath.getBucketName(); + if (validatedBucketLayouts.containsKey( + bucketLayoutCacheKey(volumeStr, bucketStr))) { + return; + } + resolveAndValidateBucketLayout(volumeStr, bucketStr); + } + /** * Get OzoneBucket object to operate in. * Optionally create volume and bucket if not found. @@ -297,13 +344,7 @@ private OzoneBucket getBucket(String volumeStr, String bucketStr, OzoneBucket bucket; try { bucket = proxy.getBucketDetails(volumeStr, bucketStr); - - // resolve the bucket layout in case of Link Bucket - BucketLayout resolvedBucketLayout = - OzoneClientUtils.resolveLinkBucketLayout(bucket, objectStore, - new HashSet<>()); - - OzoneFSUtils.validateBucketLayout(bucket.getName(), resolvedBucketLayout); + validateAndCacheBucketLayout(volumeStr, bucketStr, bucket); } catch (OMException ex) { if (createIfNotExist) { // getBucketDetails can throw VOLUME_NOT_FOUND when the parent volume @@ -345,6 +386,7 @@ private OzoneBucket getBucket(String volumeStr, String bucketStr, } // Try get bucket again bucket = proxy.getBucketDetails(volumeStr, bucketStr); + validateAndCacheBucketLayout(volumeStr, bucketStr, bucket); } else { throw ex; } @@ -696,16 +738,17 @@ private FileStatusAdapter getFileStatusForKeyOrSnapshot( 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); - return toFileStatusAdapter(status, userName, uri, qualifiedPath, - ofsPath.getNonKeyPath()); } + ensureBucketLayoutValid(ofsPath); + OzoneFileStatus status = proxy.getOzoneFileStatus( + ofsPath.getVolumeName(), ofsPath.getBucketName(), key, headOp); + return toFileStatusAdapter(status, userName, uri, qualifiedPath, + ofsPath.getNonKeyPath()); } catch (OMException e) { if (e.getResult() == OMException.ResultCodes.FILE_NOT_FOUND) { throw new FileNotFoundException(key + ": No such file or directory!"); 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 8f67adef134..a5da0626694 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 @@ -36,6 +36,7 @@ import java.net.URI; import java.time.Instant; import java.util.Collections; +import java.util.concurrent.ConcurrentHashMap; import org.apache.hadoop.fs.Path; import org.apache.hadoop.hdds.client.RatisReplicationConfig; import org.apache.hadoop.hdds.protocol.proto.HddsProtos; @@ -43,7 +44,9 @@ 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.BucketLayout; import org.apache.hadoop.ozone.om.helpers.OmKeyInfo; import org.apache.hadoop.ozone.om.helpers.OzoneFileStatus; import org.junit.jupiter.api.BeforeEach; @@ -62,11 +65,13 @@ 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); doReturn(bucket).when(adapter).getBucket(any(OFSPath.class), eq(false)); // Inject a mock object store so the volume/snapshot dispatch branches can @@ -77,10 +82,29 @@ public void setUp() throws Exception { when(volume.getCreationTime()).thenReturn(Instant.EPOCH); ObjectStore objectStore = mock(ObjectStore.class); when(objectStore.getVolume(anyString())).thenReturn(volume); - Field field = - BasicRootedOzoneClientAdapterImpl.class.getDeclaredField("objectStore"); + setField(adapter, "objectStore", objectStore); + setField(adapter, "proxy", proxy); + warmLayoutCache("vol", "bucket"); + } + + private static void setField(Object target, String name, Object value) + throws Exception { + Field field = BasicRootedOzoneClientAdapterImpl.class.getDeclaredField(name); field.setAccessible(true); - field.set(adapter, objectStore); + field.set(target, value); + } + + @SuppressWarnings("unchecked") + private void warmLayoutCache(String volumeName, String bucketName) + throws Exception { + Field cacheField = + BasicRootedOzoneClientAdapterImpl.class.getDeclaredField( + "validatedBucketLayouts"); + cacheField.setAccessible(true); + ConcurrentHashMap cache = + new ConcurrentHashMap<>(); + cache.put(volumeName + "/" + bucketName, BucketLayout.FILE_SYSTEM_OPTIMIZED); + cacheField.set(adapter, cache); } private static OzoneFileStatus fileStatus(boolean isDir) { @@ -101,25 +125,27 @@ private static OzoneFileStatus fileStatus(boolean isDir) { @Test public void keyPathThreadsHeadOp() throws IOException { - when(bucket.getFileStatus(anyString(), anyBoolean())) - .thenReturn(fileStatus(false)); + when(proxy.getOzoneFileStatus(eq("vol"), eq("bucket"), 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"), anyString(), + headOp.capture()); assertTrue(headOp.getValue()); } @Test public void fourArgOverloadDoesNotUseHeadOp() throws IOException { - when(bucket.getFileStatus(anyString(), anyBoolean())) - .thenReturn(fileStatus(true)); + when(proxy.getOzoneFileStatus(eq("vol"), eq("bucket"), 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"), anyString(), + eq(false)); } @Test @@ -130,8 +156,8 @@ public void rootPathReturnsDirectory() throws IOException { @Test public void fileNotFoundMappedToFileNotFoundException() throws IOException { - when(bucket.getFileStatus(anyString(), anyBoolean())) - .thenThrow(new OMException("missing", + when(proxy.getOzoneFileStatus(eq("vol"), eq("bucket"), anyString(), + anyBoolean())).thenThrow(new OMException("missing", OMException.ResultCodes.FILE_NOT_FOUND)); assertThrows(FileNotFoundException.class, () -> adapter.getFileStatus("/vol/bucket/key", URI_OFS, WORKING_DIR, @@ -140,8 +166,8 @@ public void fileNotFoundMappedToFileNotFoundException() throws IOException { @Test public void otherOMExceptionPropagates() throws IOException { - when(bucket.getFileStatus(anyString(), anyBoolean())) - .thenThrow(new OMException("boom", + when(proxy.getOzoneFileStatus(eq("vol"), eq("bucket"), anyString(), + anyBoolean())).thenThrow(new OMException("boom", OMException.ResultCodes.INTERNAL_ERROR)); assertThrows(OMException.class, () -> adapter.getFileStatus("/vol/bucket/key", URI_OFS, WORKING_DIR, @@ -150,8 +176,8 @@ public void otherOMExceptionPropagates() throws IOException { @Test public void bucketNotFoundMappedToFileNotFoundException() throws IOException { - when(bucket.getFileStatus(anyString(), anyBoolean())) - .thenThrow(new OMException("no bucket", + when(proxy.getOzoneFileStatus(eq("vol"), eq("bucket"), anyString(), + anyBoolean())).thenThrow(new OMException("no bucket", OMException.ResultCodes.BUCKET_NOT_FOUND)); assertThrows(FileNotFoundException.class, () -> adapter.getFileStatus("/vol/bucket/key", URI_OFS, WORKING_DIR,