Skip to content
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.

Sign up for GitHub

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

Jump to bottom

HADOOP-17458. S3A to treat "SdkClientException: Data read has a different length than the expected" as EOFException #3040

Merged
merged 4 commits into from
Jul 23, 2021
Merged

HADOOP-17458. S3A to treat "SdkClientException: Data read has a different length than the expected" as EOFException #3040

merged 4 commits into from
Jul 23, 2021

Conversation

bogthe
Copy link
Contributor

@bogthe bogthe commented May 21, 2021

HADOOP-17458

Some network exceptions would cause SdkClientException to fail with a
Data read has a different length than the expected message. These
should be recoverable.

  • Modifying translateException to recover from the above exception;

Tested with eu-west-1 and mvn -Dparallel-tests -DtestsThreadCount=32 clean verify

[INFO] Results:
[INFO]
[WARNING] Tests run: 151, Failures: 0, Errors: 0, Skipped: 87

Bogdan Stolojan added 3 commits May 21, 2021 11:46
HADOOP-17458 S3A to treat "SdkClientException: Data read has a differ…
3ae52ee 
…ent length than the expected" as EOFException

Some network exceptions would cause `SdkClientException` to fail with a
`Data read has a different length than the expected` message. These
should be recoverable.

- Modifying `translateException` to recover from the above exception;
Improving spelling
7133719
Adding test to verify translatable messages
f975516 
Added a test to check if the errors which we're translating by looking
at the message are actually doing what we're expecting.
steveloughran
steveloughran requested changes Jun 11, 2021
View reviewed changes
Copy link
Contributor

@steveloughran steveloughran left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

code looks good.
we really some unit test for this, don't we? (we should have had one from the outset....)
Could you add one in org.apache.hadoop.fs.s3a.TestInvoker ? That'll stop us breaking this in future

Tests to verify exceptions for Invoker.retry
b868a72 
Adding new unit tests to make sure that the new translatable exceptions
are being translated and retried appropriately.
@bogthe
Copy link
Contributor Author

bogthe commented Jun 14, 2021

Sure thing 👍 ! Added them.

@hadoop-yetus
Copy link

🎊 +1 overall

Vote Subsystem Runtime Logfile Comment
+0 🆗 reexec 0m 52s Docker mode activated.
_ Prechecks _
+1 💚 dupname 0m 0s No case conflicting files found.
+0 🆗 codespell 0m 0s codespell was not available.
+1 💚 @author 0m 0s The patch does not contain any @author tags.
+1 💚 test4tests 0m 0s The patch appears to include 1 new or modified test files.
_ trunk Compile Tests _
+1 💚 mvninstall 35m 23s trunk passed
+1 💚 compile 0m 45s trunk passed with JDK Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04
+1 💚 compile 0m 35s trunk passed with JDK Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10
+1 💚 checkstyle 0m 26s trunk passed
+1 💚 mvnsite 0m 43s trunk passed
+1 💚 javadoc 0m 21s trunk passed with JDK Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04
+1 💚 javadoc 0m 30s trunk passed with JDK Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10
+1 💚 spotbugs 1m 11s trunk passed
+1 💚 shadedclient 20m 24s branch has no errors when building and testing our client artifacts.
_ Patch Compile Tests _
+1 💚 mvninstall 0m 41s the patch passed
+1 💚 compile 0m 46s the patch passed with JDK Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04
+1 💚 javac 0m 46s the patch passed
+1 💚 compile 0m 35s the patch passed with JDK Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10
+1 💚 javac 0m 35s the patch passed
+1 💚 blanks 0m 0s The patch has no blanks issues.
-0 ⚠️ checkstyle 0m 22s /results-checkstyle-hadoop-tools_hadoop-aws.txt hadoop-tools/hadoop-aws: The patch generated 2 new + 8 unchanged - 0 fixed = 10 total (was 8)
+1 💚 mvnsite 0m 43s the patch passed
+1 💚 javadoc 0m 15s the patch passed with JDK Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04
+1 💚 javadoc 0m 29s the patch passed with JDK Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10
+1 💚 spotbugs 1m 31s the patch passed
+1 💚 shadedclient 20m 21s patch has no errors when building and testing our client artifacts.
_ Other Tests _
+1 💚 unit 2m 49s hadoop-aws in the patch passed.
+1 💚 asflicense 0m 41s The patch does not generate ASF License warnings.
91m 6s
Subsystem Report/Notes
Docker ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-3040/3/artifact/out/Dockerfile
GITHUB PR #3040
Optional Tests dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell
uname Linux 2d24ea91d6a9 4.15.0-136-generic #140-Ubuntu SMP Thu Jan 28 05:20:47 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux
Build tool maven
Personality dev-support/bin/hadoop.sh
git revision trunk / b868a72
Default Java Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10
Multi-JDK versions /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10
Test Results https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-3040/3/testReport/
Max. process+thread count 521 (vs. ulimit of 5500)
modules C: hadoop-tools/hadoop-aws U: hadoop-tools/hadoop-aws
Console output https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-3040/3/console
versions git=2.25.1 maven=3.6.3 spotbugs=4.2.2
Powered by Apache Yetus 0.14.0-SNAPSHOT https://yetus.apache.org

This message was automatically generated.

@bogthe
Copy link
Contributor Author

bogthe commented Jul 22, 2021

@steveloughran let me know if the tests are ok, it's been a while for this PR and wanted to get it closed :D

steveloughran
steveloughran approved these changes Jul 23, 2021
View reviewed changes
Copy link
Contributor

@steveloughran steveloughran left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

checkstyle is all line length. I'm Ok with that if it's not too wide and cutting down would actually hurt readability

@steveloughran steveloughran changed the title HADOOP-17458 S3A to treat "SdkClientException: Data read has a different length than the expected" as EOFException HADOOP-17458. S3A to treat "SdkClientException: Data read has a different length than the expected" as EOFException Jul 23, 2021
@steveloughran steveloughran merged commit 63dfd84 into apache:trunk Jul 23, 2021
@steveloughran
Copy link
Contributor

(oh, two commits on trunk earlier: https://issues.apache.org/jira/browse/HADOOP-17813 , line length is now 100 chars!)

asfgit pushed a commit that referenced this pull request Jul 23, 2021
@bogthe @steveloughran
HADOOP-17458. S3A to treat "SdkClientException: Data read has a diffe…
e89d30b 
…rent length than the expected" as EOFException (#3040)

Some network exceptions can raise SdkClientException with message
`Data read has a different length than the expected`.

These should be recoverable.

Contributed by Bogdan Stolojan

Change-Id: Ia22fd77d90971e9e02b4f947398a4749eebe5909
@apache apache deleted a comment from hadoop-yetus Jul 27, 2021
@apache apache deleted a comment from hadoop-yetus Jul 27, 2021
kiran-maturi pushed a commit to kiran-maturi/hadoop that referenced this pull request Nov 24, 2021
@bogthe
HADOOP-17458. S3A to treat "SdkClientException: Data read has a diffe…
e7de0ca 
…rent length than the expected" as EOFException (apache#3040)


Some network exceptions can raise SdkClientException with message
`Data read has a different length than the expected`.

These should be recoverable.

Contributed by Bogdan Stolojan
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@steveloughran steveloughran steveloughran approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@bogthe @hadoop-yetus @steveloughran