Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ private void deleteFSOKey(OzoneBucket bucket, String keyName)
return;
}

if (bucket.getFileStatus(keyName).isDirectory()) {
if (bucket.getFileStatus(keyName, true).isDirectory()) {
List<OzoneFileStatus> ozoneFileStatusList =
bucket.listStatus(keyName, false, "", 1);
if (ozoneFileStatusList != null && !ozoneFileStatusList.isEmpty()) {
Expand All @@ -122,7 +122,7 @@ private void deleteFSOKey(OzoneBucket bucket, String keyName)

String toKeyName = new Path(userTrashCurrent, keyName).toUri().getPath();
if (isKeyExist(bucket, toKeyName)) {
if (bucket.getFileStatus(toKeyName).isDirectory()) {
if (bucket.getFileStatus(toKeyName, true).isDirectory()) {
// if directory already exist in trash, just delete the directory
bucket.deleteKey(keyName);
return;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -414,9 +414,16 @@ public boolean deleteObjects(List<String> keyNameList) {
public FileStatusAdapter getFileStatus(String key, URI uri,
Path qualifiedPath, String userName)
throws IOException {
return getFileStatus(key, uri, qualifiedPath, userName, false);
}

@Override
public FileStatusAdapter getFileStatus(String key, URI uri,
Path qualifiedPath, String userName, boolean headOp)
throws IOException {
try {
incrementCounter(Statistic.OBJECTS_QUERY, 1);
OzoneFileStatus status = bucket.getFileStatus(key);
OzoneFileStatus status = bucket.getFileStatus(key, headOp);
return toFileStatusAdapter(status, userName, uri, qualifiedPath);

} catch (OMException e) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -785,8 +785,15 @@ public Collection<FileStatus> getTrashRoots(boolean allUsers) {
try {
if (!allUsers) {
Path userTrash = new Path(trashRoot, userName);
if (exists(userTrash) && getFileStatus(userTrash).isDirectory()) {
ret.add(getFileStatus(userTrash));
// headOp: only the entry type is needed here. Fetch once instead of
// calling exists()/getFileStatus() repeatedly for the same path.
try {
FileStatus userTrashStatus = getFileStatus(userTrash, true);
if (userTrashStatus.isDirectory()) {
ret.add(userTrashStatus);
}
} catch (FileNotFoundException ignored) {
// No trash root for this user.
}
} else {
if (exists(trashRoot)) {
Expand Down Expand Up @@ -834,6 +841,16 @@ public long getDefaultBlockSize() {

@Override
public FileStatus getFileStatus(Path f) throws IOException {
return getFileStatus(f, false);
}

/**
* @param headOp when true, requests a metadata-only (type) check so the OM
* skips the pipeline refresh (SCM round-trip) and datanode
* sorting. Used by {@link #isDirectory(Path)}/{@link #isFile(Path)},
* which only need the entry type.
*/
private FileStatus getFileStatus(Path f, boolean headOp) throws IOException {
incrementCounter(Statistic.INVOCATION_GET_FILE_STATUS, 1);
statistics.incrementReadOps(1);
LOG.trace("getFileStatus() path:{}", f);
Expand All @@ -842,7 +859,7 @@ public FileStatus getFileStatus(Path f) throws IOException {
FileStatus fileStatus = null;
try {
fileStatus = convertFileStatus(
adapter.getFileStatus(key, uri, qualifiedPath, getUsername()));
adapter.getFileStatus(key, uri, qualifiedPath, getUsername(), headOp));
} catch (IOException ex) {
if (ex instanceof OMException) {
if (((OMException) ex).getResult()
Expand Down Expand Up @@ -1014,17 +1031,25 @@ public FileStatus[] globStatus(Path pathPattern, PathFilter filter)
}

@Override
@SuppressWarnings("deprecation")
public boolean isDirectory(Path f) throws IOException {
incrementCounter(Statistic.INVOCATION_IS_DIRECTORY);
return super.isDirectory(f);
try {
// headOp: only the entry type is needed, so skip the pipeline refresh.
return getFileStatus(f, true).isDirectory();
} catch (FileNotFoundException e) {
return false;
}
}

@Override
@SuppressWarnings("deprecation")
public boolean isFile(Path f) throws IOException {
incrementCounter(Statistic.INVOCATION_IS_FILE);
return super.isFile(f);
try {
// headOp: only the entry type is needed, so skip the pipeline refresh.
return getFileStatus(f, true).isFile();
} catch (FileNotFoundException e) {
return false;
}
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -760,9 +760,15 @@ public Collection<FileStatus> getTrashRoots(boolean allUsers,
}
} else {
Path userTrash = new Path(trashRoot, username);
if (fs.exists(userTrash) &&
fs.getFileStatus(userTrash).isDirectory()) {
ret.add(fs.getFileStatus(userTrash));
// Fetch the status once instead of exists() + two getFileStatus()
// calls for the same path.
try {
FileStatus userTrashStatus = fs.getFileStatus(userTrash);
if (userTrashStatus.isDirectory()) {
ret.add(userTrashStatus);
}
} catch (FileNotFoundException ignored) {
// No trash root for this user.
}
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,136 @@
/*
* 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.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.mockito.ArgumentMatchers.anyBoolean;
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.verify;
import static org.mockito.Mockito.when;

import java.io.FileNotFoundException;
import java.io.IOException;
import java.lang.reflect.Field;
import java.net.URI;
import java.util.Collections;
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.client.OzoneBucket;
import org.apache.hadoop.ozone.om.exceptions.OMException;
import org.apache.hadoop.ozone.om.helpers.OmKeyInfo;
import org.apache.hadoop.ozone.om.helpers.OzoneFileStatus;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.mockito.ArgumentCaptor;
import org.mockito.Mockito;

/**
* Unit tests for headOp propagation through
* {@link BasicOzoneClientAdapterImpl#getFileStatus} (HDDS-15877). Uses a partial
* mock so no OM connection is required.
*/
public class TestBasicOzoneClientAdapterHeadOp {

private static final URI URI_O3FS = URI.create("o3fs://bucket.vol/");
private static final Path WORKING_DIR = new Path("/");

private BasicOzoneClientAdapterImpl adapter;
private OzoneBucket bucket;

@BeforeEach
public void setUp() throws Exception {
adapter = mock(BasicOzoneClientAdapterImpl.class, CALLS_REAL_METHODS);
bucket = mock(OzoneBucket.class);
// Inject the mock bucket so getFileStatus can run without a live OM.
Field field =
BasicOzoneClientAdapterImpl.class.getDeclaredField("bucket");
field.setAccessible(true);
field.set(adapter, bucket);
}

@AfterEach
public void tearDown() {
// Ensure no Mockito stubbing state leaks into other test classes running in
// the same JVM.
Mockito.validateMockitoUsage();
}

private static OzoneFileStatus fileStatus(boolean isDir) {
OmKeyInfo keyInfo = new OmKeyInfo.Builder()
.setVolumeName("vol")
.setBucketName("bucket")
.setKeyName("key")
.setReplicationConfig(RatisReplicationConfig.getInstance(
HddsProtos.ReplicationFactor.THREE))
.setOmKeyLocationInfos(Collections.emptyList())
.setDataSize(0)
.setCreationTime(0)
.setModificationTime(0)
.setAcls(Collections.emptyList())
.build();
return new OzoneFileStatus(keyInfo, 512, isDir);
}

@Test
public void headOpOverloadThreadsHeadOp() throws IOException {
when(bucket.getFileStatus(anyString(), anyBoolean()))
.thenReturn(fileStatus(false));

assertFalse(adapter.getFileStatus("key", URI_O3FS, WORKING_DIR, "user", true)
.isDir());

ArgumentCaptor<Boolean> headOp = ArgumentCaptor.forClass(Boolean.class);
verify(bucket).getFileStatus(anyString(), headOp.capture());
assertTrue(headOp.getValue());
}

@Test
public void fourArgOverloadDoesNotUseHeadOp() throws IOException {
when(bucket.getFileStatus(anyString(), anyBoolean()))
.thenReturn(fileStatus(true));

assertTrue(adapter.getFileStatus("key", URI_O3FS, WORKING_DIR, "user")
.isDir());
verify(bucket).getFileStatus(anyString(), eq(false));
}

@Test
public void fileNotFoundMappedToFileNotFoundException() throws IOException {
when(bucket.getFileStatus(anyString(), anyBoolean()))
.thenThrow(new OMException("missing",
OMException.ResultCodes.FILE_NOT_FOUND));
assertThrows(FileNotFoundException.class,
() -> adapter.getFileStatus("key", URI_O3FS, WORKING_DIR, "user", true));
}

@Test
public void otherOMExceptionPropagates() throws IOException {
when(bucket.getFileStatus(anyString(), anyBoolean()))
.thenThrow(new OMException("boom",
OMException.ResultCodes.INTERNAL_ERROR));
assertThrows(OMException.class,
() -> adapter.getFileStatus("key", URI_O3FS, WORKING_DIR, "user", true));
}
}
Loading