You are viewing a plain text version of this content. The canonical link for it is here.
Posted to common-issues@hadoop.apache.org by "saxenapranav (via GitHub)" <gi...@apache.org> on 2023/05/17 04:53:13 UTC
[GitHub] [hadoop] saxenapranav commented on a diff in pull request #5661: [ABFS] HADOOP-18656: Paginated Delete Driver Testing
saxenapranav commented on code in PR #5661:
URL: https://github.com/apache/hadoop/pull/5661#discussion_r1195934399
##########
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsClient.java:
##########
@@ -87,7 +87,7 @@ public class AbfsClient implements Closeable {
private final URL baseUrl;
private final SharedKeyCredentials sharedKeyCredentials;
- private final String xMsVersion = "2019-12-12";
+ private String xMsVersion = "2019-12-12";
Review Comment:
Dont we need to switch it to "2023-08-03"?
Should we switch only for deletePath and not for others?
##########
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsClient.java:
##########
@@ -867,6 +867,14 @@ public AbfsRestOperation deletePath(final String path, final boolean recursive,
final List<AbfsHttpHeader> requestHeaders = createDefaultHeaders();
final AbfsUriQueryBuilder abfsUriQueryBuilder = createDefaultUriQueryBuilder();
+
+ boolean enablePagination = abfsConfiguration.getBoolean(
+ ConfigurationKeys.FS_AZURE_ENABLE_PAGINATED_DELETE,
+ DEFAULT_ENABLE_PAGINATED_DELETE
+ );
+
Review Comment:
lets use `abfsConfiguration.isEnabledPaginatedDelete`
##########
hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/services/ITestAbfsPaginatedDelete.java:
##########
@@ -0,0 +1,300 @@
+/**
+ * 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.azurebfs.services;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.FileSystem;
+import org.apache.hadoop.fs.Path;
+import org.apache.hadoop.fs.azurebfs.AbfsConfiguration;
+import org.apache.hadoop.fs.azurebfs.AbstractAbfsIntegrationTest;
+import org.apache.hadoop.fs.azurebfs.AzureBlobFileSystem;
+import org.apache.hadoop.fs.azurebfs.constants.ConfigurationKeys;
+import org.apache.hadoop.fs.azurebfs.constants.HttpHeaderConfigurations;
+import org.apache.hadoop.fs.azurebfs.contracts.exceptions.AbfsRestOperationException;
+import org.apache.hadoop.fs.azurebfs.oauth2.ClientCredsTokenProvider;
+import org.apache.hadoop.fs.azurebfs.utils.AclTestHelpers;
+import org.apache.hadoop.fs.azurebfs.utils.TracingContext;
+import org.apache.hadoop.fs.permission.AclEntry;
+import org.apache.hadoop.fs.permission.AclEntryScope;
+import org.apache.hadoop.fs.permission.AclEntryType;
+import org.apache.hadoop.fs.permission.FsAction;
+import org.apache.hadoop.util.Lists;
+import org.junit.Assume;
+import org.junit.Test;
+
+import java.io.IOException;
+import java.net.HttpURLConnection;
+import java.util.List;
+
+import static org.apache.hadoop.fs.azurebfs.constants.ConfigurationKeys.FS_AZURE_ENABLE_PAGINATED_DELETE;
+import static org.apache.hadoop.fs.azurebfs.constants.ConfigurationKeys.FS_AZURE_ACCOUNT_AUTH_TYPE_PROPERTY_NAME;
+import static org.apache.hadoop.fs.azurebfs.constants.ConfigurationKeys.FS_AZURE_ACCOUNT_OAUTH_CLIENT_ENDPOINT;
+import static org.apache.hadoop.fs.azurebfs.constants.ConfigurationKeys.AZURE_CREATE_REMOTE_FILESYSTEM_DURING_INITIALIZATION;
+import static org.apache.hadoop.fs.azurebfs.constants.ConfigurationKeys.FS_AZURE_ACCOUNT_TOKEN_PROVIDER_TYPE_PROPERTY_NAME;
+import static org.apache.hadoop.fs.azurebfs.constants.TestConfigurationKeys.FS_AZURE_BLOB_FS_CHECKACCESS_TEST_CLIENT_ID;
+import static org.apache.hadoop.fs.azurebfs.constants.TestConfigurationKeys.FS_AZURE_BLOB_FS_CHECKACCESS_TEST_CLIENT_SECRET;
+import static org.apache.hadoop.fs.azurebfs.constants.TestConfigurationKeys.FS_AZURE_BLOB_FS_CHECKACCESS_TEST_USER_GUID;
+import static org.apache.hadoop.fs.azurebfs.constants.TestConfigurationKeys.FS_AZURE_BLOB_FS_CLIENT_ID;
+import static org.apache.hadoop.fs.azurebfs.constants.TestConfigurationKeys.FS_AZURE_BLOB_FS_CLIENT_SECRET;
+import static org.apache.hadoop.fs.azurebfs.constants.TestConfigurationKeys.FS_AZURE_TEST_NAMESPACE_ENABLED_ACCOUNT;
+import static org.apache.hadoop.test.LambdaTestUtils.intercept;
+
+public class ITestAbfsPaginatedDelete extends AbstractAbfsIntegrationTest {
+
+ private AzureBlobFileSystem superUserFs;
+ private AzureBlobFileSystem firstTestUserFs;
+ private String firstTestUserGuid;
+
+ private boolean isHnsEnabled;
+ public ITestAbfsPaginatedDelete() throws Exception {
+ }
+
+ @Override
+ public void setup() throws Exception {
+ isHnsEnabled = this.getConfiguration().getBoolean(FS_AZURE_TEST_NAMESPACE_ENABLED_ACCOUNT, false);
+ loadConfiguredFileSystem();
+ super.setup();
+ this.superUserFs = getFileSystem();
+ this.firstTestUserGuid = getConfiguration()
+ .get(FS_AZURE_BLOB_FS_CHECKACCESS_TEST_USER_GUID);
+
+ if(isHnsEnabled) {
+ // setting up ACL permissions for test user
+ setFirstTestUserFsAuth();
+ setDefaultAclOnRoot(this.firstTestUserGuid);
+ }
+
+ }
+
+ @Test
+ public void testFnsDeleteWithPaginationTrue() throws Exception {
+ Assume.assumeFalse(isHnsEnabled);
+ Path smallDirPath = createSmallDir();
+
+ AbfsClient client = getFileSystem().getAbfsStore().getClient();
+ AbfsClient finalClient = TestAbfsClient.setAbfsClientField(client, "xMsVersion", "2023-08-03");
+ AbfsConfiguration abfsConfig = finalClient.getAbfsConfiguration();
+ abfsConfig.setBoolean(FS_AZURE_ENABLE_PAGINATED_DELETE, true);
+
+ TracingContext testTracingContext = getTestTracingContext(this.firstTestUserFs, true);
+ finalClient.deletePath(smallDirPath.toString(), true, null, testTracingContext);
+
+ AbfsRestOperationException e = intercept(AbfsRestOperationException.class, () ->
+ finalClient.getPathStatus(smallDirPath.toString(), false, testTracingContext));
+ assertEquals(HttpURLConnection.HTTP_NOT_FOUND, e.getStatusCode());
+ }
+
+ @Test
+ public void testFnsDeleteWithPaginationFalse() throws Exception {
+ Assume.assumeFalse(isHnsEnabled);
+ Path smallDirPath = createSmallDir();
+
+ AbfsClient client = getFileSystem().getAbfsStore().getClient();
+ AbfsClient finalClient = TestAbfsClient.setAbfsClientField(client, "xMsVersion", "2023-08-03");
+ AbfsConfiguration abfsConfig = finalClient.getAbfsConfiguration();
+ abfsConfig.setBoolean(FS_AZURE_ENABLE_PAGINATED_DELETE, true);
+
+ TracingContext testTracingContext = getTestTracingContext(this.firstTestUserFs, true);
+ finalClient.deletePath(smallDirPath.toString(), true, null, testTracingContext);
+
+ AbfsRestOperationException e = intercept(AbfsRestOperationException.class, () ->
+ finalClient.getPathStatus(smallDirPath.toString(), false, testTracingContext));
+ assertEquals(HttpURLConnection.HTTP_NOT_FOUND, e.getStatusCode());
+ }
+
+ @Test
+ public void testVersionForPagination() throws Exception {
+ Assume.assumeTrue(isHnsEnabled);
+ Path smallDirPath = createSmallDir();
+ AbfsClient client = this.firstTestUserFs.getAbfsStore().getClient();
+ AbfsConfiguration abfsConfig = client.getAbfsConfiguration();
+
+ // delete should fail with bad request as version does not support pagination
+ abfsConfig.setBoolean(ConfigurationKeys.FS_AZURE_ENABLE_PAGINATED_DELETE, true);
+ AbfsRestOperationException e = intercept(AbfsRestOperationException.class, () ->
+ client.deletePath(smallDirPath.toString(), true, null, getTestTracingContext(this.firstTestUserFs, false))
+ );
+ assertEquals(HttpURLConnection.HTTP_BAD_REQUEST, e.getStatusCode());
+ }
+
+ @Test
+ public void testInvalidPaginationTrueRecursiveFalse() throws Exception {
+ Assume.assumeTrue(isHnsEnabled);
+ Path smallDirPath = createSmallDir();
+
+ AbfsClient client = this.firstTestUserFs.getAbfsStore().getClient();
+ AbfsClient finalClient = TestAbfsClient.setAbfsClientField(client, "xMsVersion", "2023-08-03");
+ AbfsConfiguration abfsConfig = finalClient.getAbfsConfiguration();
+ abfsConfig.setBoolean(ConfigurationKeys.FS_AZURE_ENABLE_PAGINATED_DELETE, true);
+
+ // delete should fail with HTTP as recursive will be set to false
+ // but pagination parameter is set to true
+ String path = smallDirPath.toString();
+ AbfsRestOperationException e = intercept(AbfsRestOperationException.class, () ->
+ finalClient.deletePath(path, false, null, getTestTracingContext(this.firstTestUserFs, false)));
+ assertEquals(HttpURLConnection.HTTP_BAD_REQUEST, e.getStatusCode());
+ }
+
+ @Test
+ public void testInvalidPaginationFalseRandomCt() throws Exception {
+ Assume.assumeTrue(isHnsEnabled);
+ Path smallDirPath = createSmallDir();
+
+ AbfsClient client = this.firstTestUserFs.getAbfsStore().getClient();
+ AbfsClient finalClient = TestAbfsClient.setAbfsClientField(client, "xMsVersion", "2023-08-03");
+ AbfsConfiguration abfsConfig = finalClient.getAbfsConfiguration();
+ TracingContext testTracingContext = getTestTracingContext(this.firstTestUserFs, true);
+
+ abfsConfig.setBoolean(ConfigurationKeys.FS_AZURE_ENABLE_PAGINATED_DELETE, false);
+ String ct = "randomToken12345";
+ AbfsRestOperationException e = intercept(AbfsRestOperationException.class, () ->
+ finalClient.deletePath(smallDirPath.toString(), true, ct, getTestTracingContext(this.firstTestUserFs, false))
+ );
+
+ assertEquals(HttpURLConnection.HTTP_BAD_REQUEST, e.getStatusCode());
+ }
+
+ @Test
+ public void testInvalidRecursiveFalseRandomCt() throws Exception {
+ Assume.assumeTrue(isHnsEnabled);
+ Path smallDirPath = createSmallDir();
+
+ AbfsClient client = this.firstTestUserFs.getAbfsStore().getClient();
+ AbfsClient finalClient = TestAbfsClient.setAbfsClientField(client, "xMsVersion", "2023-08-03");
+ AbfsConfiguration abfsConfig = finalClient.getAbfsConfiguration();
+ TracingContext testTracingContext = getTestTracingContext(this.firstTestUserFs, true);
+
+ abfsConfig.setBoolean(ConfigurationKeys.FS_AZURE_ENABLE_PAGINATED_DELETE, true);
+ String ct = "randomToken12345";
+ AbfsRestOperationException e = intercept(AbfsRestOperationException.class, () ->
+ finalClient.deletePath(smallDirPath.toString(), false, ct, getTestTracingContext(this.firstTestUserFs, false))
+ );
+
+ assertEquals(HttpURLConnection.HTTP_BAD_REQUEST, e.getStatusCode());
+ }
+
+ @Test
+ public void testInvalidRecursiveFalsePaginationTrue() throws Exception {
+ Assume.assumeTrue(isHnsEnabled);
+ Path smallDirPath = createSmallDir();
+
+ AbfsClient client = this.firstTestUserFs.getAbfsStore().getClient();
+ AbfsClient finalClient = TestAbfsClient.setAbfsClientField(client, "xMsVersion", "2023-08-03");
+ AbfsConfiguration abfsConfig = finalClient.getAbfsConfiguration();
+ TracingContext testTracingContext = getTestTracingContext(this.firstTestUserFs, true);
+
+ abfsConfig.setBoolean(ConfigurationKeys.FS_AZURE_ENABLE_PAGINATED_DELETE, true);
+ AbfsRestOperationException e = intercept(AbfsRestOperationException.class, () ->
+ finalClient.deletePath(smallDirPath.toString(), false, null, getTestTracingContext(this.firstTestUserFs, false))
+ );
+ }
+
+ @Test
+ public void testValidRecursiveTruePaginationFalse() throws Exception {
+ Assume.assumeTrue(isHnsEnabled);
+ Path smallDirPath = createSmallDir();
+
+ AbfsClient client = this.firstTestUserFs.getAbfsStore().getClient();
+ AbfsClient finalClient = TestAbfsClient.setAbfsClientField(client, "xMsVersion", "2023-08-03");
+ AbfsConfiguration abfsConfig = finalClient.getAbfsConfiguration();
+ TracingContext testTracingContext = getTestTracingContext(this.firstTestUserFs, true);
+
+ abfsConfig.setBoolean(ConfigurationKeys.FS_AZURE_ENABLE_PAGINATED_DELETE, false);
+ finalClient.deletePath(smallDirPath.toString(), true, null, testTracingContext);
+
+ AbfsRestOperationException e = intercept(AbfsRestOperationException.class, () ->
+ finalClient.getPathStatus(smallDirPath.toString(), false, testTracingContext));
+ assertEquals(HttpURLConnection.HTTP_NOT_FOUND, e.getStatusCode());
+ }
+
+ private void setFirstTestUserFsAuth() throws IOException {
+ if (this.firstTestUserFs != null) {
+ return;
+ }
+ checkIfConfigIsSet(FS_AZURE_ACCOUNT_OAUTH_CLIENT_ENDPOINT
+ + "." + getAccountName());
+ Configuration conf = getRawConfiguration();
+ setTestFsConf(FS_AZURE_BLOB_FS_CLIENT_ID, FS_AZURE_BLOB_FS_CHECKACCESS_TEST_CLIENT_ID);
+ setTestFsConf(FS_AZURE_BLOB_FS_CLIENT_SECRET,
+ FS_AZURE_BLOB_FS_CHECKACCESS_TEST_CLIENT_SECRET);
+ conf.set(FS_AZURE_ACCOUNT_AUTH_TYPE_PROPERTY_NAME, AuthType.OAuth.name());
+ conf.set(FS_AZURE_ACCOUNT_TOKEN_PROVIDER_TYPE_PROPERTY_NAME + "."
+ + getAccountName(), ClientCredsTokenProvider.class.getName());
+ conf.setBoolean(AZURE_CREATE_REMOTE_FILESYSTEM_DURING_INITIALIZATION,
+ false);
+ this.firstTestUserFs = (AzureBlobFileSystem) FileSystem.newInstance(getRawConfiguration());
+ }
+
+
+ private void setTestFsConf(final String fsConfKey,
+ final String testFsConfKey) {
+ final String confKeyWithAccountName = fsConfKey + "." + getAccountName();
+ final String confValue = getConfiguration()
+ .getString(testFsConfKey, "");
+ getRawConfiguration().set(confKeyWithAccountName, confValue);
+ }
+
+ private void setDefaultAclOnRoot(String uid)
+ throws IOException {
+ List<AclEntry> aclSpec = Lists.newArrayList(AclTestHelpers
+ .aclEntry(AclEntryScope.ACCESS, AclEntryType.USER, uid, FsAction.ALL),
+ AclTestHelpers.aclEntry(AclEntryScope.DEFAULT, AclEntryType.USER, uid, FsAction.ALL));
+ this.superUserFs.modifyAclEntries(new Path("/"), aclSpec);
+ }
+
+ private String getContinuationToken(AbfsHttpOperation resultOp) {
+ String continuation = resultOp.getResponseHeader(HttpHeaderConfigurations.X_MS_CONTINUATION);
+ return continuation;
+ }
+
+
+ private Path createLargeDir() throws IOException {
+ AzureBlobFileSystem fs = getFileSystem();
+ String rootPath = "/largeDir";
+ String firstFilePath = rootPath + "/placeholderFile";
+ fs.create(new Path(firstFilePath));
+
+ for (int i = 1; i <= 515; i++) {
+ String dirPath = "/dirLevel1" + String.valueOf(i) + "/dirLevel2" + String.valueOf(i);
+ String filePath = rootPath + dirPath + "/file" + String.valueOf(i);
+ fs.create(new Path(filePath));
+ }
Review Comment:
Lets have parallel create.
Why 515?
##########
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsClient.java:
##########
@@ -72,8 +73,7 @@
import static org.apache.hadoop.fs.azurebfs.AbfsStatistic.RENAME_PATH_ATTEMPTS;
import static org.apache.hadoop.fs.azurebfs.AzureBlobFileSystemStore.extractEtagHeader;
import static org.apache.hadoop.fs.azurebfs.constants.AbfsHttpConstants.*;
-import static org.apache.hadoop.fs.azurebfs.constants.FileSystemConfigurations.DEFAULT_DELETE_CONSIDERED_IDEMPOTENT;
-import static org.apache.hadoop.fs.azurebfs.constants.FileSystemConfigurations.SERVER_SIDE_ENCRYPTION_ALGORITHM;
+import static org.apache.hadoop.fs.azurebfs.constants.FileSystemConfigurations.*;
Review Comment:
wildcard import.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org