hadoop-hdfs-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Todd Lipcon (JIRA)" <j...@apache.org>
Subject [jira] Commented: (HDFS-941) Datanode xceiver protocol should allow reuse of a connection
Date Thu, 18 Mar 2010 21:20:27 GMT

    [ https://issues.apache.org/jira/browse/HDFS-941?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12847117#action_12847117
] 

Todd Lipcon commented on HDFS-941:
----------------------------------

Style notes:

- in BlockReader:
{code}
+      LOG.warn("Could not write to datanode " + sock.getInetAddress() +
+               ": " + e.getMessage());
{code}
should be more specific - like "Could not write read result status code" and also indicate
in the warning somehow that this is not a critical problem. Perhaps "info" level is better?
(in my experience if people see WARN they think something is seriously wrong)

- please move the inner SocketCacheEntry class down lower in DFSInputStream
- in SocketCacheEntry.setOwner, can you use IOUtils.closeStream to close reader? Similarly
in SocketCacheEntry.close
- We expect the following may happen reasonably often, right?
{code}
+        // Our socket is no good.
+        DFSClient.LOG.warn("Error making BlockReader. Closing stale " + entry.sock.toString());
{code}
I think this should probably be debug level.

- The edits to the docs in DataNode.java are good - if possible they should probably move
into HDFS-1001 though, no?

- the do { ... } while () loop is a bit hard to follow in DataXceiver. Would it be possible
to rearrange the code a bit to be more linear? (eg setting DN_KEEPALIVE_TIMEOUT right before
the read at the beginning of the loop if workDone > 0 would be easier to follow in my opinion)

- In DataXceiver:
{code}
+      } catch (IOException ioe) {
+        LOG.error("Error reading client status response. Will close connection. Err: " +
ioe);
{code}
Doesn't this yield error messages on every incomplete client read? Since the response is optional,
this seems more like a DEBUG.

Bigger stuff:

- I think there is a concurrency issue here. Namely, the positional read API calls through
into fetchBlockByteRange, which will use the existing cached socket, regardless of other concurrent
operations. So we may end up with multiple block readers on the same socket and everything
will fall apart.

Can you add a test case which tests concurrent use of a DFSInputStream? Maybe a few threads
doing random positional reads while another thread does seeks and sequential reads?

- Regarding the cache size of one - I don't think this is quite true. For a use case like
HBase, the region server is continually slamming the local datanode with random read requests
from several client threads. Is the idea that such an application should be using multiple
DFSInputStreams to read the same file and handle the multithreading itself?

- In DataXceiver, SocketException is caught and ignored while sending a block. ("// Its ok
for remote side to close the connection anytime." I think there are other SocketException
types (eg timeout) that could throw here aside from a connection close, so in that case we
need to IOUtils.closeStream(out) I believe. A test case for this could be to open a BlockReader,
read some bytes, then stop reading so that the other side's BlockSender generates a timeout.


- Not sure about this removal in the finally clause of opWriteBlock:
{code}
-      IOUtils.closeStream(replyOut);
{code}
(a) We still need to close in the case of an downstream-generated exception. Otherwise we'll
read the next data bytes from the writer as an operation and have undefined results.
(b) To keep this patch less dangerous, maybe we should not add the reuse feature for operations
other than read? Read's the only operation where we expect a lot of very short requests coming
in - not much benefit for writes, etc, plus they're more complicated.

> Datanode xceiver protocol should allow reuse of a connection
> ------------------------------------------------------------
>
>                 Key: HDFS-941
>                 URL: https://issues.apache.org/jira/browse/HDFS-941
>             Project: Hadoop HDFS
>          Issue Type: Improvement
>          Components: data-node, hdfs client
>    Affects Versions: 0.22.0
>            Reporter: Todd Lipcon
>            Assignee: bc Wong
>         Attachments: HDFS-941-1.patch
>
>
> Right now each connection into the datanode xceiver only processes one operation.
> In the case that an operation leaves the stream in a well-defined state (eg a client
reads to the end of a block successfully) the same connection could be reused for a second
operation. This should improve random read performance significantly.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


Mime
View raw message