[
https://issues.apache.org/jira/browse/HDFS-6735?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14071382#comment-14071382
]
stack commented on HDFS-6735:
-----------------------------
Nice test. We'd not check in the test since it does not assert anything? We'd
just check it in as a utility testing concurrent pread throughput?
On the below, it is 'safe' because the data member being queried if 'final'?
- synchronized boolean shortCircuitForbidden() {
+ boolean shortCircuitForbidden() {
return locatedBlocks.isUnderConstruction();
}
I'd think you'd want a comment at least in LocatedBlocks#underConstruction
warning an upper layer is dependent on it being final in case LocatedBlocks
changes and starts to allow blocks complete under a stream.
Looking at this change:
- pos = offset;
- blockEnd = blk.getStartOffset() + blk.getBlockSize() - 1;
- currentLocatedBlock = blk;
+ synchronized (this) {
+ pos = offset;
+ blockEnd = blk.getStartOffset() + blk.getBlockSize() - 1;
+ currentLocatedBlock = blk;
+ }
... it makes sense. Should the line from a few lines above:
locatedBlocks.insertRange(targetBlockIdx, newBlocks.getLocatedBlocks());
... be inside a synchronization too? Could two threads be updating block
locations at same time?
The below looks safe to me:
- private synchronized List<LocatedBlock> getFinalizedBlockRange(
+ private List<LocatedBlock> getFinalizedBlockRange(
Else patch looks great.
> A minor optimization to avoid pread() be blocked by read() inside the same
> DFSInputStream
> -----------------------------------------------------------------------------------------
>
> Key: HDFS-6735
> URL: https://issues.apache.org/jira/browse/HDFS-6735
> Project: Hadoop HDFS
> Issue Type: Improvement
> Components: hdfs-client
> Affects Versions: 3.0.0
> Reporter: Liang Xie
> Assignee: Liang Xie
> Attachments: HDFS-6735.txt
>
>
> In current DFSInputStream impl, there're a couple of coarser-grained locks in
> read/pread path, and it has became a HBase read latency pain point so far. In
> HDFS-6698, i made a minor patch against the first encourtered lock, around
> getFileLength, in deed, after reading code and testing, it shows still other
> locks we could improve.
> In this jira, i'll make a patch against other locks, and a simple test case
> to show the issue and the improved result.
> This is important for HBase application, since in current HFile read path, we
> issue all read()/pread() requests in the same DFSInputStream for one HFile.
> (Multi streams solution is another story i had a plan to do, but probably
> will take more time than i expected)
--
This message was sent by Atlassian JIRA
(v6.2#6252)
