[ https://issues.apache.org/jira/browse/HDDS-351?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16603428#comment-16603428 ]
Xiaoyu Yao commented on HDDS-351: --------------------------------- Thanks [~ajayydv] for working on this. The patch looks good to me overall. A few more comments: ContainerStateManger.java Line 512: this changes to return an empty set instead of throwing exception. From the two callers, I see ContainerManpping#getContainerWithPipeline has been updated to throw when an empty set is returned. But ContainerReportHandler#emitReplicationRequestEvent() is not. In the case of an empty set is returned, I think we should throw exception as well here as there is nothing to replicate here. HddsConfigKeys.java Line 67: NIT: suggest rename to HDDS_SCM_CHILLMODE_THRESHOLD_PCT "hdds.scm.chillmod.threshold.pct" Line 71: NIT: suggest rename HDDS_SCM_CHILLMODE_THRESHOLD_PCT_DEFAULT Ozone-default.xml Line 1110: the tag should be "HDDS, SCM, OPERATION" SCMChillModeManager.java Line 19-20: NIT: seems the license header are accidentally changed with extra line changes. Line 58: unused variable can be removed Line 61: can we define containerWithMinReplicas as AtomicLong and use AtomicLong#doubleValue() when doing the calculation in validate() at line 160. Line 59/69: we don't need to maintain a local copy of EventQueue. onMessage->validateChillModeExitRules->exitChillMode() can pass the unused EventPublisher. Line 71-72, 146-150: this can be optimized to reduce unnecessary map-list-map conversion. ContainerStateManager#getContainerMap can return a map directly. Also, can you clarify if this rule expects close container only from containerStateManager? Line 103: seems only CONT_EXIT_RULE get to process the registration report? But all the other rules are invoked to validate. Can you clarify with the expected usage for the ChillModeExitRule interface? ** TestStorageContainerManager.java Line 535: can we add some validation on the chill mode exit state after DN starts? Can we add more unit tests on the exit rules, e.g., the default percentage rule? > Add chill mode state to SCM > --------------------------- > > Key: HDDS-351 > URL: https://issues.apache.org/jira/browse/HDDS-351 > Project: Hadoop Distributed Data Store > Issue Type: Bug > Reporter: Ajay Kumar > Assignee: Ajay Kumar > Priority: Major > Fix For: 0.2.1 > > Attachments: HDDS-351.00.patch, HDDS-351.01.patch, HDDS-351.02.patch, > HDDS-351.03.patch, HDDS-351.04.patch > > > Add chill mode state to SCM -- This message was sent by Atlassian JIRA (v7.6.3#76005) --------------------------------------------------------------------- To unsubscribe, e-mail: hdfs-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: hdfs-issues-h...@hadoop.apache.org