Mailing-List: contact hdfs-issues-help@hadoop.apache.org; run by ezmlm
Precedence: bulk
Date: Fri, 26 May 2017 17:19:04 +0000 (UTC)
From: "Anu Engineer (JIRA)" <jira@apache.org>
To: hdfs-issues@hadoop.apache.org
Message-ID: <JIRA.13070275.1494307267000.304110.1495819144545@Atlassian.JIRA>
In-Reply-To: <JIRA.13070275.1494307267000@Atlassian.JIRA>
References: <JIRA.13070275.1494307267000@Atlassian.JIRA> <JIRA.13070275.1494307267825@jira-lw-us.apache.org>
Subject: [jira] [Commented] (HDFS-11779) Ozone: KSM: add listBuckets
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit
archived-at: Fri, 26 May 2017 17:19:11 -0000


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

Anu Engineer commented on HDFS-11779:
-------------------------------------

[~cheersyang] Thanks for the patch. My comments are based on the earlier patch, but I thought they are still relevant. Some very minor comments.

* {{KeySpaceManagerProtocol.java}} and few other files.
{code}
import javax.annotation.Nullable;
import javax.annotation.Nonnull;
{code}
Personally I like these annotations, but javax is traditionally a namespace that is used for UI. While I have no better suggestion (I know there are other projects which can provide this), would you 
please reconsider if we need to add these ? 


* {{BucketManagerImpl.java}}
Check if this value is greater than 0 ?  _maxNumOfBuckets_ 

* {{KeySpaceManager.java}}
 catch (Exception ex) ==> should this be IOException

 * {{MetadataManagerImpl.java}} 
Just a meta-comment -- {{getVolumeBucket}} I don't want to change this code in this patch, but I would 
 like to formalize all these parsing assumptions in one single class -- so this info is localized in one file.

* {{ResultCodes.FAILED_VOLUME_NOT_FOUND}} ==> VOLUME_NOT_FOUND


* Please forgive me if my understanding is not correct. Would you not be able to collapse this into a single 
filter -- since /volume/prefix -- must match -- and  both levelDB and RocksDB would do that match ? 
{code}
246	    LevelDBKeyFilter bucketPrefixFilter = key -> {
247	      String[] volumeBucket = getVolumeBucket(key);
248	      return bucketPrefix == null ? true :
249	          volumeBucket[1].startsWith(bucketPrefix);
250	    };
251	    LevelDBKeyFilter volumeNameFilter = key -> {
252	      String[] volumeBucket = getVolumeBucket(key);
253	      return volumeName.equals(volumeBucket[0]);
254	    };
{code}

> Ozone: KSM: add listBuckets
> ---------------------------
>
>                 Key: HDFS-11779
>                 URL: https://issues.apache.org/jira/browse/HDFS-11779
>             Project: Hadoop HDFS
>          Issue Type: Sub-task
>          Components: ozone
>    Affects Versions: HDFS-7240
>            Reporter: Anu Engineer
>            Assignee: Weiwei Yang
>         Attachments: HDFS-11779-HDFS-7240.001.patch, HDFS-11779-HDFS-7240.002.patch, HDFS-11779-HDFS-7240.003.patch, HDFS-11779-HDFS-7240.004.patch, HDFS-11779-HDFS-7240.005.patch, HDFS-11779-HDFS-7240.006.patch
>
>
> Lists buckets of a given volume. Similar to listVolumes, paging supported via prevKey, prefix and maxKeys.


--
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