hadoop-hdfs-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Xiaoyu Yao (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (HDFS-11769) Ozone: KSM: Add createVolume API
Date Wed, 10 May 2017 22:38:04 GMT

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

Xiaoyu Yao commented on HDFS-11769:
-----------------------------------

Thanks [~msingh] for working on this. The patch looks good to me overall. It has some overlap
with HDFS-11768 due dependency. That should be cleared out when HDFS-11768 is in. 

Below are some earlier feedback:

*VolumeManager.java*

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?

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.

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.

We need to have a shutdown method to ensure the db is closed properly upon shutdown of KSM.

Line 82/92/111/117: 
Can we use DFSUtil.bytes2String/DFSUtil.string2Bytes to take care of string encoding/decoding?

Line 102: can we define the magic number 1024 in OzoneConsts?
Also, we should add a KSM Exception as you commented in TODO.

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:
{code} 
  WriteBatch batch = store.createWriteBatch();
  try {
      batch.put(...);
      batch.put(...);
      store.writeBatch(batch);
  } finally {
      store.closeBatch(batch);
  }
 {code}

*TestDistributedOzoneVolumes.java*

The tests added look good to me. One suggestion: is it possible to abstract some common helpers
to dedup. 

> 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
>
>
> 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: hdfs-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: hdfs-issues-help@hadoop.apache.org


Mime
View raw message