HDFS-16422. Fix thread safety of EC decoding during concurrent preads #3881
Conversation
This file contains hidden or 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
|
💔 -1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
Contributor
Author
|
@sodonnel Do you have time to take a look on this pr? Thanks a lot. |
jojochuang
reviewed
Jan 25, 2022
Contributor
There was a problem hiding this comment.
IMO, we should make RawErasureDecoder.decode(ByteBuffer[] inputs, int[] erasedIndexes, ByteBuffer[] outputs) synchronized instead.
Contributor
Author
There was a problem hiding this comment.
Thanks for your review. @jojochuang
Yes, that's better. I will update it in a few minutes.
|
💔 -1 overall
This message was automatically generated. |
cndaimin
force-pushed
the
fix_ec_coder
branch
from
65aedd2 to
4045218
Compare
|
💔 -1 overall
This message was automatically generated. |
cndaimin
force-pushed
the
fix_ec_coder
branch
from
4045218 to
a1b7145
Compare
|
💔 -1 overall
This message was automatically generated. |
Contributor
|
@whbing Did you encounter this problem? |
jojochuang
requested changes
Feb 2, 2022
Contributor
jojochuang
left a comment
jojochuang
left a comment
There was a problem hiding this comment.
The fix looks good but I don't like the test.
It looks like a very heavy test for something trivial. The test ran for 5 minutes and timed out on my laptop. From the look of the test it seems to have some randomness embedded in it.
IMO, I can accept the PR if the test is removed. The fix itself is good enough.
Contributor
Author
|
Thanks for your review. @jojochuang |
|
💔 -1 overall
This message was automatically generated. |
cndaimin
force-pushed
the
fix_ec_coder
branch
from
59885e8 to
7f72a4c
Compare
|
💔 -1 overall
This message was automatically generated. |
asfgit
pushed a commit
that referenced
this pull request
Feb 11, 2022
asfgit
pushed a commit
that referenced
this pull request
Feb 11, 2022
(cherry picked from commit 0e74f1e)
Member
|
Thanks for the fix. Should we also make the encoder thread safe? |
Contributor
|
Not sure.... there's a reentrant lock inside AbstractNativeRawEncoder but it's the read lock that's held, not write lock, to improve concurrency (HADOOP-15499 I suppose data race problem is considered? |
Contributor
|
hey, please remember to keep the JIRA ID on the commit message. thanks. |
steveloughran
pushed a commit
to steveloughran/hadoop
that referenced
this pull request
Apr 5, 2022
(cherry picked from commit 0e74f1e)
steveloughran
pushed a commit
to steveloughran/hadoop
that referenced
this pull request
Apr 12, 2022
…apache#3881) (cherry picked from commit 0e74f1e) Change-Id: I8ad77ac5a0e967645c3c15844cb3b5dc4c30a89f
asfgit
pushed a commit
that referenced
this pull request
Apr 13, 2022
(cherry picked from commit 0e74f1e)
HarshitGupta11
pushed a commit
to HarshitGupta11/hadoop
that referenced
this pull request
Nov 28, 2022
jojochuang
pushed a commit
to jojochuang/hadoop
that referenced
this pull request
May 23, 2023
…rrent preads (apache#3881) (cherry picked from commit 0e74f1e) (cherry picked from commit 9071c96) (cherry picked from commit 6219247) Change-Id: I6aae0c9c8f29ed5b1746bd1e37e7880e9f1d4929
LiuGuH
pushed a commit
to LiuGuH/hadoop
that referenced
this pull request
Mar 26, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
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.
Reading data on an erasure-coded file with missing replicas(internal block of block group) will cause online reconstruction: read
dataUnitspart of data and decode them into the target missing data. EachDFSStripedInputStreamobject has aRawErasureDecoderobject, and when we doing pread concurrently,RawErasureDecoder.decodewill be invoked concurrently too.RawErasureDecoder.decodeis not thread safe, as a result of that we get wrong data from pread occasionally.