[
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
TestDistributedOzoneVolumes.java
The tests added look good to me. One suggestion: is it possible to abstract
some common helpers to dedup.
---> done
KeySpaceManagerProtocolClientSideTranslatorPB.java
resp = rpcProxy.createVolume(NULL_RPC_CONTROLLER,
req.build());
resp has a status field and if it is not OK, we should throw.
---> done
ObjectStoreHandler.java
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
VolumeManager.java#VolumeManager
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 levelDB.
---> done
VolumeManager.java#createVolume
throw new IOException("Too many volumes for user:" + dbUserName);
dbUserName --> args.getOwnerName()
---> done
DistributedStorageHandler.java
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
(v6.3.15#6346)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
