[ https://issues.apache.org/jira/browse/HDFS-11779?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16027511#comment-16027511 ]
Weiwei Yang commented on HDFS-11779: ------------------------------------ Hello [~xyao] and [~anu] V7 patch contains following changes bq. Nullable and Nonnull annotations Removed from v7 patch. bq. BucketManagerImpl.java: Check if this value is greater than 0 ? maxNumOfBuckets Add a check in {{DistributedStorageHandler#listBuckets}} to check make sure the {{maxKey}} value is in range (0, 1024), the max value defined in {{OzoneConsts#MAX_LISTBUCKETS_SIZE}}. bq. KeySpaceManager.java: catch (Exception ex) ==> should this be IOException Fixed. bq. 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. Removed {{getVolumeBucket}}, it is no longer needed in latest patch. bq. Would you not be able to collapse this into a single filter. Yes, I've done this in latest patch in a single filter. bq. DistributedStorageHandler.java Line 272: To avoid overflow, we might consider enforce paging by having a default MaxKeys per list request (say 100) if the MaxKeys is not specified in the request. If the caller really want to listAll, they can specify the MaxKeys with a negative value (like -1). Per Anu's comment, I have added a check to make sure a request can only retrieve 0 ~ 1024 number of buckets per request. End user is not able to retrieve all buckets once by providing a negative number. This should be enough for user and also safer. bq. ResultCodes.FAILED_VOLUME_NOT_FOUND ==> VOLUME_NOT_FOUND This is the only one comment I did not address in latest patch. Via HDFS-11769, {{KSMException}} was added to KSM to wrap up server side exceptions, and {{KeySpaceManagerProtocolServerSideTranslatorPB}} convert the exception into {{ListBucketsResponse}} which contains a {{Status}} info. Then {{KeySpaceManagerProtocolServerClientTranslatorPB}} parses the {{Status}} info and throws an IOException. If we want to change this manner, there is a lot of places to change. Looks like this needs some more work to get it consistent every where. Thanks > 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-unsubscr...@hadoop.apache.org For additional commands, e-mail: hdfs-issues-h...@hadoop.apache.org