hadoop-hdfs-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Weiwei Yang (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (HDFS-11679) Ozone: SCM CLI: Implement list container command
Date Wed, 24 May 2017 09:40:04 GMT

    [ 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


Mime
View raw message