[
https://issues.apache.org/jira/browse/HDDS-123?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16495567#comment-16495567
]
Mukul Kumar Singh commented on HDDS-123:
----------------------------------------
Thanks for working on this [~bharatviswa]. The patch looks really good to me.
Please find my comments as following.
1) ContainerSet:55. The log line can be replaced with " private static final
Logger LOG = LoggerFactory.getLogger(ContainerSet.class);
"
2) ContainerSet:105. Lets assert that the entry is present. i.e. lets assert
that a non null entry is removed.
3) ContainerSet:212-220. most of the calls here start with
"container.getContainerData()" Can we cache this and re-use everywhere.
4) ContainerSet:154, the check in the javadoc should be > in place of ">=".
5) KeyValueContainer:52. this new line is not required.
6) TestContainerSet:28, please expand the wildcards.
7) TestContainerSet:56, and other place. Same comment as 3),
"container.getContainerData()" can be cached.
> ContainerSet class to manage ContainerMap
> ------------------------------------------
>
> Key: HDDS-123
> URL: https://issues.apache.org/jira/browse/HDDS-123
> Project: Hadoop Distributed Data Store
> Issue Type: Sub-task
> Reporter: Bharat Viswanadham
> Assignee: Bharat Viswanadham
> Priority: Major
> Fix For: 0.2.1
>
> Attachments: HDDS-123-HDDS-48.00.patch
>
>
> Create a ContainerSet class, which manages containerMap.
> Previously container map is in ContainerManagerImpl, with refactoring work it
> should be moved to ContainerSet.
> This class should handle add/get/remove container from containerMap.
> And also now it should handle containerReport.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
