This is an automated email from the ASF dual-hosted git repository. rakeshr pushed a commit to branch HDDS-2939 in repository https://gitbox.apache.org/repos/asf/ozone.git
commit ba6eee0065bf1fe137d03f1107f335c1de68368c Author: Rakesh Radhakrishnan <[email protected]> AuthorDate: Fri Jan 22 21:15:11 2021 +0530 HDDS-4717. Fix TestOzoneFileSystemV1 and TestObjectStoreV1 cases (#1815) --- .../hadoop/ozone/client/io/KeyOutputStream.java | 8 + .../apache/hadoop/fs/ozone/TestOzoneDirectory.java | 26 --- .../hadoop/fs/ozone/TestOzoneFileSystem.java | 9 +- .../apache/hadoop/ozone/om/TestObjectStoreV1.java | 192 ++++++++++++--------- .../org/apache/hadoop/ozone/om/KeyManagerImpl.java | 112 ++++++++---- .../ozone/om/request/key/OMKeyDeleteRequestV1.java | 6 + 6 files changed, 215 insertions(+), 138 deletions(-) diff --git a/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/io/KeyOutputStream.java b/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/io/KeyOutputStream.java index b2a4e92..106ed45 100644 --- a/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/io/KeyOutputStream.java +++ b/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/io/KeyOutputStream.java @@ -92,6 +92,8 @@ public class KeyOutputStream extends OutputStream { private boolean isException; private final BlockOutputStreamEntryPool blockOutputStreamEntryPool; + private long clientID; + /** * A constructor for testing purpose only. */ @@ -127,6 +129,11 @@ public class KeyOutputStream extends OutputStream { return retryCount; } + @VisibleForTesting + public long getClientID() { + return clientID; + } + @SuppressWarnings({"parameternumber", "squid:S00107"}) public KeyOutputStream( OzoneClientConfig config, @@ -158,6 +165,7 @@ public class KeyOutputStream extends OutputStream { this.retryCount = 0; this.isException = false; this.writeOffset = 0; + this.clientID = handler.getId(); } /** diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestOzoneDirectory.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestOzoneDirectory.java index 87e9f09..56c6177 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestOzoneDirectory.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestOzoneDirectory.java @@ -23,8 +23,6 @@ import org.apache.hadoop.fs.FileSystem; import org.apache.hadoop.fs.Path; import org.apache.hadoop.fs.CommonConfigurationKeysPublic; import org.apache.hadoop.hdds.conf.OzoneConfiguration; -import org.apache.hadoop.hdds.utils.db.Table; -import org.apache.hadoop.hdds.utils.db.TableIterator; import org.apache.hadoop.ozone.MiniOzoneCluster; import org.apache.hadoop.ozone.OzoneConsts; import org.apache.hadoop.ozone.TestDataUtil; @@ -33,7 +31,6 @@ import org.apache.hadoop.ozone.om.OMConfigKeys; import org.apache.hadoop.ozone.om.OMMetadataManager; import org.apache.hadoop.ozone.om.helpers.OmBucketInfo; import org.apache.hadoop.ozone.om.helpers.OmDirectoryInfo; -import org.apache.hadoop.ozone.om.helpers.OmKeyInfo; import org.apache.hadoop.util.StringUtils; import org.junit.After; import org.junit.Assert; @@ -49,7 +46,6 @@ import java.util.concurrent.TimeoutException; import static org.apache.hadoop.fs.CommonConfigurationKeysPublic.FS_TRASH_INTERVAL_KEY; import static org.apache.hadoop.ozone.OzoneConfigKeys.OZONE_FS_ITERATE_BATCH_SIZE; -import static org.junit.Assert.fail; /** * Test verifies the entries and operations in directory table. @@ -95,28 +91,6 @@ public class TestOzoneDirectory { Assert.assertEquals("Wrong OM numKeys metrics", 4, cluster.getOzoneManager().getMetrics().getNumKeys()); - // verify entries in directory table - TableIterator<String, ? extends - Table.KeyValue<String, OmDirectoryInfo>> iterator = - omMgr.getDirectoryTable().iterator(); - iterator.seekToFirst(); - int count = dirKeys.size(); - Assert.assertEquals("Unexpected directory table entries!", 4, count); - while (iterator.hasNext()) { - count--; - Table.KeyValue<String, OmDirectoryInfo> value = iterator.next(); - verifyKeyFormat(value.getKey(), dirKeys); - } - Assert.assertEquals("Unexpected directory table entries!", 0, count); - - // verify entries in key table - TableIterator<String, ? extends - Table.KeyValue<String, OmKeyInfo>> keyTableItr = - omMgr.getKeyTable().iterator(); - while (keyTableItr.hasNext()) { - fail("Shouldn't add any entries in KeyTable!"); - } - // create sub-dirs under same parent Path subDir5 = new Path("/d1/d2/d3/d4/d5"); fs.mkdirs(subDir5); diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestOzoneFileSystem.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestOzoneFileSystem.java index 44ea7b9..bf9e09f 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestOzoneFileSystem.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestOzoneFileSystem.java @@ -598,10 +598,17 @@ public class TestOzoneFileSystem { ArrayList<String> actualPathList = new ArrayList<>(); if (rootItemCount != fileStatuses.length) { for (int i = 0; i < fileStatuses.length; i++) { - actualPaths.add(fileStatuses[i].getPath().getName()); + boolean duplicate = + actualPaths.add(fileStatuses[i].getPath().getName()); + if (!duplicate) { + LOG.info("Duplicate path:{} in FileStatusList", + fileStatuses[i].getPath().getName()); + } actualPathList.add(fileStatuses[i].getPath().getName()); } if (rootItemCount != actualPathList.size()) { + LOG.info("actualPathsSize: {}", actualPaths.size()); + LOG.info("actualPathListSize: {}", actualPathList.size()); actualPaths.removeAll(paths); actualPathList.removeAll(paths); LOG.info("actualPaths: {}", actualPaths); diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestObjectStoreV1.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestObjectStoreV1.java index ee127cf..b88bbc3 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestObjectStoreV1.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestObjectStoreV1.java @@ -17,17 +17,23 @@ package org.apache.hadoop.ozone.om; import org.apache.commons.lang3.RandomStringUtils; +import org.apache.hadoop.fs.CommonConfigurationKeysPublic; +import org.apache.hadoop.fs.FileStatus; +import org.apache.hadoop.fs.FileSystem; +import org.apache.hadoop.fs.Path; import org.apache.hadoop.hdds.client.ReplicationFactor; import org.apache.hadoop.hdds.client.ReplicationType; import org.apache.hadoop.hdds.conf.OzoneConfiguration; import org.apache.hadoop.hdds.utils.db.Table; -import org.apache.hadoop.hdds.utils.db.TableIterator; import org.apache.hadoop.ozone.MiniOzoneCluster; +import org.apache.hadoop.ozone.OzoneConsts; +import org.apache.hadoop.ozone.TestDataUtil; import org.apache.hadoop.ozone.client.ObjectStore; import org.apache.hadoop.ozone.client.OzoneBucket; import org.apache.hadoop.ozone.client.OzoneClient; import org.apache.hadoop.ozone.client.OzoneKeyDetails; import org.apache.hadoop.ozone.client.OzoneVolume; +import org.apache.hadoop.ozone.client.io.KeyOutputStream; import org.apache.hadoop.ozone.client.io.OzoneOutputStream; import org.apache.hadoop.ozone.om.exceptions.OMException; import org.apache.hadoop.ozone.om.helpers.OmDirectoryInfo; @@ -36,6 +42,7 @@ import org.apache.hadoop.ozone.om.request.TestOMRequestUtils; import org.apache.hadoop.util.StringUtils; import org.junit.Assert; import org.junit.AfterClass; +import org.junit.After; import org.junit.BeforeClass; import org.junit.Rule; import org.junit.Test; @@ -45,6 +52,8 @@ import java.io.IOException; import java.util.HashMap; import java.util.UUID; +import static org.apache.hadoop.ozone.OzoneConfigKeys.OZONE_FS_ITERATE_BATCH_SIZE; +import static org.apache.hadoop.ozone.OzoneConsts.OM_KEY_PREFIX; import static org.junit.Assert.assertEquals; import static org.junit.Assert.fail; @@ -55,6 +64,9 @@ public class TestObjectStoreV1 { private static String clusterId; private static String scmId; private static String omId; + private static String volumeName; + private static String bucketName; + private static FileSystem fs; @Rule public Timeout timeout = new Timeout(240000); @@ -78,12 +90,51 @@ public class TestObjectStoreV1 { .setOmId(omId) .build(); cluster.waitForClusterToBeReady(); + // create a volume and a bucket to be used by OzoneFileSystem + OzoneBucket bucket = TestDataUtil.createVolumeAndBucket(cluster); + volumeName = bucket.getVolumeName(); + bucketName = bucket.getName(); + + String rootPath = String.format("%s://%s.%s/", + OzoneConsts.OZONE_URI_SCHEME, bucket.getName(), + bucket.getVolumeName()); + // Set the fs.defaultFS and start the filesystem + conf.set(CommonConfigurationKeysPublic.FS_DEFAULT_NAME_KEY, rootPath); + // Set the number of keys to be processed during batch operate. + conf.setInt(OZONE_FS_ITERATE_BATCH_SIZE, 5); + fs = FileSystem.get(conf); + } + + @After + public void tearDown() throws Exception { + deleteRootDir(); + } + + /** + * Cleanup files and directories. + * + * @throws IOException DB failure + */ + private void deleteRootDir() throws IOException { + Path root = new Path("/"); + FileStatus[] fileStatuses = fs.listStatus(root); + + if (fileStatuses == null) { + return; + } + + for (FileStatus fStatus : fileStatuses) { + fs.delete(fStatus.getPath(), true); + } + + fileStatuses = fs.listStatus(root); + if (fileStatuses != null) { + Assert.assertEquals("Delete root failed!", 0, fileStatuses.length); + } } @Test public void testCreateKey() throws Exception { - String volumeName = "volume" + RandomStringUtils.randomNumeric(5); - String bucketName = "bucket" + RandomStringUtils.randomNumeric(5); String parent = "a/b/c/"; String file = "key" + RandomStringUtils.randomNumeric(5); String key = parent + file; @@ -91,74 +142,67 @@ public class TestObjectStoreV1 { OzoneClient client = cluster.getClient(); ObjectStore objectStore = client.getObjectStore(); - objectStore.createVolume(volumeName); - OzoneVolume ozoneVolume = objectStore.getVolume(volumeName); Assert.assertTrue(ozoneVolume.getName().equals(volumeName)); - ozoneVolume.createBucket(bucketName); - OzoneBucket ozoneBucket = ozoneVolume.getBucket(bucketName); Assert.assertTrue(ozoneBucket.getName().equals(bucketName)); - Table<String, OmKeyInfo> openKeyTable = + Table<String, OmKeyInfo> openFileTable = cluster.getOzoneManager().getMetadataManager().getOpenKeyTable(); // before file creation - verifyKeyInFileTable(openKeyTable, file, 0, true); + verifyKeyInFileTable(openFileTable, file, 0, true); String data = "random data"; OzoneOutputStream ozoneOutputStream = ozoneBucket.createKey(key, data.length(), ReplicationType.RATIS, ReplicationFactor.ONE, new HashMap<>()); - OmDirectoryInfo dirPathC = getDirInfo(volumeName, bucketName, parent); + KeyOutputStream keyOutputStream = + (KeyOutputStream) ozoneOutputStream.getOutputStream(); + long clientID = keyOutputStream.getClientID(); + + OmDirectoryInfo dirPathC = getDirInfo(parent); Assert.assertNotNull("Failed to find dir path: a/b/c", dirPathC); // after file creation - verifyKeyInOpenFileTable(openKeyTable, file, dirPathC.getObjectID(), - false); + verifyKeyInOpenFileTable(openFileTable, clientID, file, + dirPathC.getObjectID(), false); ozoneOutputStream.write(data.getBytes(), 0, data.length()); ozoneOutputStream.close(); - Table<String, OmKeyInfo> keyTable = + Table<String, OmKeyInfo> fileTable = cluster.getOzoneManager().getMetadataManager().getKeyTable(); - // After closing the file. File entry should be removed from openFileTable // and it should be added to fileTable. - verifyKeyInFileTable(keyTable, file, dirPathC.getObjectID(), false); - verifyKeyInOpenFileTable(openKeyTable, file, dirPathC.getObjectID(), - true); + verifyKeyInFileTable(fileTable, file, dirPathC.getObjectID(), false); + verifyKeyInOpenFileTable(openFileTable, clientID, file, + dirPathC.getObjectID(), true); ozoneBucket.deleteKey(key); // after key delete - verifyKeyInFileTable(keyTable, file, dirPathC.getObjectID(), true); - verifyKeyInOpenFileTable(openKeyTable, file, dirPathC.getObjectID(), - true); + verifyKeyInFileTable(fileTable, file, dirPathC.getObjectID(), true); + verifyKeyInOpenFileTable(openFileTable, clientID, file, + dirPathC.getObjectID(), true); } @Test public void testLookupKey() throws Exception { - String volumeName = "volume" + RandomStringUtils.randomNumeric(5); - String bucketName = "bucket" + RandomStringUtils.randomNumeric(5); String parent = "a/b/c/"; - String file = "key" + RandomStringUtils.randomNumeric(5); - String key = parent + file; + String fileName = "key" + RandomStringUtils.randomNumeric(5); + String key = parent + fileName; OzoneClient client = cluster.getClient(); ObjectStore objectStore = client.getObjectStore(); - objectStore.createVolume(volumeName); - OzoneVolume ozoneVolume = objectStore.getVolume(volumeName); Assert.assertTrue(ozoneVolume.getName().equals(volumeName)); - ozoneVolume.createBucket(bucketName); - OzoneBucket ozoneBucket = ozoneVolume.getBucket(bucketName); Assert.assertTrue(ozoneBucket.getName().equals(bucketName)); - Table<String, OmKeyInfo> openKeyTable = + Table<String, OmKeyInfo> openFileTable = cluster.getOzoneManager().getMetadataManager().getOpenKeyTable(); String data = "random data"; @@ -166,19 +210,23 @@ public class TestObjectStoreV1 { data.length(), ReplicationType.RATIS, ReplicationFactor.ONE, new HashMap<>()); - OmDirectoryInfo dirPathC = getDirInfo(volumeName, bucketName, parent); + KeyOutputStream keyOutputStream = + (KeyOutputStream) ozoneOutputStream.getOutputStream(); + long clientID = keyOutputStream.getClientID(); + + OmDirectoryInfo dirPathC = getDirInfo(parent); Assert.assertNotNull("Failed to find dir path: a/b/c", dirPathC); // after file creation - verifyKeyInOpenFileTable(openKeyTable, file, dirPathC.getObjectID(), - false); + verifyKeyInOpenFileTable(openFileTable, clientID, fileName, + dirPathC.getObjectID(), false); ozoneOutputStream.write(data.getBytes(), 0, data.length()); // open key try { ozoneBucket.getKey(key); - fail("Should throw exception as file is not visible and its still " + + fail("Should throw exception as fileName is not visible and its still " + "open for writing!"); } catch (OMException ome) { // expected @@ -190,34 +238,33 @@ public class TestObjectStoreV1 { OzoneKeyDetails keyDetails = ozoneBucket.getKey(key); Assert.assertEquals(key, keyDetails.getName()); - Table<String, OmKeyInfo> keyTable = + Table<String, OmKeyInfo> fileTable = cluster.getOzoneManager().getMetadataManager().getKeyTable(); // When closing the key, entry should be removed from openFileTable // and it should be added to fileTable. - verifyKeyInFileTable(keyTable, file, dirPathC.getObjectID(), false); - verifyKeyInOpenFileTable(openKeyTable, file, dirPathC.getObjectID(), - true); + verifyKeyInFileTable(fileTable, fileName, dirPathC.getObjectID(), false); + verifyKeyInOpenFileTable(openFileTable, clientID, fileName, + dirPathC.getObjectID(), true); ozoneBucket.deleteKey(key); // get deleted key try { ozoneBucket.getKey(key); - fail("Should throw exception as file not exists!"); + fail("Should throw exception as fileName not exists!"); } catch (OMException ome) { // expected assertEquals(ome.getResult(), OMException.ResultCodes.KEY_NOT_FOUND); } // after key delete - verifyKeyInFileTable(keyTable, file, dirPathC.getObjectID(), true); - verifyKeyInOpenFileTable(openKeyTable, file, dirPathC.getObjectID(), - true); + verifyKeyInFileTable(fileTable, fileName, dirPathC.getObjectID(), true); + verifyKeyInOpenFileTable(openFileTable, clientID, fileName, + dirPathC.getObjectID(), true); } - private OmDirectoryInfo getDirInfo(String volumeName, String bucketName, - String parentKey) throws Exception { + private OmDirectoryInfo getDirInfo(String parentKey) throws Exception { OMMetadataManager omMetadataManager = cluster.getOzoneManager().getMetadataManager(); long bucketId = TestOMRequestUtils.getBucketId(volumeName, bucketName, @@ -238,51 +285,38 @@ public class TestObjectStoreV1 { private void verifyKeyInFileTable(Table<String, OmKeyInfo> fileTable, String fileName, long parentID, boolean isEmpty) throws IOException { - TableIterator<String, ? extends Table.KeyValue<String, OmKeyInfo>> iterator - = fileTable.iterator(); + String dbFileKey = parentID + OM_KEY_PREFIX + fileName; + OmKeyInfo omKeyInfo = fileTable.get(dbFileKey); if (isEmpty) { - Assert.assertTrue("Table is not empty!", fileTable.isEmpty()); + Assert.assertNull("Table is not empty!", omKeyInfo); } else { - Assert.assertFalse("Table is empty!", fileTable.isEmpty()); - while (iterator.hasNext()) { - Table.KeyValue<String, OmKeyInfo> next = iterator.next(); - Assert.assertEquals("Invalid Key: " + next.getKey(), - parentID + "/" + fileName, next.getKey()); - OmKeyInfo omKeyInfo = next.getValue(); - Assert.assertEquals("Invalid Key", fileName, - omKeyInfo.getFileName()); - Assert.assertEquals("Invalid Key", fileName, - omKeyInfo.getKeyName()); - Assert.assertEquals("Invalid Key", parentID, - omKeyInfo.getParentObjectID()); - } + Assert.assertNotNull("Table is empty!", omKeyInfo); + // used startsWith because the key format is, + // <parentID>/fileName/<clientID> and clientID is not visible. + Assert.assertEquals("Invalid Key: " + omKeyInfo.getObjectInfo(), + omKeyInfo.getKeyName(), fileName); + Assert.assertEquals("Invalid Key", parentID, + omKeyInfo.getParentObjectID()); } } private void verifyKeyInOpenFileTable(Table<String, OmKeyInfo> openFileTable, - String fileName, long parentID, boolean isEmpty) throws IOException { - TableIterator<String, ? extends Table.KeyValue<String, OmKeyInfo>> iterator - = openFileTable.iterator(); - + long clientID, String fileName, long parentID, boolean isEmpty) + throws IOException { + String dbOpenFileKey = + parentID + OM_KEY_PREFIX + fileName + OM_KEY_PREFIX + clientID; + OmKeyInfo omKeyInfo = openFileTable.get(dbOpenFileKey); if (isEmpty) { - Assert.assertTrue("Table is not empty!", openFileTable.isEmpty()); + Assert.assertNull("Table is not empty!", omKeyInfo); } else { - Assert.assertFalse("Table is empty!", openFileTable.isEmpty()); - while (iterator.hasNext()) { - Table.KeyValue<String, OmKeyInfo> next = iterator.next(); - // used startsWith because the key format is, - // <parentID>/fileName/<clientID> and clientID is not visible. - Assert.assertTrue("Invalid Key: " + next.getKey(), - next.getKey().startsWith(parentID + "/" + fileName)); - OmKeyInfo omKeyInfo = next.getValue(); - Assert.assertEquals("Invalid Key", fileName, - omKeyInfo.getFileName()); - Assert.assertEquals("Invalid Key", fileName, - omKeyInfo.getKeyName()); - Assert.assertEquals("Invalid Key", parentID, - omKeyInfo.getParentObjectID()); - } + Assert.assertNotNull("Table is empty!", omKeyInfo); + // used startsWith because the key format is, + // <parentID>/fileName/<clientID> and clientID is not visible. + Assert.assertEquals("Invalid Key: " + omKeyInfo.getObjectInfo(), + omKeyInfo.getKeyName(), fileName); + Assert.assertEquals("Invalid Key", parentID, + omKeyInfo.getParentObjectID()); } } 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 db28ff7..055ab13 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 @@ -31,7 +31,6 @@ import java.util.HashMap; import java.util.HashSet; import java.util.Iterator; import java.util.List; -import java.util.LinkedHashSet; import java.util.Map; import java.util.Objects; import java.util.Set; @@ -2320,6 +2319,7 @@ public class KeyManagerImpl implements KeyManager { return fileStatusList; } + @SuppressWarnings("methodlength") public List<OzoneFileStatus> listStatusV1(OmKeyArgs args, boolean recursive, String startKey, long numEntries, String clientAddress) throws IOException { @@ -2327,10 +2327,32 @@ public class KeyManagerImpl implements KeyManager { // unsorted OMKeyInfo list contains combine results from TableCache and DB. List<OzoneFileStatus> fileStatusFinalList = new ArrayList<>(); - LinkedHashSet<OzoneFileStatus> fileStatusList = new LinkedHashSet<>(); + if (numEntries <= 0) { return fileStatusFinalList; } + + /** + * A map sorted by OmKey to combine results from TableCache and DB for + * each entity - Dir & File. + * + * Two separate maps are required because the order of seek -> (1)Seek + * files in fileTable (2)Seek dirs in dirTable. + * + * StartKey should be added to the final listStatuses, so if we combine + * files and dirs into a single map then directory with lower precedence + * will appear at the top of the list even if the startKey is given as + * fileName. + * + * For example, startKey="a/file1". As per the seek order, first fetches + * all the files and then it will start seeking all the directories. + * Assume a directory name exists "a/b". With one map, the sorted list will + * be ["a/b", "a/file1"]. But the expected list is: ["a/file1", "a/b"], + * startKey element should always be at the top of the listStatuses. + */ + TreeMap<String, OzoneFileStatus> cacheFileMap = new TreeMap<>(); + TreeMap<String, OzoneFileStatus> cacheDirMap = new TreeMap<>(); + String volumeName = args.getVolumeName(); String bucketName = args.getBucketName(); String keyName = args.getKeyName(); @@ -2373,12 +2395,12 @@ public class KeyManagerImpl implements KeyManager { seekFileInDB = metadataManager.getOzonePathKey(prefixKeyInDB, ""); seekDirInDB = metadataManager.getOzonePathKey(prefixKeyInDB, ""); - // Order of seek -> (1)Seek dirs in dirTable (2)Seek files in fileTable + // Order of seek -> (1)Seek files in fileTable (2)Seek dirs in dirTable // 1. Seek the given key in key table. - countEntries = getFilesFromDirectory(fileStatusList, seekFileInDB, + countEntries = getFilesFromDirectory(cacheFileMap, seekFileInDB, prefixPath, prefixKeyInDB, startKey, countEntries, numEntries); // 2. Seek the given key in dir table. - getDirectories(fileStatusList, seekDirInDB, prefixPath, prefixKeyInDB, + getDirectories(cacheDirMap, seekDirInDB, prefixPath, prefixKeyInDB, startKey, countEntries, numEntries, volumeName, bucketName, recursive); } else { @@ -2420,7 +2442,7 @@ public class KeyManagerImpl implements KeyManager { // dirTable. So, its not required to search again in the fileTable. // Seek the given key in dirTable. - getDirectories(fileStatusList, seekDirInDB, prefixPath, + getDirectories(cacheDirMap, seekDirInDB, prefixPath, prefixKeyInDB, startKey, countEntries, numEntries, volumeName, bucketName, recursive); } else { @@ -2430,11 +2452,11 @@ public class KeyManagerImpl implements KeyManager { seekDirInDB = metadataManager.getOzonePathKey(prefixKeyInDB, ""); // 1. Seek the given key in key table. - countEntries = getFilesFromDirectory(fileStatusList, seekFileInDB, + countEntries = getFilesFromDirectory(cacheFileMap, seekFileInDB, prefixPath, prefixKeyInDB, startKey, countEntries, numEntries); // 2. Seek the given key in dir table. - getDirectories(fileStatusList, seekDirInDB, prefixPath, + getDirectories(cacheDirMap, seekDirInDB, prefixPath, prefixKeyInDB, startKey, countEntries, numEntries, volumeName, bucketName, recursive); } @@ -2451,12 +2473,16 @@ public class KeyManagerImpl implements KeyManager { metadataManager.getLock().releaseReadLock(BUCKET_LOCK, volumeName, bucketName); } - List<OmKeyInfo> keyInfoList = new ArrayList<>(fileStatusList.size()); - for (OzoneFileStatus fileStatus : fileStatusList) { - if (fileStatus.isFile()) { - keyInfoList.add(fileStatus.getKeyInfo()); - } + + List<OmKeyInfo> keyInfoList = new ArrayList<>(); + for (OzoneFileStatus fileStatus : cacheFileMap.values()) { + fileStatusFinalList.add(fileStatus); + keyInfoList.add(fileStatus.getKeyInfo()); + } + for (OzoneFileStatus fileStatus : cacheDirMap.values()) { + fileStatusFinalList.add(fileStatus); } + // refreshPipeline flag check has been removed as part of // https://issues.apache.org/jira/browse/HDDS-3658. // Please refer this jira for more details. @@ -2464,20 +2490,23 @@ public class KeyManagerImpl implements KeyManager { if (args.getSortDatanodes()) { sortDatanodes(clientAddress, keyInfoList.toArray(new OmKeyInfo[0])); } - fileStatusFinalList.addAll(fileStatusList); return fileStatusFinalList; } @SuppressWarnings("parameternumber") - protected int getDirectories(Set<OzoneFileStatus> fileStatusList, + protected int getDirectories( + TreeMap<String, OzoneFileStatus> cacheKeyMap, String seekDirInDB, String prefixPath, long prefixKeyInDB, String startKey, int countEntries, long numEntries, String volumeName, String bucketName, boolean recursive) throws IOException { + // A set to keep track of keys deleted in cache but not flushed to DB. + Set<String> deletedKeySet = new TreeSet<>(); + Table dirTable = metadataManager.getDirectoryTable(); - countEntries = listStatusFindDirsInTableCache(fileStatusList, dirTable, + countEntries = listStatusFindDirsInTableCache(cacheKeyMap, dirTable, prefixKeyInDB, seekDirInDB, prefixPath, startKey, volumeName, - bucketName, countEntries, numEntries); + bucketName, countEntries, numEntries, deletedKeySet); TableIterator<String, ? extends Table.KeyValue<String, OmDirectoryInfo>> iterator = dirTable.iterator(); @@ -2485,6 +2514,11 @@ public class KeyManagerImpl implements KeyManager { while (iterator.hasNext() && numEntries - countEntries > 0) { OmDirectoryInfo dirInfo = iterator.value().getValue(); + if (deletedKeySet.contains(dirInfo.getPath())) { + iterator.next(); // move to next entry in the table + // entry is actually deleted in cache and can exists in DB + continue; + } if (!OMFileRequest.isImmediateChild(dirInfo.getParentObjectID(), prefixKeyInDB)) { break; @@ -2496,7 +2530,7 @@ public class KeyManagerImpl implements KeyManager { dirInfo.getName()); OmKeyInfo omKeyInfo = OMFileRequest.getOmKeyInfo(volumeName, bucketName, dirInfo, dirName); - fileStatusList.add(new OzoneFileStatus(omKeyInfo, scmBlockSize, + cacheKeyMap.put(dirName, new OzoneFileStatus(omKeyInfo, scmBlockSize, true)); countEntries++; } @@ -2507,20 +2541,28 @@ public class KeyManagerImpl implements KeyManager { return countEntries; } - private int getFilesFromDirectory(Set<OzoneFileStatus> fileStatusList, + private int getFilesFromDirectory( + TreeMap<String, OzoneFileStatus> cacheKeyMap, String seekKeyInDB, String prefixKeyPath, long prefixKeyInDB, String startKey, int countEntries, long numEntries) throws IOException { + // A set to keep track of keys deleted in cache but not flushed to DB. + Set<String> deletedKeySet = new TreeSet<>(); + Table<String, OmKeyInfo> keyTable = metadataManager.getKeyTable(); - countEntries = listStatusFindFilesInTableCache(fileStatusList, keyTable, + countEntries = listStatusFindFilesInTableCache(cacheKeyMap, keyTable, prefixKeyInDB, seekKeyInDB, prefixKeyPath, startKey, - countEntries, numEntries); + countEntries, numEntries, deletedKeySet); TableIterator<String, ? extends Table.KeyValue<String, OmKeyInfo>> iterator = keyTable.iterator(); iterator.seek(seekKeyInDB); while (iterator.hasNext() && numEntries - countEntries > 0) { OmKeyInfo keyInfo = iterator.value().getValue(); - + if (deletedKeySet.contains(keyInfo.getPath())) { + iterator.next(); // move to next entry in the table + // entry is actually deleted in cache and can exists in DB + continue; + } if (!OMFileRequest.isImmediateChild(keyInfo.getParentObjectID(), prefixKeyInDB)) { break; @@ -2530,7 +2572,8 @@ public class KeyManagerImpl implements KeyManager { String fullKeyPath = OMFileRequest.getAbsolutePath(prefixKeyPath, keyInfo.getKeyName()); keyInfo.setKeyName(fullKeyPath); - fileStatusList.add(new OzoneFileStatus(keyInfo, scmBlockSize, false)); + cacheKeyMap.put(fullKeyPath, + new OzoneFileStatus(keyInfo, scmBlockSize, false)); countEntries++; iterator.next(); // move to next entry in the table } @@ -2542,10 +2585,10 @@ public class KeyManagerImpl implements KeyManager { */ @SuppressWarnings("parameternumber") private int listStatusFindFilesInTableCache( - Set<OzoneFileStatus> fileStatusList, Table<String, + TreeMap<String, OzoneFileStatus> cacheKeyMap, Table<String, OmKeyInfo> keyTable, long prefixKeyInDB, String seekKeyInDB, String prefixKeyPath, String startKey, int countEntries, - long numEntries) { + long numEntries, Set<String> deletedKeySet) { Iterator<Map.Entry<CacheKey<String>, CacheValue<OmKeyInfo>>> cacheIter = keyTable.cacheIterator(); @@ -2558,6 +2601,7 @@ public class KeyManagerImpl implements KeyManager { OmKeyInfo cacheOmKeyInfo = entry.getValue().getCacheValue(); // cacheOmKeyInfo is null if an entry is deleted in cache if(cacheOmKeyInfo == null){ + deletedKeySet.add(cacheKey); continue; } @@ -2571,7 +2615,7 @@ public class KeyManagerImpl implements KeyManager { omKeyInfo.getKeyName()); omKeyInfo.setKeyName(fullKeyPath); - countEntries = addKeyInfoToFileStatusList(fileStatusList, prefixKeyInDB, + countEntries = addKeyInfoToFileStatusList(cacheKeyMap, prefixKeyInDB, seekKeyInDB, startKey, countEntries, cacheKey, omKeyInfo, false); } @@ -2583,10 +2627,11 @@ public class KeyManagerImpl implements KeyManager { */ @SuppressWarnings("parameternumber") private int listStatusFindDirsInTableCache( - Set<OzoneFileStatus> fileStatusList, Table<String, + TreeMap<String, OzoneFileStatus> cacheKeyMap, Table<String, OmDirectoryInfo> dirTable, long prefixKeyInDB, String seekKeyInDB, String prefixKeyPath, String startKey, String volumeName, - String bucketName, int countEntries, long numEntries) { + String bucketName, int countEntries, long numEntries, + Set<String> deletedKeySet) { Iterator<Map.Entry<CacheKey<String>, CacheValue<OmDirectoryInfo>>> cacheIter = dirTable.cacheIterator(); @@ -2599,7 +2644,9 @@ public class KeyManagerImpl implements KeyManager { cacheIter.next(); String cacheKey = entry.getKey().getCacheKey(); OmDirectoryInfo cacheOmDirInfo = entry.getValue().getCacheValue(); + // cacheOmKeyInfo is null if an entry is deleted in cache if(cacheOmDirInfo == null){ + deletedKeySet.add(cacheKey); continue; } String fullDirPath = OMFileRequest.getAbsolutePath(prefixKeyPath, @@ -2607,7 +2654,7 @@ public class KeyManagerImpl implements KeyManager { OmKeyInfo cacheDirKeyInfo = OMFileRequest.getOmKeyInfo(volumeName, bucketName, cacheOmDirInfo, fullDirPath); - countEntries = addKeyInfoToFileStatusList(fileStatusList, prefixKeyInDB, + countEntries = addKeyInfoToFileStatusList(cacheKeyMap, prefixKeyInDB, seekKeyInDB, startKey, countEntries, cacheKey, cacheDirKeyInfo, true); } @@ -2615,7 +2662,8 @@ public class KeyManagerImpl implements KeyManager { } @SuppressWarnings("parameternumber") - private int addKeyInfoToFileStatusList(Set<OzoneFileStatus> fileStatusList, + private int addKeyInfoToFileStatusList( + TreeMap<String, OzoneFileStatus> cacheKeyMap, long prefixKeyInDB, String seekKeyInDB, String startKey, int countEntries, String cacheKey, OmKeyInfo cacheOmKeyInfo, boolean isDirectory) { @@ -2627,7 +2675,7 @@ public class KeyManagerImpl implements KeyManager { if (cacheKey.startsWith(seekKeyInDB)) { OzoneFileStatus fileStatus = new OzoneFileStatus(cacheOmKeyInfo, scmBlockSize, isDirectory); - fileStatusList.add(fileStatus); + cacheKeyMap.put(cacheOmKeyInfo.getKeyName(), fileStatus); countEntries++; } } else { @@ -2641,7 +2689,7 @@ public class KeyManagerImpl implements KeyManager { cacheKey.compareTo(seekKeyInDB) >= 0) { OzoneFileStatus fileStatus = new OzoneFileStatus(cacheOmKeyInfo, scmBlockSize, isDirectory); - fileStatusList.add(fileStatus); + cacheKeyMap.put(cacheOmKeyInfo.getKeyName(), fileStatus); countEntries++; } } diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyDeleteRequestV1.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyDeleteRequestV1.java index af3bc82..af5c4df 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyDeleteRequestV1.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyDeleteRequestV1.java @@ -31,6 +31,7 @@ import org.apache.hadoop.ozone.om.helpers.OmBucketInfo; import org.apache.hadoop.ozone.om.helpers.OmKeyInfo; import org.apache.hadoop.ozone.om.helpers.OmVolumeArgs; import org.apache.hadoop.ozone.om.helpers.OzoneFileStatus; +import org.apache.hadoop.ozone.om.helpers.OzoneFSUtils; import org.apache.hadoop.ozone.om.ratis.utils.OzoneManagerDoubleBufferHelper; import org.apache.hadoop.ozone.om.request.file.OMFileRequest; import org.apache.hadoop.ozone.om.request.util.OmResponseUtil; @@ -119,6 +120,11 @@ public class OMKeyDeleteRequestV1 extends OMKeyDeleteRequest { } OmKeyInfo omKeyInfo = keyStatus.getKeyInfo(); + // New key format for the fileTable & dirTable. + // For example, the user given key path is '/a/b/c/d/e/file1', then in DB + // keyName field stores only the leaf node name, which is 'file1'. + String fileName = OzoneFSUtils.getFileName(keyName); + omKeyInfo.setKeyName(fileName); // Set the UpdateID to current transactionLogIndex omKeyInfo.setUpdateID(trxnLogIndex, ozoneManager.isRatisEnabled()); --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
