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 9015011268ede0c087730ec5355a3c297c995380 Author: Rakesh Radhakrishnan <[email protected]> AuthorDate: Wed Jan 13 08:36:37 2021 +0530 HDDS-4658. LookupKey: do lookup in dir and file tables (#1775) --- .../hadoop/fs/ozone/TestOzoneFileSystemV1.java | 15 ++++ .../hadoop/ozone/client/rpc/TestReadRetries.java | 55 +++++++++++--- .../apache/hadoop/ozone/om/TestObjectStoreV1.java | 83 ++++++++++++++++++++++ .../org/apache/hadoop/ozone/om/KeyManagerImpl.java | 32 +++++++-- .../ozone/om/request/key/OMKeyCreateRequestV1.java | 3 +- 5 files changed, 175 insertions(+), 13 deletions(-) diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestOzoneFileSystemV1.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestOzoneFileSystemV1.java index 2938714..e574e94 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestOzoneFileSystemV1.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestOzoneFileSystemV1.java @@ -325,6 +325,11 @@ public class TestOzoneFileSystemV1 extends TestOzoneFileSystem { */ @Test public void testRenameWithNonExistentSource() throws Exception { + // Skip as this will run only in new layout + if (!enabledFileSystemPaths) { + return; + } + final String root = "/root"; final String dir1 = root + "/dir1"; final String dir2 = root + "/dir2"; @@ -350,6 +355,11 @@ public class TestOzoneFileSystemV1 extends TestOzoneFileSystem { */ @Test public void testRenameDirToItsOwnSubDir() throws Exception { + // Skip as this will run only in new layout + if (!enabledFileSystemPaths) { + return; + } + final String root = "/root"; final String dir1 = root + "/dir1"; final Path dir1Path = new Path(fs.getUri().toString() + dir1); @@ -377,6 +387,11 @@ public class TestOzoneFileSystemV1 extends TestOzoneFileSystem { */ @Test public void testRenameDestinationParentDoesntExist() throws Exception { + // Skip as this will run only in new layout + if (!enabledFileSystemPaths) { + return; + } + final String root = "/root_dir"; final String dir1 = root + "/dir1"; final String dir2 = dir1 + "/dir2"; diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestReadRetries.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestReadRetries.java index af95190..ce2f10b 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestReadRetries.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestReadRetries.java @@ -18,6 +18,8 @@ package org.apache.hadoop.ozone.client.rpc; import java.io.IOException; +import java.util.Arrays; +import java.util.Collection; import java.util.HashMap; import java.util.List; import java.util.UUID; @@ -47,24 +49,32 @@ import org.apache.hadoop.ozone.client.OzoneVolume; import org.apache.hadoop.ozone.client.io.KeyOutputStream; import org.apache.hadoop.ozone.client.io.OzoneInputStream; import org.apache.hadoop.ozone.client.io.OzoneOutputStream; +import org.apache.hadoop.ozone.om.OMConfigKeys; import org.apache.hadoop.ozone.om.OzoneManager; +import org.apache.hadoop.ozone.om.exceptions.OMException; import org.apache.hadoop.ozone.om.helpers.OmKeyArgs; import org.apache.hadoop.ozone.om.helpers.OmKeyLocationInfo; -import org.junit.AfterClass; +import org.junit.After; import org.junit.Assert; import static java.nio.charset.StandardCharsets.UTF_8; import static org.junit.Assert.fail; -import org.junit.BeforeClass; +import org.junit.Before; import org.junit.Rule; import org.junit.Test; -import org.junit.rules.ExpectedException; import org.junit.rules.Timeout; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; + +import static org.junit.Assert.assertEquals; + +import org.junit.rules.ExpectedException; /** * Test read retries from multiple nodes in the pipeline. */ +@RunWith(Parameterized.class) public class TestReadRetries { /** @@ -84,16 +94,27 @@ public class TestReadRetries { storageContainerLocationClient; private static final String SCM_ID = UUID.randomUUID().toString(); + private String layoutVersion; + public TestReadRetries(String layoutVersion) { + this.layoutVersion = layoutVersion; + } + + @Parameterized.Parameters + public static Collection<Object[]> data() { + return Arrays.asList(new Object[]{"V0"}, new Object[]{"V1"}); + } /** * Create a MiniOzoneCluster for testing. * @throws Exception */ - @BeforeClass - public static void init() throws Exception { + @Before + public void init() throws Exception { OzoneConfiguration conf = new OzoneConfiguration(); conf.setInt(ScmConfigKeys.OZONE_SCM_PIPELINE_OWNER_CONTAINER_COUNT, 1); + conf.setBoolean(OMConfigKeys.OZONE_OM_ENABLE_FILESYSTEM_PATHS, true); + conf.set(OMConfigKeys.OZONE_OM_LAYOUT_VERSION, layoutVersion); cluster = MiniOzoneCluster.newBuilder(conf) .setNumDatanodes(3) .setScmId(SCM_ID) @@ -112,8 +133,8 @@ public class TestReadRetries { /** * Close OzoneClient and shutdown MiniOzoneCluster. */ - @AfterClass - public static void shutdown() throws IOException { + @After + public void shutdown() throws IOException { if(ozClient != null) { ozClient.close(); } @@ -140,7 +161,7 @@ public class TestReadRetries { volume.createBucket(bucketName); OzoneBucket bucket = volume.getBucket(bucketName); - String keyName = UUID.randomUUID().toString(); + String keyName = "a/b/c/" + UUID.randomUUID().toString(); OzoneOutputStream out = bucket .createKey(keyName, value.getBytes(UTF_8).length, ReplicationType.RATIS, @@ -188,6 +209,13 @@ public class TestReadRetries { cluster.shutdownHddsDatanode(datanodeDetails); // try to read, this should be successful readKey(bucket, keyName, value); + + // read intermediate directory + verifyIntermediateDir(bucket, "a/b/c/"); + verifyIntermediateDir(bucket, "a/b/c"); + verifyIntermediateDir(bucket, "/a/b/c/"); + verifyIntermediateDir(bucket, "/a/b/c"); + // shutdown the second datanode datanodeDetails = datanodes.get(1); cluster.shutdownHddsDatanode(datanodeDetails); @@ -210,6 +238,17 @@ public class TestReadRetries { factory.releaseClient(clientSpi, false); } + private void verifyIntermediateDir(OzoneBucket bucket, + String dir) throws IOException { + try { + bucket.getKey(dir); + fail("Should throw exception for directory listing"); + } catch (OMException ome) { + // expected + assertEquals(OMException.ResultCodes.KEY_NOT_FOUND, ome.getResult()); + } + } + private void readKey(OzoneBucket bucket, String keyName, String data) throws IOException { OzoneKey key = bucket.getKey(keyName); 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 c6ae4ca..ee127cf 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 @@ -26,8 +26,10 @@ import org.apache.hadoop.ozone.MiniOzoneCluster; 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.OzoneOutputStream; +import org.apache.hadoop.ozone.om.exceptions.OMException; import org.apache.hadoop.ozone.om.helpers.OmDirectoryInfo; import org.apache.hadoop.ozone.om.helpers.OmKeyInfo; import org.apache.hadoop.ozone.om.request.TestOMRequestUtils; @@ -43,6 +45,9 @@ import java.io.IOException; import java.util.HashMap; import java.util.UUID; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.fail; + public class TestObjectStoreV1 { private static MiniOzoneCluster cluster = null; @@ -133,6 +138,84 @@ public class TestObjectStoreV1 { 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; + + 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 = + cluster.getOzoneManager().getMetadataManager().getOpenKeyTable(); + + String data = "random data"; + OzoneOutputStream ozoneOutputStream = ozoneBucket.createKey(key, + data.length(), ReplicationType.RATIS, ReplicationFactor.ONE, + new HashMap<>()); + + OmDirectoryInfo dirPathC = getDirInfo(volumeName, bucketName, parent); + Assert.assertNotNull("Failed to find dir path: a/b/c", dirPathC); + + // after file creation + verifyKeyInOpenFileTable(openKeyTable, file, 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 " + + "open for writing!"); + } catch (OMException ome) { + // expected + assertEquals(ome.getResult(), OMException.ResultCodes.KEY_NOT_FOUND); + } + + ozoneOutputStream.close(); + + OzoneKeyDetails keyDetails = ozoneBucket.getKey(key); + Assert.assertEquals(key, keyDetails.getName()); + + Table<String, OmKeyInfo> keyTable = + 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); + + ozoneBucket.deleteKey(key); + + // get deleted key + try { + ozoneBucket.getKey(key); + fail("Should throw exception as file 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); + } + private OmDirectoryInfo getDirInfo(String volumeName, String bucketName, String parentKey) throws Exception { OMMetadataManager omMetadataManager = 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 dc70369..db28ff7 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 @@ -658,9 +658,11 @@ public class KeyManagerImpl implements KeyManager { bucketName); OmKeyInfo value = null; try { - String keyBytes = metadataManager.getOzoneKey( - volumeName, bucketName, keyName); - value = metadataManager.getKeyTable().get(keyBytes); + if (OzoneManagerRatisUtils.isOmLayoutVersionV1()) { + value = getOmKeyInfoV1(volumeName, bucketName, keyName); + } else { + value = getOmKeyInfo(volumeName, bucketName, keyName); + } } catch (IOException ex) { if (ex instanceof OMException) { throw ex; @@ -680,7 +682,7 @@ public class KeyManagerImpl implements KeyManager { LOG.debug("volume:{} bucket:{} Key:{} not found", volumeName, bucketName, keyName); } - throw new OMException("Key not found", KEY_NOT_FOUND); + throw new OMException("Key:" + keyName + " not found", KEY_NOT_FOUND); } // add block token for read. @@ -697,6 +699,28 @@ public class KeyManagerImpl implements KeyManager { return value; } + private OmKeyInfo getOmKeyInfo(String volumeName, String bucketName, + String keyName) throws IOException { + String keyBytes = metadataManager.getOzoneKey( + volumeName, bucketName, keyName); + return metadataManager.getKeyTable().get(keyBytes); + } + + /** + * Look up will return only closed fileInfo. This will return null if the + * keyName is a directory or if the keyName is still open for writing. + */ + private OmKeyInfo getOmKeyInfoV1(String volumeName, String bucketName, + String keyName) throws IOException { + OzoneFileStatus fileStatus = + OMFileRequest.getOMKeyInfoIfExists(metadataManager, + volumeName, bucketName, keyName, scmBlockSize); + if (fileStatus == null) { + return null; + } + return fileStatus.isFile() ? fileStatus.getKeyInfo() : null; + } + private void addBlockToken4Read(OmKeyInfo value) throws IOException { Preconditions.checkNotNull(value, "OMKeyInfo cannot be null"); if (grpcBlockTokenEnabled) { diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyCreateRequestV1.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyCreateRequestV1.java index 416e462..a49c01e 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyCreateRequestV1.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyCreateRequestV1.java @@ -196,8 +196,9 @@ public class OMKeyCreateRequestV1 extends OMKeyCreateRequest { // Prepare response. Sets user given full key name in the 'keyName' // attribute in response object. + int clientVersion = getOmRequest().getVersion(); omResponse.setCreateKeyResponse(CreateKeyResponse.newBuilder() - .setKeyInfo(omFileInfo.getProtobuf(keyName)) + .setKeyInfo(omFileInfo.getProtobuf(keyName, clientVersion)) .setID(clientID) .setOpenVersion(openVersion).build()) .setCmdType(Type.CreateKey); --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
