Uma Maheswara Rao G commented on HDFS-13110:

Thank you [~rakeshr]

Please find my comments:

+      String nextSPSPathId = impl.getNextSPSPathId();{code}
I think we can return path instead of string id? Because having long into 
string may be odd. Because mostly we have to use that as long again. How about 
we return string path itself. External scanner will use that path directly. 
Internal scanner can use getInode(src) to get Inode and do scanning. This will 
make things clean.


public class ItemInfo<T> {
  private T startId;
  private T fileId;

We could rename StartID to spsPath and fileId to file ?

 3. getNextSPSPathId() —> getNextSPSPath()

 4. In SPS class:

//TODO Add join here on SPS rpc server also

Could you please remove this TODO? 

5. AttemptedItemInfo <T> itemInfo = iter.next();

Please format this line


   * Gets the block collection id for which storage movements check necessary
   * and make the movement if required.
   * @return block collection info
  public synchronized ItemInfo<T> get() {

Please update here that it gets the file to satisfy storage.


// Some of the blocks are low redundant, so marking the status as
+    if (hasLowRedundancyBlocks) {
+      status = BlocksMovingAnalysis.Status.FEW_LOW_REDUNDANCY_BLOCKS;
+    }

I think if some blocks paired successfully, then we could mark few blocks 
faired. If no blocks paired successfully and block low redundancy then only 
state can be BlocksMovingAnalysis.Status.FEW_LOW_REDUNDANCY_BLOCKS

> [SPS]: Reduce the number of APIs in NamenodeProtocol used by external 
> satisfier
> -------------------------------------------------------------------------------
>                 Key: HDFS-13110
>                 URL: https://issues.apache.org/jira/browse/HDFS-13110
>             Project: Hadoop HDFS
>          Issue Type: Sub-task
>            Reporter: Rakesh R
>            Assignee: Rakesh R
>            Priority: Major
>         Attachments: HDFS-13110-HDFS-10285-00.patch, 
> HDFS-13110-HDFS-10285-01.patch, HDFS-13110-HDFS-10285-02.patch, 
> HDFS-13110-HDFS-10285-03.patch
> This task is to address the following [~daryn]'s comments. Please refer 
> HDFS-10285 to see more detailed discussion.
> *Comment-10)*
> {quote}
> NamenodeProtocolTranslatorPB
> Most of the api changes appear unnecessary.
> IntraSPSNameNodeContext#getFileInfo swallows all IOEs, based on assumption 
> that any and all IOEs means FNF which probably isn’t the intention during rpc 
> exceptions.
> {quote}
>  *Comment-13)*
> {quote}
> StoragePolicySatisfier
>  It appears to make back-to-back calls to hasLowRedundancyBlocks and 
> getFileInfo for every file. Haven’t fully groked the code, but if low 
> redundancy is not the common case, then it shouldn’t be called unless/until 
> needed. It looks like files that are under replicated are re-queued again?
> {quote}

This message was sent by Atlassian JIRA

To unsubscribe, e-mail: hdfs-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: hdfs-issues-h...@hadoop.apache.org

Reply via email to