pkuwm commented on a change in pull request #557: Add instance capacity gauge
URL: https://github.com/apache/helix/pull/557#discussion_r346576251
##########
File path:
helix-core/src/main/java/org/apache/helix/monitoring/mbeans/dynamicMBeans/DynamicMBeanProvider.java
##########
@@ -103,25 +100,30 @@ protected synchronized boolean
doRegister(Collection<DynamicMetric<?, ?>> dynami
* @param description description of the MBean
* @param dynamicMetrics the DynamicMetrics
*/
- private void updateAttributtInfos(Collection<DynamicMetric<?, ?>>
dynamicMetrics,
+ protected void updateAttributesInfo(Collection<DynamicMetric<?, ?>>
dynamicMetrics,
String description) {
- _attributeMap.clear();
+ if (dynamicMetrics == null || dynamicMetrics.isEmpty()) {
Review comment:
Using an empty collection to remove the metrics is kind of hacky way to me.
Let's confirm:
1. Do we allow null or empty to delete metrics? The original code allows
this.
2. If we don't allow null to delete metrics, we will do nothing if it is
null. We allow empty collection to delete metrics.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscr...@helix.apache.org
For additional commands, e-mail: reviews-h...@helix.apache.org
