Mailing-List: contact hdfs-issues-help@hadoop.apache.org; run by ezmlm
Precedence: bulk
Date: Wed, 24 May 2017 09:40:04 +0000 (UTC)
From: "Weiwei Yang (JIRA)" <jira@apache.org>
To: hdfs-issues@hadoop.apache.org
Message-ID: <JIRA.13065103.1492609827000.281239.1495618804367@Atlassian.JIRA>
In-Reply-To: <JIRA.13065103.1492609827000@Atlassian.JIRA>
References: <JIRA.13065103.1492609827000@Atlassian.JIRA> <JIRA.13065103.1492609827241@jira-lw-us.apache.org>
Subject: [jira] [Commented] (HDFS-11679) Ozone: SCM CLI: Implement list
 container command
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit
archived-at: Wed, 24 May 2017 09:40:10 -0000


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

Weiwei Yang commented on HDFS-11679:
------------------------------------

Hi [~yuanbo]

Thanks for working on this. Here some initial comments

*ListContainerHandler.java*

There is a findbugs warning, please fix that.

*ContainerMapping.java*

# The way to get a range of keys in levelDB seems inappropriate, instead of scanning all the keys in DB, can we call API {{iter.seek(byte[] key)}} and {{iter.forEachRemaining}} to get the range of keys?
# Can we wrap up the code to get a range of keys into {{LevelDBStore}}? Wrap the common code in this class so other places can reuse. (When this code is added, please also add UT to make sure it behaves correctly).
# Please make sure the DB iterator is closed after finishing the operation.
# Two methods {{listContainer(String startName, int count)}} and {{public List<Pipeline> listContainer(String startName, String endName) }} have too many similar code, can reduce the dup code as much as possible?

*Mapping.java*

Can we merge the 2 APIs into a single one?

{code}
List<Pipeline> listContainer(String startName, String endName, int count)
{code}

*StorageContainerLocationProtocolClientSideTranslatorPB*

# Minor: line 43, 44 unused imports
# line 192: it says "Start container name cannot be null", I thought this argument could be optional, in that case we print all the containers in SCM. This is useful when user firstly runs the command, the container names are unknown to them, so they just simply give (null, null, 100) to print 100 containers from beginning. Does that make sense?
# line 197 - 204: you cannot use if-else clause to parse the argument. If endName is not null, rest of code will not run.

*StorageContainerLocationProtocolServerSideTranslatorPB*

# Same comment to the argument check. I think you don't need to have the check twice on client side as well as the server side. Just make sure server side checks everything correctly should be fine.

Thanks

> Ozone: SCM CLI: Implement list container command
> ------------------------------------------------
>
>                 Key: HDFS-11679
>                 URL: https://issues.apache.org/jira/browse/HDFS-11679
>             Project: Hadoop HDFS
>          Issue Type: Sub-task
>            Reporter: Weiwei Yang
>            Assignee: Yuanbo Liu
>              Labels: command-line
>         Attachments: HDFS-11679-HDFS-7240.001.patch
>
>
> Implement the command to list containers
> {code}
> hdfs scm -container list -start <container name> [-count <100> | -end <name>]{code}
> Lists all containers known to SCM. The option -start allows the listing to start from a specified container and -count controls the number of entries returned but it is mutually exclusive with the -end option which returns keys from the -start to -end range.


--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

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