[
https://issues.apache.org/jira/browse/HDFS-2246?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13103205#comment-13103205
]
Todd Lipcon commented on HDFS-2246:
-----------------------------------
Regarding the most recent patch:
{code}
+ (targetAddr.getAddress().equals(localHost) ||
+ targetAddr.getHostName().startsWith("localhost"))) {
{code}
Is it possible that getHostName here will incur a reverse DNS lookup and be
problematic?
{code}
+ // InvalidBlockTokenException is only thrown when the data pipeline
+ // returns an error code. For local block reads, if the token has
+ // expired we will get only a generic RemoteException from the RPC
+ // to get the local path, and this requires the unfortunate hack
+ // below.
+ boolean isInvalidBlock = (ex instanceof InvalidBlockTokenException)
+ || (ex.getMessage() != null &&
+ ex.getMessage().endsWith("is expired."));
{code}
Can you refactor this repeated code and comment into a new function like
{{boolean isExpiredBlockTokenException(Throwable t)}}?
{code}
+ if (shortCircuitLocalReads && localHost != null &&
+ (targetAddr.getAddress().equals(localHost) ||
+ targetAddr.getHostName().startsWith("localhost"))) {
{code}
Refactor the above repeated code into a function like {{boolean
shouldTryLocalRead(InetAddress addr)}}
- The patch seems to change a number of logs from WARN level to INFO level.
That seems like a separate change.
{code}
+ /** Retrives the filename of the blockfile and the metafile from the datanode
{code}
Typo "retrieves"
{code}
+ if (isBlockTokenEnabled && UserGroupInformation.isSecurityEnabled()) {
+ Set<TokenIdentifier> tokenIds = UserGroupInformation.getCurrentUser()
+ .getTokenIdentifiers();
+ if (tokenIds.size() != 1) {
+ throw new IOException("Can't continue with getBlockPathInfo() "
+ + "authorization since " + tokenIds.size() + "
BlockTokenIdentifier "
+ + "is found.");
+ }
+ for (TokenIdentifier tokenId : tokenIds) {
+ BlockTokenIdentifier id = (BlockTokenIdentifier) tokenId;
+ if (LOG.isDebugEnabled()) {
+ LOG.debug("Got: " + id.toString());
+ }
+ blockTokenSecretManager.checkAccess(id, null, block,
+ BlockTokenSecretManager.AccessMode.READ);
+ }
+ }
{code}
This is all repeated code from {{recoverBlock}} - please refactor into a
{{checkRpcAuthorizedForBlock()}} function
{code}
+/**
+ * This class tests that a file need not be closed before its
+ * data can be read by another client.
+ */
+public class TestFileLocalRead extends junit.framework.TestCase {
{code}
This comment was copy-pasted from a different test case. Also, should use JUnit
4 for new tests.
> Shortcut a local client reads to a Datanodes files directly
> -----------------------------------------------------------
>
> Key: HDFS-2246
> URL: https://issues.apache.org/jira/browse/HDFS-2246
> Project: Hadoop HDFS
> Issue Type: Improvement
> Reporter: Sanjay Radia
> Attachments: 0001-HDFS-347.-Local-reads.patch,
> localReadShortcut20-security.2patch
>
>
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
