HADOOP-18408. ABFS: Setting correct value of requireRenameResilience for nonHNS configuration #4758
Conversation
|
:::: AGGREGATED TEST RESULT :::: HNS-OAuth[INFO] Results: HNS-SharedKey[INFO] Results: NonHNS-SharedKey[INFO] Results: AppendBlob-HNS-OAuth[INFO] Results: Time taken: 60 mins 59 secs. |
...op-azure/src/test/java/org/apache/hadoop/fs/azurebfs/commit/ITestAbfsRenameStageFailure.java
Outdated
Show resolved
Hide resolved
...t/java/org/apache/hadoop/mapreduce/lib/output/committer/manifest/TestRenameStageFailure.java
Outdated
Show resolved
Hide resolved
sreeb-msft
changed the title
|
The latest commit passes the testResilienceAsExpected test in ITestAbfsRenameStageFailure. The requireRenameResilience value is correctly set to False for a nonHNS account, instead of True. This is valid, as etags are not being preserved for nonHNS accounts, it is expected that resilient commits would not be possible. Hence, the requirement is changed to False. |
|
🎊 +1 overall
This message was automatically generated. |
...op-azure/src/test/java/org/apache/hadoop/fs/azurebfs/commit/ITestAbfsRenameStageFailure.java
Outdated
Show resolved
Hide resolved
...op-azure/src/test/java/org/apache/hadoop/fs/azurebfs/commit/ITestAbfsRenameStageFailure.java
Outdated
Show resolved
Hide resolved
|
💔 -1 overall
This message was automatically generated. |
|
@steveloughran can you please take a look at this PR? Here is the corresponding JIRA. |
|
🎊 +1 overall
This message was automatically generated. |
| @@ -41,6 +42,12 @@ public ITestAbfsRenameStageFailure() throws Exception { | |||
| binding = new ABFSContractTestBinding(); | |||
| } | |||
|
|
|||
| protected boolean isNamespaceEnabled() { | |||
| FileSystem fs = getFileSystem(); | |||
| String namespaceEnabled = fs.getConf().get("fs.azure.test.namespace.enabled"); | |||
There was a problem hiding this comment.
there's a Configuration.getBoolean(key, defval) which should be used for evaluating true/false. it knows about trimming strings, case independence etc. have a look at the class and see what else it can do. using the basic get() without a default is generally a rare action.
better.
return conf.getBoolean("fs.azure.test.namespace.enabled", false)
- other tests, specificlly everything under
AbstractAbfsIntegrationTest, probe the fs for having ACLs and use that to dynamically determine fs capabilities. seegetIsNamespaceEnabled()for how it is done.
i would prefer you copy the AbstractAbfsIntegrationTest algorithm as it is based on the actual FS capabilities
There was a problem hiding this comment.
Have made changes accordingly in the latest commit (picking up file capabilities dynamically instead of from the test config). Can you please take a look at it? Thank you!
...op-azure/src/test/java/org/apache/hadoop/fs/azurebfs/commit/ITestAbfsRenameStageFailure.java
Outdated
Show resolved
Hide resolved
|
looks good. have you tested it with a hierachical fs to make sure all is good there? |
|
🎊 +1 overall
This message was automatically generated. |
@steveloughran Yes, tested and it passes for HNS settings too. |
…onfiguration (#4758) ITestAbfsManifestCommitProtocol to set requireRenameResilience to false for nonHNS configuration Contributed by Sree Bhattacharyya
…onfiguration (apache#4758) ITestAbfsManifestCommitProtocol to set requireRenameResilience to false for nonHNS configuration (apache#4758) Contributed by Sree Bhattacharyya
This commit ignores the testResilienceAsExpected test in ITestAbfsRenameStageFailure.
ITestAbfsRenameStageFailure fails for the NonHNS SharedKey configuration -
[ERROR] ITestAbfsRenameStageFailure>TestRenameStageFailure.testResilienceAsExpected:126 [resilient commit support] expected:<[tru]e> but was:<[fals]e>The check for whether ResilientCommits are possible depends on whether etags are preserved in rename or not. If not, an exception is thrown which leads the ResilientCommitByRename flag to remain set to
nulland ultimately causes the test failure.In case of nonHNS accounts, as etags are not preserved in rename, the test run is not valid. Hence for this particular configuration the test has been ignored.