HADOOP-18960: [ABFS] Making Contract tests run in sequential and Other Test Fixes #7104
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
:::: AGGREGATED TEST RESULT :::: ============================================================
|
anujmodi2021
changed the title
anujmodi2021
changed the title
|
@steveloughran @mukund-thakur |
|
🎊 +1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
| @@ -201,4 +183,20 @@ public void testAuthFailException() throws Exception { | |||
| .describedAs("Incorrect error message: " + errorDesc) | |||
| .contains("Auth failure: "); | |||
| } | |||
|
|
|||
| private Configuration getEntries(final int numOfRetries) { | |||
| @@ -65,8 +67,8 @@ public void setup() throws Exception { | |||
| TestConfigurationKeys.FS_AZURE_TEST_NAMESPACE_ENABLED_ACCOUNT, false); | |||
| Assume.assumeTrue(isHNSEnabled); | |||
| loadConfiguredFileSystem(); | |||
| this.getConfiguration().set(ConfigurationKeys.FS_AZURE_SAS_TOKEN_PROVIDER_TYPE, TEST_AUTHZ_CLASS); | |||
| this.getConfiguration().set(ConfigurationKeys.FS_AZURE_ACCOUNT_AUTH_TYPE_PROPERTY_NAME, "SAS"); | |||
| this.getConfiguration().set(FS_AZURE_SAS_TOKEN_PROVIDER_TYPE, TEST_AUTHZ_CLASS); | |||
| @@ -68,6 +68,9 @@ protected Configuration createConfiguration() { | |||
| protected AbstractFSContract createContract(final Configuration conf) { | |||
| conf.setInt(AZURE_READ_AHEAD_RANGE, MIN_BUFFER_SIZE); | |||
| conf.setInt(AZURE_READ_BUFFER_SIZE, MIN_BUFFER_SIZE); | |||
| // Disabling cache to make sure new configs are picked up. | |||
There was a problem hiding this comment.
pull out into an azure test utils class, just as `S3ATestUtils.disableFilesystemCaching()does
There was a problem hiding this comment.
Great suggestions. Taken
There was a problem hiding this comment.
Already had a AbfsTestUtils class defined. Added there itself.
I noticed that class has other methods without any usage. Not sure how but was wondering should they be removed or not.
| config.set(AZURE_CUSTOM_TOKEN_FETCH_RETRY_COUNT, Integer.toString( | ||
| numOfRetries)); | ||
| // Stop filesystem creation as it will lead to calls to store. | ||
| config.set(AZURE_CREATE_REMOTE_FILESYSTEM_DURING_INITIALIZATION, "false"); |
There was a problem hiding this comment.
Taken everywhere applicable
| numOfRetries)); | ||
| // Stop filesystem creation as it will lead to calls to store. | ||
| config.set(AZURE_CREATE_REMOTE_FILESYSTEM_DURING_INITIALIZATION, "false"); | ||
| config.set(FS_AZURE_ACCOUNT_IS_HNS_ENABLED, config.getBoolean( |
| testConfig.set(ConfigurationKeys.FS_AZURE_SAS_TOKEN_PROVIDER_TYPE, TEST_ERR_AUTHZ_CLASS); | ||
| Configuration testConfig = new Configuration(this.getConfiguration().getRawConfiguration()); | ||
| testConfig.set(FS_AZURE_SAS_TOKEN_PROVIDER_TYPE, TEST_ERR_AUTHZ_CLASS); | ||
| testConfig.set(MOCK_SASTOKENPROVIDER_RETURN_EMPTY_SAS_TOKEN, "true"); |
| testConfig.set(FS_AZURE_SAS_TOKEN_PROVIDER_TYPE, TEST_ERR_AUTHZ_CLASS); | ||
| testConfig.set(MOCK_SASTOKENPROVIDER_RETURN_EMPTY_SAS_TOKEN, "true"); | ||
| // Setting IS_HNS_ENABLED to avoid the exception thrown by the HNS check. | ||
| testConfig.set(FS_AZURE_ACCOUNT_IS_HNS_ENABLED, this.getIsNamespaceEnabled(fs) + ""); |
There was a problem hiding this comment.
why do we need to append "" at the end?
There was a problem hiding this comment.
Was needed to conver boolean to string. But with setBoolean not needed.
| @@ -68,6 +68,9 @@ protected Configuration createConfiguration() { | |||
| protected AbstractFSContract createContract(final Configuration conf) { | |||
| conf.setInt(AZURE_READ_AHEAD_RANGE, MIN_BUFFER_SIZE); | |||
| conf.setInt(AZURE_READ_BUFFER_SIZE, MIN_BUFFER_SIZE); | |||
| // Disabling cache to make sure new configs are picked up. | |||
| conf.set("fs.abfss.impl.disable.cache", "true"); | |||
| conf.set("fs.abfs.impl.disable.cache", "true"); | |||
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
steveloughran
requested changes
Oct 18, 2024
| @@ -21,6 +21,7 @@ | |||
| import com.microsoft.azure.storage.blob.CloudBlobClient; | |||
| import com.microsoft.azure.storage.blob.CloudBlobContainer; | |||
|
|
|||
| import org.apache.hadoop.conf.Configuration; | |||
There was a problem hiding this comment.
nit: import ordering
|
💔 -1 overall
This message was automatically generated. |
|
Thanks for suggestions @steveloughran |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description of PR
Target Jira: https://issues.apache.org/jira/browse/HADOOP-18960
The execution of contract-tests in
abfsworks as perexecutionId = integration-test-abfs-parallel-classesof the pom. The tests would work in different jvms, and at a given instance multiple such jvms could be there, depending on ${testsThreadCount}. The problem is that all the test jvms for contract-test use the same container for test runs which is defined byfs.contract.test.fs.abfs. Due to this, one jvm root-contract-runs can influence other jvm's root-contract-runs. This leads to CI failures for hadoop-azure package.Hence making these tests run in parallel so that they don't interfere with each other.
This PR also fixes some test failures reported by Jira: https://issues.apache.org/jira/browse/HADOOP-19106
How was this patch tested?
Test Suite Run Result pasted in comments below.
The failing tests are known and due to some missing configs. They are fixed in #6847
For code changes: