hadoop-hdfs-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Mukul Kumar Singh (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (HDFS-11769) Ozone: KSM: Add createVolume API
Date Sat, 13 May 2017 18:57:04 GMT

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

Mukul Kumar Singh commented on HDFS-11769:

Thanks [~anu] and [~xyao] for the review.

Can we abstract VolumeManager as an interface and put the implementation into VolumeManagerImpl
for better testability? 
This way, KeySpaceManager, BucketManager and KeyManager,etc. can be easily developed/tested
with a mocked VolumeManager?
---> This is already done using KeyspaceManagerProtocol
Line 51: 
can you add some comments on KSM DB and the schema for volumes. Note the same 
DB will be used for bucket/key.
---> Will address this as part of another bug
Line 52: can you add some comments on the expected usage of the lock? Note it will 
be shared among VolumeManager/BucketManager/KeyManager for KSM DB read/write.
{{Will address this as part of another bug}}
We need to have a shutdown method to ensure the db is closed properly upon shutdown of KSM.
---> done
Line 82/92/111/117: 
Can we use DFSUtil.bytes2String/DFSUtil.string2Bytes to take care of string encoding/decoding?
---> done
Line 102: can we define the magic number 1024 in OzoneConsts?
---> done
Also, we should add a KSM Exception as you commented in TODO.
---> done
Line 118: we need to close the batch to avoid resource leak.
Can you add wrapper for createWriteBatch()/writeBatch()/closeBatch() for LevelDBStore and
modify the code to atomic update DB for volume creation using the wrappers like below:
---> done
The tests added look good to me. One suggestion: is it possible to abstract some common helpers
to dedup.
---> done
resp = rpcProxy.createVolume(NULL_RPC_CONTROLLER,
resp has a status field and if it is not OK, we should throw.
---> done
Not really part of this change, but in one of our earlier patches
we stopped using this string – distributed. 
if ("distributed".equalsIgnoreCase(shType))
---> this was solved when the workspace updated
File ksmDBFile = new File(ksmDBPath, KSM_DB_NAME);
Should create this path if this does not exist ?
---> done
options.createIfMissing(true); – we don't need this since this is the default option for
---> done
throw new IOException("Too many volumes for user:" + dbUserName);
dbUserName --> args.getOwnerName()
---> done
Thanks for fixing the comment in line 78. I think we should add a new param instead of modifying
the StorageContainerLocationProtocol param.
---> done

> Ozone: KSM: Add createVolume API
> --------------------------------
>                 Key: HDFS-11769
>                 URL: https://issues.apache.org/jira/browse/HDFS-11769
>             Project: Hadoop HDFS
>          Issue Type: Sub-task
>          Components: ozone
>    Affects Versions: HDFS-7240
>            Reporter: Anu Engineer
>            Assignee: Mukul Kumar Singh
>         Attachments: HDFS-11769-HDFS-7240.001.patch, HDFS-11769-HDFS-7240.002.patch
> Add createVolume API in KSM.
> createVolume API allows administrators to create a volume. The arguments to the API are:
> * Admin Name - The name of the administrator who is creating this volume. Volumes can
be created only by administrators.
> *  User Name - The name of the owner of this volume.
> * Quota - Quota information for this volume.
> This JIRA proposes to add the protobuf layer for createVolume and handling in KSM. We
will file a followup JIRA for the KSM client.

This message was sent by Atlassian JIRA

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

View raw message