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

Jing Zhao commented on HDFS-8447:
---------------------------------

# Let's add a comment here explaining the intention of the overflow check. We
  can also add extra check to make sure start and length are not negative, or
  mention the check has been done in the function's caller.
{code}
+    long end = start + length < 0 ? Long.MAX_VALUE : start + length;
{code}
# The following code with the current variable name is not easy to follow. How
  about renaming start -> offset, old -> blockStart, and pos -> blockEnd? We
  can also move the if condition into a separate function. Some javadoc
  explaining the logic can also be helpful.
{code}
+      if ((old < end && pos > start) ||
+          (length == 0 && old <= start && start < pos) ||
+          (b.getNumBytes() == 0 && start <= old && pos < end)) {
+        lbs.add(createLocatedBlock(b, old));
+      } else if (old >= end && old >= visibleFileLength) {
+        break;
+      }
{code}
# Looks like with the following change the AccessMode.WRITE is never assigned?
{code}
-    return createLocatedBlock(ucBlock, pos, 
BlockTokenIdentifier.AccessMode.WRITE);
+    return createLocatedBlock(ucBlock, pos);
{code}
# Actually the following logic is very tricky and with the new code semantic is
  harder to follow. We'd better use some example in the javadoc to explain the
  logic here.
{code}
+   * @return
+   * the list of blocks that cover the range of
+   * [start, start + length) plus the last block that
+   * does not go beyond visibleFileLength.
{code}
{code}
+      } else if (old >= end && old >= visibleFileLength) {
+        break;
+      }
{code}


> Decouple information of files in GetLocatedBlocks
> -------------------------------------------------
>
>                 Key: HDFS-8447
>                 URL: https://issues.apache.org/jira/browse/HDFS-8447
>             Project: Hadoop HDFS
>          Issue Type: Improvement
>            Reporter: Haohui Mai
>            Assignee: Haohui Mai
>         Attachments: HDFS-8447.000.patch, HDFS-8447.001.patch, 
> HDFS-8447.002.patch, HDFS-8447.003.patch, HDFS-8447.004.patch, 
> HDFS-8447.005.patch
>
>
> The current implementation of {{BlockManager.getLocatedBlocks()}} requires 
> the information of files to be passed as parameters. These information does 
> not affect the results of getting the physical locations of blocks.
> This jira proposes to refactor the call so that 
> {{BlockManager.getLocatedBlocks()}} depends only on the block information.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Reply via email to