Skip to content

HDDS-11232. Avoid redundant getBucketInfo RPC on OFS getFileStatus #10792

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
jojochuang wants to merge 4 commits into
base: master
Choose a base branch
from
+148 −27
Open

HDDS-11232. Avoid redundant getBucketInfo RPC on OFS getFileStatus #10792

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
d5d8dc6
Avoid redundant getBucketInfo RPC on OFS getFileStatus.
jojochuang Jul 17, 2026
9a326e5
Fix OBS bucket cleanup in getFileStatus layout test.
jojochuang Jul 17, 2026
729623c
Retrigger CI after test cleanup fix.
jojochuang Jul 17, 2026
47a477d
Update headOp unit tests for direct getOzoneFileStatus path.
jojochuang Jul 17, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 52 additions & 0 deletions ...tion-test/src/test/java/org/apache/hadoop/fs/ozone/AbstractRootedOzoneFileSystemTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down
67 changes: 55 additions & 12 deletions ...fs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicRootedOzoneClientAdapterImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<String, BucketLayout> validatedBucketLayouts =
new ConcurrentHashMap<>();
Comment on lines +136 to +140

/**
* Create new OzoneClientAdapter implementation.
Expand Down Expand Up @@ -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.
Expand All @@ -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
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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!");
Expand Down
56 changes: 41 additions & 15 deletions ...mon/src/test/java/org/apache/hadoop/fs/ozone/TestBasicRootedOzoneClientAdapterHeadOp.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,14 +36,17 @@
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;
import org.apache.hadoop.ozone.OFSPath;
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;
Expand All @@ -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
Expand All @@ -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<String, BucketLayout> cache =
new ConcurrentHashMap<>();
cache.put(volumeName + "/" + bucketName, BucketLayout.FILE_SYSTEM_OPTIMIZED);
cacheField.set(adapter, cache);
Comment on lines +100 to +107
}

private static OzoneFileStatus fileStatus(boolean isDir) {
Expand All @@ -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<Boolean> 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
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand Down