diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneConfigKeys.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneConfigKeys.java index fa87ab2076f7..4e7606ce9f95 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneConfigKeys.java +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneConfigKeys.java @@ -443,12 +443,6 @@ public final class OzoneConfigKeys { public static final boolean OZONE_CLIENT_KEY_LATEST_VERSION_LOCATION_DEFAULT = true; - public static final String OZONE_CLIENT_TEST_OFS_DEFAULT_BUCKET_LAYOUT = - "ozone.client.test.ofs.default.bucket.layout"; - - public static final String OZONE_CLIENT_TEST_OFS_BUCKET_LAYOUT_DEFAULT = - "FILE_SYSTEM_OPTIMIZED"; - public static final String OZONE_CLIENT_REQUIRED_OM_VERSION_MIN_KEY = "ozone.client.required.om.version.min"; diff --git a/hadoop-hdds/common/src/main/resources/ozone-default.xml b/hadoop-hdds/common/src/main/resources/ozone-default.xml index 075bf65acb19..4c08e47823ed 100644 --- a/hadoop-hdds/common/src/main/resources/ozone-default.xml +++ b/hadoop-hdds/common/src/main/resources/ozone-default.xml @@ -3043,7 +3043,7 @@ ozone.default.bucket.layout - OBJECT_STORE + OZONE, MANAGEMENT Default bucket layout used by Ozone Manager during bucket creation when a client does not specify the @@ -3053,21 +3053,6 @@ FILE_SYSTEM_OPTIMIZED: This layout allows the bucket to support atomic rename/delete operations and also allows interoperability between S3 and FS APIs. Keys written via S3 API with a "/" delimiter will create intermediate directories. - For testing purposes LEGACY layout is also allowed. LEGACY layout is not recommended and will be removed in the - future. - - - - - ozone.client.test.ofs.default.bucket.layout - FILE_SYSTEM_OPTIMIZED - OZONE, CLIENT - - This configuration is used for testing purposes. Default bucket layout value when buckets are created using OFS. - Supported values are LEGACY and FILE_SYSTEM_OPTIMIZED. - FILE_SYSTEM_OPTIMIZED: This layout allows the bucket to support atomic rename/delete operations and - also allows interoperability between S3 and FS APIs. Keys written via S3 API with a "/" delimiter - will create intermediate directories. diff --git a/hadoop-hdds/docs/content/feature/PrefixFSO.md b/hadoop-hdds/docs/content/feature/PrefixFSO.md index 7d87b26d732a..894f36a48f1d 100644 --- a/hadoop-hdds/docs/content/feature/PrefixFSO.md +++ b/hadoop-hdds/docs/content/feature/PrefixFSO.md @@ -68,12 +68,12 @@ The following configuration can be configured in `ozone-site.xml` to define the if the client has not specified the bucket layout argument. Supported values are `OBJECT_STORE` and `FILE_SYSTEM_OPTIMIZED`. -By default, the buckets will default to `OBJECT_STORE` behaviour. +By default, this config value is empty. Ozone will default to `LEGACY` bucket layout if it finds an empty config value. ```XML ozone.default.bucket.layout - OBJECT_STORE + ``` \ No newline at end of file diff --git a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/OMConfigKeys.java b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/OMConfigKeys.java index eacb3f7959f5..7ebd628cfb1a 100644 --- a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/OMConfigKeys.java +++ b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/OMConfigKeys.java @@ -280,9 +280,11 @@ private OMConfigKeys() { public static final String OZONE_DEFAULT_BUCKET_LAYOUT = "ozone.default.bucket.layout"; public static final String OZONE_DEFAULT_BUCKET_LAYOUT_DEFAULT = - BucketLayout.OBJECT_STORE.name(); + BucketLayout.LEGACY.name(); public static final String OZONE_BUCKET_LAYOUT_FILE_SYSTEM_OPTIMIZED = BucketLayout.FILE_SYSTEM_OPTIMIZED.name(); + public static final String OZONE_BUCKET_LAYOUT_OBJECT_STORE = + BucketLayout.OBJECT_STORE.name(); /** * Configuration properties for Directory Deleting Service. 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 5c5c0e154411..0e0552298ccb 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 @@ -199,9 +199,6 @@ public static void initClusterAndEnv() throws IOException, bucketLayout.name()); } else { bucketLayout = BucketLayout.LEGACY; - // We need the OFS buckets to be in LEGACY layout for this test. - conf.set(OzoneConfigKeys.OZONE_CLIENT_TEST_OFS_DEFAULT_BUCKET_LAYOUT, - BucketLayout.LEGACY.name()); conf.set(OMConfigKeys.OZONE_DEFAULT_BUCKET_LAYOUT, bucketLayout.name()); conf.setBoolean(OMConfigKeys.OZONE_OM_ENABLE_FILESYSTEM_PATHS, @@ -520,22 +517,29 @@ public void testListStatusInBucket() throws Exception { Path dir1 = new Path(root, "dir1"); Path dir12 = new Path(dir1, "dir12"); Path dir2 = new Path(root, "dir2"); - fs.mkdirs(dir12); - fs.mkdirs(dir2); - - // ListStatus on root should return dir1 (even though /dir1 key does not - // exist) and dir2 only. dir12 is not an immediate child of root and - // hence should not be listed. - FileStatus[] fileStatuses = ofs.listStatus(root); - Assert.assertEquals( - "FileStatus should return only the immediate children", - 2, fileStatuses.length); - - // Verify that dir12 is not included in the result of the listStatus on root - String fileStatus1 = fileStatuses[0].getPath().toUri().getPath(); - String fileStatus2 = fileStatuses[1].getPath().toUri().getPath(); - Assert.assertNotEquals(fileStatus1, dir12.toString()); - Assert.assertNotEquals(fileStatus2, dir12.toString()); + try { + fs.mkdirs(dir12); + fs.mkdirs(dir2); + + // ListStatus on root should return dir1 (even though /dir1 key does not + // exist) and dir2 only. dir12 is not an immediate child of root and + // hence should not be listed. + FileStatus[] fileStatuses = ofs.listStatus(root); + Assert.assertEquals( + "FileStatus should return only the immediate children", + 2, fileStatuses.length); + + // Verify that dir12 is not included in the result of the listStatus on + // root + String fileStatus1 = fileStatuses[0].getPath().toUri().getPath(); + String fileStatus2 = fileStatuses[1].getPath().toUri().getPath(); + Assert.assertNotEquals(fileStatus1, dir12.toString()); + Assert.assertNotEquals(fileStatus2, dir12.toString()); + } finally { + // cleanup + fs.delete(dir1, true); + fs.delete(dir2, true); + } } /** @@ -546,25 +550,27 @@ public void testListStatusOnLargeDirectory() throws Exception { Path root = new Path("/" + volumeName + "/" + bucketName); Set paths = new TreeSet<>(); int numDirs = LISTING_PAGE_SIZE + LISTING_PAGE_SIZE / 2; - for (int i = 0; i < numDirs; i++) { - Path p = new Path(root, String.valueOf(i)); - fs.mkdirs(p); - paths.add(p.getName()); - } - - FileStatus[] fileStatuses = ofs.listStatus(root); - Assert.assertEquals( - "Total directories listed do not match the existing directories", - numDirs, fileStatuses.length); + try { + for (int i = 0; i < numDirs; i++) { + Path p = new Path(root, String.valueOf(i)); + fs.mkdirs(p); + paths.add(p.getName()); + } - for (int i = 0; i < numDirs; i++) { - Assert.assertTrue(paths.contains(fileStatuses[i].getPath().getName())); - } + FileStatus[] fileStatuses = ofs.listStatus(root); + Assert.assertEquals( + "Total directories listed do not match the existing directories", + numDirs, fileStatuses.length); - // Cleanup - for (int i = 0; i < numDirs; i++) { - Path p = new Path(root, String.valueOf(i)); - fs.delete(p, true); + for (int i = 0; i < numDirs; i++) { + Assert.assertTrue(paths.contains(fileStatuses[i].getPath().getName())); + } + } finally { + // Cleanup + for (int i = 0; i < numDirs; i++) { + Path p = new Path(root, String.valueOf(i)); + fs.delete(p, true); + } } } @@ -1445,16 +1451,21 @@ private void checkInvalidPath(Path path) throws Exception { public void testRenameFile() throws Exception { final String dir = "/dir" + new Random().nextInt(1000); Path dirPath = new Path(getBucketPath() + dir); - getFs().mkdirs(dirPath); - Path file1Source = new Path(getBucketPath() + dir + "/file1_Copy"); - ContractTestUtils.touch(getFs(), file1Source); Path file1Destin = new Path(getBucketPath() + dir + "/file1"); - assertTrue("Renamed failed", getFs().rename(file1Source, file1Destin)); - assertTrue("Renamed failed: /dir/file1", getFs().exists(file1Destin)); - FileStatus[] fStatus = getFs().listStatus(dirPath); - assertEquals("Renamed failed", 1, fStatus.length); + try { + getFs().mkdirs(dirPath); + + ContractTestUtils.touch(getFs(), file1Source); + assertTrue("Renamed failed", getFs().rename(file1Source, file1Destin)); + assertTrue("Renamed failed: /dir/file1", getFs().exists(file1Destin)); + FileStatus[] fStatus = getFs().listStatus(dirPath); + assertEquals("Renamed failed", 1, fStatus.length); + } finally { + // clean up + fs.delete(dirPath, true); + } } @@ -1492,25 +1503,32 @@ public void testRenameToParentDir() throws Exception { final String dir1 = root + "/dir1"; final String dir2 = dir1 + "/dir2"; final Path dir2SourcePath = new Path(getBucketPath() + dir2); - getFs().mkdirs(dir2SourcePath); final Path destRootPath = new Path(getBucketPath() + root); - Path file1Source = new Path(getBucketPath() + dir1 + "/file2"); - ContractTestUtils.touch(getFs(), file1Source); - - // rename source directory to its parent directory(destination). - assertTrue("Rename failed", getFs().rename(dir2SourcePath, destRootPath)); - final Path expectedPathAfterRename = - new Path(getBucketPath() + root + "/dir2"); - assertTrue("Rename failed", - getFs().exists(expectedPathAfterRename)); - - // rename source file to its parent directory(destination). - assertTrue("Rename failed", getFs().rename(file1Source, destRootPath)); - final Path expectedFilePathAfterRename = - new Path(getBucketPath() + root + "/file2"); - assertTrue("Rename failed", - getFs().exists(expectedFilePathAfterRename)); + try { + getFs().mkdirs(dir2SourcePath); + + ContractTestUtils.touch(getFs(), file1Source); + + // rename source directory to its parent directory(destination). + assertTrue("Rename failed", getFs().rename(dir2SourcePath, destRootPath)); + final Path expectedPathAfterRename = + new Path(getBucketPath() + root + "/dir2"); + assertTrue("Rename failed", + getFs().exists(expectedPathAfterRename)); + + // rename source file to its parent directory(destination). + assertTrue("Rename failed", getFs().rename(file1Source, destRootPath)); + final Path expectedFilePathAfterRename = + new Path(getBucketPath() + root + "/file2"); + assertTrue("Rename failed", + getFs().exists(expectedFilePathAfterRename)); + } finally { + // clean up + fs.delete(file1Source, true); + fs.delete(dir2SourcePath, true); + fs.delete(destRootPath, true); + } } /** @@ -1535,6 +1553,9 @@ public void testRenameDirToItsOwnSubDir() throws Exception { " its own subdirectory"); } catch (IllegalArgumentException e) { //expected + } finally { + // clean up + fs.delete(sourceRoot, true); } } 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 7d444872f709..d824352d5f6d 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 @@ -166,13 +166,16 @@ public void testRenameDirToItsOwnSubDir() throws Exception { final Path dir1Path = new Path(getBucketPath() + dir1); // Add a sub-dir1 to the directory to be moved. final Path subDir1 = new Path(dir1Path, "sub_dir1"); - getFs().mkdirs(subDir1); - LOG.info("Created dir1 {}", subDir1); - final Path sourceRoot = new Path(getBucketPath() + root); - LOG.info("Rename op-> source:{} to destin:{}", sourceRoot, subDir1); - // rename should fail and return false - Assert.assertFalse(getFs().rename(sourceRoot, subDir1)); + try { + getFs().mkdirs(subDir1); + LOG.info("Created dir1 {}", subDir1); + LOG.info("Rename op-> source:{} to destin:{}", sourceRoot, subDir1); + // rename should fail and return false + Assert.assertFalse(getFs().rename(sourceRoot, subDir1)); + } finally { + getFs().delete(sourceRoot, true); + } } @Override diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestObjectStore.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestObjectStore.java index 1f433d3011a7..6e4b5e6c4b7b 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestObjectStore.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestObjectStore.java @@ -84,17 +84,17 @@ public void testCreateBucketWithBucketLayout() throws Exception { store.createVolume(sampleVolumeName); OzoneVolume volume = store.getVolume(sampleVolumeName); - // Case 1: Bucket layout: Empty and OM default bucket layout: OBJECT_STORE + // Case 1: Bucket layout: Empty and OM default bucket layout: LEGACY BucketArgs.Builder builder = BucketArgs.newBuilder(); volume.createBucket(sampleBucketName, builder.build()); OzoneBucket bucket = volume.getBucket(sampleBucketName); Assert.assertEquals(sampleBucketName, bucket.getName()); - Assert.assertEquals(BucketLayout.OBJECT_STORE, + Assert.assertEquals(BucketLayout.LEGACY, bucket.getBucketLayout()); - // Case 2: Bucket layout: DEFAULT + // Case 2: Bucket layout: OBJECT_STORE sampleBucketName = UUID.randomUUID().toString(); - builder.setBucketLayout(BucketLayout.DEFAULT); + builder.setBucketLayout(BucketLayout.OBJECT_STORE); volume.createBucket(sampleBucketName, builder.build()); bucket = volume.getBucket(sampleBucketName); Assert.assertEquals(sampleBucketName, bucket.getName()); @@ -107,7 +107,16 @@ public void testCreateBucketWithBucketLayout() throws Exception { volume.createBucket(sampleBucketName, builder.build()); bucket = volume.getBucket(sampleBucketName); Assert.assertEquals(sampleBucketName, bucket.getName()); - Assert.assertNotEquals(BucketLayout.LEGACY, bucket.getBucketLayout()); + Assert.assertEquals(BucketLayout.LEGACY, bucket.getBucketLayout()); + + // Case 3: Bucket layout: FILE_SYSTEM_OPTIMIZED + sampleBucketName = UUID.randomUUID().toString(); + builder.setBucketLayout(BucketLayout.FILE_SYSTEM_OPTIMIZED); + volume.createBucket(sampleBucketName, builder.build()); + bucket = volume.getBucket(sampleBucketName); + Assert.assertEquals(sampleBucketName, bucket.getName()); + Assert.assertEquals(BucketLayout.FILE_SYSTEM_OPTIMIZED, + bucket.getBucketLayout()); } /** diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/recon/TestReconWithOzoneManagerFSO.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/recon/TestReconWithOzoneManagerFSO.java index e2d59dfb8945..a893ee58134e 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/recon/TestReconWithOzoneManagerFSO.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/recon/TestReconWithOzoneManagerFSO.java @@ -32,7 +32,6 @@ import org.apache.hadoop.ozone.container.ContainerTestHelper; import org.apache.hadoop.ozone.container.TestHelper; import org.apache.hadoop.ozone.om.OMConfigKeys; -import org.apache.hadoop.ozone.om.helpers.BucketLayout; import org.apache.hadoop.ozone.recon.api.NSSummaryEndpoint; import org.apache.hadoop.ozone.recon.api.types.NamespaceSummaryResponse; import org.apache.hadoop.ozone.recon.api.types.EntityType; @@ -67,7 +66,7 @@ public class TestReconWithOzoneManagerFSO { public static void init() throws Exception { conf = new OzoneConfiguration(); conf.set(OMConfigKeys.OZONE_DEFAULT_BUCKET_LAYOUT, - BucketLayout.FILE_SYSTEM_OPTIMIZED.name()); + OMConfigKeys.OZONE_BUCKET_LAYOUT_FILE_SYSTEM_OPTIMIZED); cluster = MiniOzoneCluster.newBuilder(conf) .setNumDatanodes(1) diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/shell/TestOzoneShellHA.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/shell/TestOzoneShellHA.java index c0f634255c26..45f435720673 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/shell/TestOzoneShellHA.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/shell/TestOzoneShellHA.java @@ -963,20 +963,4 @@ public void testListVolumeBucketKeyShouldPrintValidJsonArray() testVolumes.forEach(vol -> execute(ozoneShell, new String[] { "volume", "delete", vol})); } - - @Test - public void testClientBucketLayoutValidation() throws Exception { - String[] args = new String[]{ - "bucket", "create", "o3://" + omServiceId + "/volume7" + "/bucketTest1", - "--layout", "LEGACY" - }; - try { - execute(ozoneShell, args); - Assert.fail("Should throw exception on unsupported bucket layouts!"); - } catch (Exception e) { - GenericTestUtils.assertExceptionContains( - "expected one of [FILE_SYSTEM_OPTIMIZED, OBJECT_STORE] ", - e); - } - } } diff --git a/hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicRootedOzoneClientAdapterImpl.java b/hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicRootedOzoneClientAdapterImpl.java index 35a027da901c..4a38566abd03 100644 --- a/hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicRootedOzoneClientAdapterImpl.java +++ b/hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicRootedOzoneClientAdapterImpl.java @@ -54,7 +54,6 @@ import org.apache.hadoop.ozone.OFSPath; import org.apache.hadoop.ozone.OmUtils; import org.apache.hadoop.ozone.OzoneConfigKeys; -import org.apache.hadoop.ozone.client.BucketArgs; import org.apache.hadoop.ozone.client.ObjectStore; import org.apache.hadoop.ozone.client.OzoneBucket; import org.apache.hadoop.ozone.client.OzoneClient; @@ -104,7 +103,6 @@ public class BasicRootedOzoneClientAdapterImpl private ReplicationConfig replicationConfig; private boolean securityEnabled; private int configuredDnPort; - private BucketLayout defaultOFSBucketLayout; private OzoneConfiguration config; /** @@ -191,11 +189,6 @@ public BasicRootedOzoneClientAdapterImpl(String omHost, int omPort, this.configuredDnPort = conf.getInt( OzoneConfigKeys.DFS_CONTAINER_IPC_PORT, OzoneConfigKeys.DFS_CONTAINER_IPC_PORT_DEFAULT); - - // Fetches the bucket layout to be used by OFS. - this.defaultOFSBucketLayout = BucketLayout.fromString( - conf.get(OzoneConfigKeys.OZONE_CLIENT_TEST_OFS_DEFAULT_BUCKET_LAYOUT, - OzoneConfigKeys.OZONE_CLIENT_TEST_OFS_BUCKET_LAYOUT_DEFAULT)); config = conf; } finally { Thread.currentThread().setContextClassLoader(contextClassLoader); @@ -264,9 +257,7 @@ private OzoneBucket getBucket(String volumeStr, String bucketStr, // Create the bucket try { // Buckets created by OFS should be in FSO layout - volume.createBucket(bucketStr, - BucketArgs.newBuilder().setBucketLayout( - this.defaultOFSBucketLayout).build()); + volume.createBucket(bucketStr); } catch (OMException newBucEx) { // Ignore the case where another client created the bucket if (!newBucEx.getResult().equals(BUCKET_ALREADY_EXISTS)) { diff --git a/hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/shell/bucket/CreateBucketHandler.java b/hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/shell/bucket/CreateBucketHandler.java index e1592e521a88..a836f02b73bc 100644 --- a/hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/shell/bucket/CreateBucketHandler.java +++ b/hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/shell/bucket/CreateBucketHandler.java @@ -57,11 +57,28 @@ public class CreateBucketHandler extends BucketHandler { " user if not specified") private String ownerName; - enum AllowedBucketLayouts { FILE_SYSTEM_OPTIMIZED, OBJECT_STORE } + enum AllowedBucketLayouts { + FILE_SYSTEM_OPTIMIZED("FILE_SYSTEM_OPTIMIZED"), + OBJECT_STORE("OBJECT_STORE"), + DEFAULT(""); + + // Assigning a value to each enum + private final String layout; + AllowedBucketLayouts(String layout) { + this.layout = layout; + } + + // Overriding toString() method to return the value passed to the + // constructor. + @Override + public String toString() { + return this.layout; + } + } @Option(names = { "--layout", "-l" }, description = "Allowed Bucket Layouts: ${COMPLETION-CANDIDATES}", - defaultValue = "OBJECT_STORE") + defaultValue = "") private AllowedBucketLayouts allowedBucketLayout; @CommandLine.Mixin @@ -80,7 +97,7 @@ public void execute(OzoneClient client, OzoneAddress address) BucketArgs.Builder bb; BucketLayout bucketLayout = - BucketLayout.valueOf(allowedBucketLayout.toString()); + BucketLayout.fromString(allowedBucketLayout.toString()); bb = new BucketArgs.Builder().setStorageType(StorageType.DEFAULT) .setVersioning(false).setBucketLayout(bucketLayout) .setOwner(ownerName);