-
Notifications
You must be signed in to change notification settings - Fork 9.1k
Hadoop 17132. ABFS: Fix Rename and Delete Idempotency check trigger #2150
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Test results: From East US2 accounts. With and Without HNS results below: non-HNS HNS The 2 tests from NetworkStatistics is expecting a certain number of GetFileStatus calls before the test issues any request. This is needs to be fixed. Created JIRA to track it : https://issues.apache.org/jira/browse/HADOOP-17137 |
|
💔 -1 overall
This message was automatically generated. |
|
Javadoc failure because of JDK11 support. Being addressed in https://issues.apache.org/jira/browse/HADOOP-16862 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This has pretty serious testing...the production code isn't so complex but I'm still going to have to trust you and bilihari's review
LGTM
A couple of minor nits and then it's ready to go in.
| * HTTP operations. | ||
| */ | ||
| void execute() throws AzureBlobFileSystemException { | ||
| public void execute() throws AzureBlobFileSystemException { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tag this as @VisibleForTesting too
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
| false, | ||
| null)); | ||
|
|
||
| // mock idempotency check to mimick retried case |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
check spelling of mimic
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
|
Thanks @steveloughran . Have addressed the comments. |
|
💔 -1 overall
This message was automatically generated. |
|
Javadoc failure because of JDK11 support. Being addressed in https://issues.apache.org/jira/browse/HADOOP-16862 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LTGM, +1
Trigger to call the idempotent code introduced in https://issues.apache.org/jira/browse/HADOOP-17015 is incomplete.
When a request fails at the AbfsRestOperation, an exception is thrown from the REST response handling code . HADOOP-17015 was failing to catch it before checking the AbfsRestOperation instance status code.
The request retry count was also getting set to 1 even when request wasnt re-tired. That has been fixed too.
Tests are added by setting final fields in AbfsClient by reflection and mimicking the idempotency check return values to ensure that the trigger handles all scenarios.