[jira] [Commented] (HDDS-183) Integrate Volumeset, ContainerSet and HddsDispatcher

["Hanisha Koneru (JIRA)" <ji...@apache.org>]

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

Hanisha Koneru commented on HDDS-183:
-------------------------------------

Thanks for working on this [~bharatviswa].
A few comments:
* ContainerReader
** Line 106, 112 : if check redundant
** Line 160, 164 : Catch blocks can be combined as one.
** Line 147 : We should not cast to KeyValueContainerData
** verifyContainerFile() - If containerType does not match KeyValueContainer, we should log an error.
** parseKeyValueContainerData() can have void return type as the containerData is updated in place. 
    Also, can we move this function to KeyValueContainerUtils or some other KeyValue class. It would be good to keep the common implementations free of specific containerType code.
** ContainerFilter class is unused.
* ContainerDataYaml
** Line 84, 174 - new Tag("KeyValueContainerData”) should be defined as a static variable in KeyValueContainerData or KeyValueContainer class.
** Line 207 - state can be “INVALID” also.
** Line 152-157 : Instead of defining the valid fields here, can we maintain a list of valid yaml fields for KVContainer in KVContainerData. This way, if a new field is added to KVContainerData, we can update this list itself and avoid "When a new field needs to be added, it needs to be added here.” Also, these fields should be defined as static final varaibles.
** It would be good to move ConstructKeyValueContainerData class also to keyvalue package. 
* DeleteBlocksCommandHandler - we should handle different containerTypes here. For types other than KV, we can just log an error.
* VersionEndpointTask
** Line 70 : keyValues not used.
** Line 84 : ozoneContainer.getDispatcher().setScmId(scmId); should be outside the for loop for volumeMap.
* HddsVolume - changes are not required.
* VolumeSet - NIT - Line 358 : Unused variable srb
* KeyValueContainerUtil - NIT - Line 213 : check sum -> checksum, Container Name -> Container ID (just to avoid confusion for admins when debugging logs).
* KeyValueHandler
** We need the kvContainer.writeUnlock() in handleDeleteContainer(). When we detect that the container is open, we should immediately release the lock and not wait for any other operation (even loggging).


> Integrate Volumeset, ContainerSet and HddsDispatcher
> ----------------------------------------------------
>
>                 Key: HDDS-183
>                 URL: https://issues.apache.org/jira/browse/HDDS-183
>             Project: Hadoop Distributed Data Store
>          Issue Type: Sub-task
>            Reporter: Bharat Viswanadham
>            Assignee: Bharat Viswanadham
>            Priority: Major
>             Fix For: 0.2.1
>
>         Attachments: HDDS-183-HDDS-48.00.patch, HDDS-183-HDDS-48.01.patch, HDDS-183-HDDS-48.02.patch, HDDS-183-HDDS-48.03.patch
>
>
> This Jira adds following:
> 1. Use new VolumeSet.
> 2. build container map from .container files during startup.
> 3. Integrate HddsDispatcher.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

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