diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestRootedOzoneFileSystem.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestRootedOzoneFileSystem.java index 7a970d7bdc53..a5a2ec056a31 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestRootedOzoneFileSystem.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestRootedOzoneFileSystem.java @@ -86,6 +86,7 @@ import java.io.FileNotFoundException; import java.io.IOException; +import java.net.URI; import java.nio.charset.StandardCharsets; import java.nio.file.Paths; import java.security.PrivilegedExceptionAction; @@ -116,6 +117,7 @@ import static org.apache.hadoop.hdds.client.ECReplicationConfig.EcCodec.RS; import static org.apache.hadoop.ozone.OzoneAcl.AclScope.ACCESS; import static org.apache.hadoop.ozone.OzoneConfigKeys.OZONE_FS_ITERATE_BATCH_SIZE; +import static org.apache.hadoop.ozone.OzoneConfigKeys.OZONE_FS_LISTING_PAGE_SIZE; import static org.apache.hadoop.ozone.OzoneConsts.OZONE_URI_DELIMITER; import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_OM_ADDRESS_KEY; import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_OM_ENABLE_OFS_SHARED_TMP_DIR; @@ -150,28 +152,31 @@ public class TestRootedOzoneFileSystem { @Parameterized.Parameters public static Collection data() { return Arrays.asList( - new Object[]{true, true, true}, - new Object[]{true, true, false}, - new Object[]{true, false, false}, - new Object[]{false, true, false}, - new Object[]{false, false, false} + new Object[]{true, true, true, false}, + new Object[]{true, true, false, false}, + new Object[]{true, false, false, false}, + new Object[]{false, true, false, false}, + new Object[]{false, false, false, false}, + new Object[]{true, true, false, true}, + new Object[]{false, false, false, true} ); } public TestRootedOzoneFileSystem(boolean setDefaultFs, - boolean enableOMRatis, boolean isAclEnabled) { + boolean enableOMRatis, boolean isAclEnabled, boolean noFlush) { // Ignored. Actual init done in initParam(). // This empty constructor is still required to avoid argument exception. } @Parameterized.BeforeParam - public static void initParam(boolean setDefaultFs, - boolean enableOMRatis, boolean isAclEnabled) + public static void initParam(boolean setDefaultFs, boolean enableOMRatis, + boolean isAclEnabled, boolean noFlush) throws IOException, InterruptedException, TimeoutException { // Initialize the cluster before EACH set of parameters enabledFileSystemPaths = setDefaultFs; omRatisEnabled = enableOMRatis; enableAcl = isAclEnabled; + useOnlyCache = noFlush; initClusterAndEnv(); } @@ -218,6 +223,8 @@ public static Path getBucketPath() { private static boolean isBucketFSOptimized = false; private static boolean enableAcl; + private static boolean useOnlyCache; + private static OzoneConfiguration conf; private static MiniOzoneCluster cluster = null; private static FileSystem fs; @@ -291,6 +298,18 @@ public static void initClusterAndEnv() throws IOException, userOfs = UGI_USER1.doAs( (PrivilegedExceptionAction)() -> (RootedOzoneFileSystem) FileSystem.get(conf)); + + if (useOnlyCache) { + if (omRatisEnabled) { + cluster.getOzoneManager().getOmRatisServer().getOmStateMachine() + .getOzoneManagerDoubleBuffer().stopDaemon(); + } else { + cluster.getOzoneManager().getOmServerProtocol() + .getOzoneManagerDoubleBuffer().stopDaemon(); + cluster.getOzoneManager().getOmServerProtocol() + .setShouldFlushCache(false); + } + } } protected OMMetrics getOMMetrics() { @@ -446,8 +465,9 @@ public void testListStatus() throws Exception { ContractTestUtils.touch(fs, file4); fileStatuses = ofs.listStatus(parent); Assert.assertEquals( - "FileStatus did not return all children of the directory", - 3, fileStatuses.length); + "FileStatus did not return all children of" + + " the directory : Got " + Arrays.toString( + fileStatuses), 3, fileStatuses.length); // Cleanup fs.delete(parent, true); @@ -563,59 +583,55 @@ public void testListStatusIteratorWithPathNotFound() throws Exception { */ @Test public void testListStatusIteratorOnPageSize() throws Exception { - int[] pageSize = { - 1, LISTING_PAGE_SIZE, LISTING_PAGE_SIZE + 1, - LISTING_PAGE_SIZE - 1, LISTING_PAGE_SIZE + LISTING_PAGE_SIZE / 2, - LISTING_PAGE_SIZE + LISTING_PAGE_SIZE + final int pageSize = 32; + int[] dirCounts = { + 1, + pageSize - 1, + pageSize, + pageSize + 1, + pageSize + pageSize / 2, + pageSize + pageSize }; - for (int numDir : pageSize) { - int range = numDir / LISTING_PAGE_SIZE; - switch (range) { - case 0: - listStatusIterator(numDir); - break; - case 1: - listStatusIterator(numDir); - break; - case 2: - listStatusIterator(numDir); - break; - default: - listStatusIterator(numDir); - } - } - } - - private void listStatusIterator(int numDirs) throws IOException { + OzoneConfiguration config = new OzoneConfiguration(conf); + config.setInt(OZONE_FS_LISTING_PAGE_SIZE, pageSize); + URI uri = FileSystem.getDefaultUri(config); + config.setBoolean( + String.format("fs.%s.impl.disable.cache", uri.getScheme()), true); + FileSystem subject = FileSystem.get(uri, config); Path root = new Path("/" + volumeName + "/" + bucketName); - Set paths = new TreeSet<>(); + Path dir = new Path(root, "listStatusIterator"); try { - for (int i = 0; i < numDirs; i++) { - Path p = new Path(root, String.valueOf(i)); - fs.mkdirs(p); - paths.add(p.getName()); + Set paths = new TreeSet<>(); + for (int dirCount : dirCounts) { + listStatusIterator(subject, dir, paths, dirCount); } + } finally { + subject.delete(dir, true); + } + } - RemoteIterator iterator = ofs.listStatusIterator(root); - int iCount = 0; - if (iterator != null) { - while (iterator.hasNext()) { - FileStatus fileStatus = iterator.next(); - iCount++; - Assert.assertTrue(paths.contains(fileStatus.getPath().getName())); - } - } - Assert.assertEquals( - "Total directories listed do not match the existing directories", - numDirs, iCount); + private static void listStatusIterator(FileSystem subject, + Path dir, Set paths, int total) throws IOException { + for (int i = paths.size(); i < total; i++) { + Path p = new Path(dir, String.valueOf(i)); + subject.mkdirs(p); + paths.add(p.getName()); + } - } finally { - // Cleanup - for (int i = 0; i < numDirs; i++) { - Path p = new Path(root, String.valueOf(i)); - fs.delete(p, true); + RemoteIterator iterator = subject.listStatusIterator(dir); + int iCount = 0; + if (iterator != null) { + while (iterator.hasNext()) { + FileStatus fileStatus = iterator.next(); + iCount++; + String filename = fileStatus.getPath().getName(); + assertTrue(filename + " not found", paths.contains(filename)); } } + + assertEquals( + "Total directories listed do not match the existing directories", + total, iCount); } /** @@ -2414,6 +2430,9 @@ private void createLinkBucket(String linkVolume, String linkBucket, @Test public void testSnapshotRead() throws Exception { + if (useOnlyCache) { + return; + } // Init data OzoneBucket bucket1 = TestDataUtil.createVolumeAndBucket(client, bucketLayout); @@ -2460,6 +2479,9 @@ public void testFileSystemDeclaresCapability() throws Throwable { @Test public void testSnapshotDiff() throws Exception { + if (useOnlyCache) { + return; + } OzoneBucket bucket1 = TestDataUtil.createVolumeAndBucket(client, bucketLayout); Path volumePath1 = new Path(OZONE_URI_DELIMITER, bucket1.getVolumeName()); diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestRootedOzoneFileSystemWithFSO.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestRootedOzoneFileSystemWithFSO.java index 0ae54e1b63a2..7b0eb6882bb0 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestRootedOzoneFileSystemWithFSO.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestRootedOzoneFileSystemWithFSO.java @@ -57,14 +57,16 @@ public class TestRootedOzoneFileSystemWithFSO @Parameterized.Parameters public static Collection data() { return Arrays.asList( - new Object[]{true, true, false}, - new Object[]{true, false, false} + new Object[]{true, true, false, false}, + new Object[]{true, false, false, false}, + new Object[]{true, true, false, true}, + new Object[]{true, false, false, true} ); } public TestRootedOzoneFileSystemWithFSO(boolean setDefaultFs, - boolean enableOMRatis, boolean enableAcl) { - super(setDefaultFs, enableOMRatis, enableAcl); + boolean enableOMRatis, boolean isAclEnabled, boolean noFlush) { + super(setDefaultFs, enableOMRatis, isAclEnabled, noFlush); } @BeforeClass diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java index a23088211231..1e62c5c57caa 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java @@ -1441,7 +1441,13 @@ public static boolean isKeyDeleted(String key, Table keyTable) { private void listStatusFindKeyInTableCache( Iterator, CacheValue>> cacheIter, String keyArgs, String startCacheKey, boolean recursive, - TreeMap cacheKeyMap) { + TreeMap cacheKeyMap) throws IOException { + + Map remainingKeys = new HashMap<>(); + // extract the /volume/buck/ prefix from the startCacheKey + int volBuckEndIndex = StringUtils.ordinalIndexOf( + startCacheKey, OZONE_URI_DELIMITER, 3); + String volumeBuckPrefix = startCacheKey.substring(0, volBuckEndIndex + 1); while (cacheIter.hasNext()) { Map.Entry, CacheValue> entry = @@ -1452,14 +1458,14 @@ private void listStatusFindKeyInTableCache( } OmKeyInfo cacheOmKeyInfo = entry.getValue().getCacheValue(); // cacheOmKeyInfo is null if an entry is deleted in cache - if (cacheOmKeyInfo != null - && cacheKey.startsWith(startCacheKey) - && cacheKey.compareTo(startCacheKey) >= 0) { + if (cacheOmKeyInfo != null && cacheKey.startsWith( + keyArgs) && cacheKey.compareTo(startCacheKey) >= 0) { if (!recursive) { String remainingKey = StringUtils.stripEnd(cacheKey.substring( - startCacheKey.length()), OZONE_URI_DELIMITER); + keyArgs.length()), OZONE_URI_DELIMITER); // For non-recursive, the remaining part of key can't have '/' if (remainingKey.contains(OZONE_URI_DELIMITER)) { + remainingKeys.put(cacheKey, cacheOmKeyInfo); continue; } } @@ -1474,6 +1480,31 @@ private void listStatusFindKeyInTableCache( cacheKeyMap.put(cacheKey, null); } } + + // let's say fsPaths is disabled, then creating a key like a/b/c + // will not create intermediate keys in the keyTable so only entry + // in the keyTable would be {a/b/c}. This would be skipped from getting + // added to cacheKeyMap above as remainingKey would be {b/c} and it + // contains the slash, In this case we track such keys which are not added + // to the map, find the immediate child and check if they are present in + // the map. If not create a fake dir and add it. This is similar to the + // logic in findKeyInDbWithIterator. + if (!recursive) { + for (Map.Entry entry : remainingKeys.entrySet()) { + String remainingKey = entry.getKey(); + String immediateChild = + OzoneFSUtils.getImmediateChild(remainingKey, keyArgs); + if (!cacheKeyMap.containsKey(immediateChild)) { + // immediateChild contains volume/bucket prefix remove it. + String immediateChildKeyName = + immediateChild.replaceAll(volumeBuckPrefix, ""); + OmKeyInfo fakeDirEntry = + createDirectoryKey(entry.getValue(), immediateChildKeyName); + cacheKeyMap.put(immediateChild, + new OzoneFileStatus(fakeDirEntry, scmBlockSize, true)); + } + } + } } /** @@ -1521,14 +1552,15 @@ public List listStatus(OmKeyArgs args, boolean recursive, String startKey, long numEntries, String clientAddress, boolean allowPartialPrefixes) throws IOException { Preconditions.checkNotNull(args, "Key args can not be null"); - String volName = args.getVolumeName(); - String buckName = args.getBucketName(); + String volumeName = args.getVolumeName(); + String bucketName = args.getBucketName(); + String keyName = args.getKeyName(); List fileStatusList = new ArrayList<>(); if (numEntries <= 0) { return fileStatusList; } - if (isBucketFSOptimized(volName, buckName)) { + if (isBucketFSOptimized(volumeName, bucketName)) { Preconditions.checkArgument(!recursive); OzoneListStatusHelper statusHelper = new OzoneListStatusHelper(metadataManager, scmBlockSize, @@ -1540,9 +1572,6 @@ public List listStatus(OmKeyArgs args, boolean recursive, return buildFinalStatusList(statuses, args, clientAddress); } - String volumeName = args.getVolumeName(); - String bucketName = args.getBucketName(); - String keyName = args.getKeyName(); // A map sorted by OmKey to combine results from TableCache and DB. TreeMap cacheKeyMap = new TreeMap<>(); @@ -1564,8 +1593,8 @@ public List listStatus(OmKeyArgs args, boolean recursive, metadataManager.getLock().acquireReadLock(BUCKET_LOCK, volumeName, bucketName); try { - keyTable = metadataManager - .getKeyTable(getBucketLayout(metadataManager, volName, buckName)); + keyTable = metadataManager.getKeyTable( + getBucketLayout(metadataManager, volumeName, bucketName)); iterator = getIteratorForKeyInTableCache(recursive, startKey, volumeName, bucketName, cacheKeyMap, keyArgs, keyTable); } finally { @@ -1687,7 +1716,11 @@ private void findKeyInDbWithIterator(boolean recursive, String startKey, if (!entryKeyName.equals(immediateChild)) { OmKeyInfo fakeDirEntry = createDirectoryKey( omKeyInfo, immediateChild); - cacheKeyMap.put(entryInDb, + String fakeDirKey = ozoneManager.getMetadataManager() + .getOzoneKey(fakeDirEntry.getVolumeName(), + fakeDirEntry.getBucketName(), + fakeDirEntry.getKeyName()); + cacheKeyMap.put(fakeDirKey, new OzoneFileStatus(fakeDirEntry, scmBlockSize, true)); } else { diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmMetadataManagerImpl.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmMetadataManagerImpl.java index e6b027427d23..5616048ffd28 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmMetadataManagerImpl.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmMetadataManagerImpl.java @@ -1486,6 +1486,9 @@ private List listAllVolumes(String prefix, String startKey, Map.Entry, CacheValue> entry = cacheIterator.next(); omVolumeArgs = entry.getValue().getCacheValue(); + if (omVolumeArgs == null) { + continue; + } volumeName = omVolumeArgs.getVolume(); if (!prefixIsEmpty && !volumeName.startsWith(prefix)) { diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/TrashOzoneFileSystem.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/TrashOzoneFileSystem.java index 115a8c5f8e0a..bc650339dbdd 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/TrashOzoneFileSystem.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/TrashOzoneFileSystem.java @@ -308,6 +308,9 @@ public Collection getTrashRoots(boolean allUsers) { Map.Entry, CacheValue> entry = bucketIterator.next(); OmBucketInfo omBucketInfo = entry.getValue().getCacheValue(); + if (omBucketInfo == null) { + continue; + } Path volumePath = new Path(OZONE_URI_DELIMITER, omBucketInfo.getVolumeName()); Path bucketPath = new Path(volumePath, omBucketInfo.getBucketName()); diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/ratis/OzoneManagerStateMachine.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/ratis/OzoneManagerStateMachine.java index ddaa066373fa..994c81cc5a27 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/ratis/OzoneManagerStateMachine.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/ratis/OzoneManagerStateMachine.java @@ -743,4 +743,9 @@ public long getTermForIndex(long transactionIndex) { public void awaitDoubleBufferFlush() throws InterruptedException { ozoneManagerDoubleBuffer.awaitFlush(); } + + @VisibleForTesting + public OzoneManagerDoubleBuffer getOzoneManagerDoubleBuffer() { + return ozoneManagerDoubleBuffer; + } } diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/protocolPB/OzoneManagerProtocolServerSideTranslatorPB.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/protocolPB/OzoneManagerProtocolServerSideTranslatorPB.java index c8e9b679cf4d..d26cce0c1374 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/protocolPB/OzoneManagerProtocolServerSideTranslatorPB.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/protocolPB/OzoneManagerProtocolServerSideTranslatorPB.java @@ -77,6 +77,9 @@ public class OzoneManagerProtocolServerSideTranslatorPB implements ProtocolMessageEnum> dispatcher; private final RequestValidations requestValidations; + // always true, only used in tests + private boolean shouldFlushCache = true; + /** * Constructs an instance of the server handler. * @@ -284,7 +287,9 @@ private OMResponse submitRequestDirectlyToOM(OMRequest request) { return createErrorResponse(request, ex); } try { - omClientResponse.getFlushFuture().get(); + if (shouldFlushCache) { + omClientResponse.getFlushFuture().get(); + } if (LOG.isTraceEnabled()) { LOG.trace("Future for {} is completed", request); } @@ -340,4 +345,14 @@ public static Logger getLog() { public void awaitDoubleBufferFlush() throws InterruptedException { ozoneManagerDoubleBuffer.awaitFlush(); } + + @VisibleForTesting + public OzoneManagerDoubleBuffer getOzoneManagerDoubleBuffer() { + return ozoneManagerDoubleBuffer; + } + + @VisibleForTesting + public void setShouldFlushCache(boolean shouldFlushCache) { + this.shouldFlushCache = shouldFlushCache; + } }