jiajunwang commented on a change in pull request #1187:
URL: https://github.com/apache/helix/pull/1187#discussion_r470346498
##########
File path:
helix-core/src/main/java/org/apache/helix/controller/stages/CustomizedViewAggregationStage.java
##########
@@ -152,6 +162,68 @@ private void computeCustomizedStateView(final Resource
resource, final String st
if (curCustomizedView == null ||
!curCustomizedView.getRecord().equals(view.getRecord())) {
// Add customized view to the list which will be written to ZK later.
updatedCustomizedViews.add(view);
+ updatedStartTimestamps.put(resourceName,
+ customizedStateOutput.getResourceStartTimeMap(stateType,
resourceName));
+ }
+ }
+
+ private void reportLatency(ClusterEvent event, List<CustomizedView>
updatedCustomizedViews,
+ Map<String, CustomizedView> curCustomizedViews,
+ Map<String, Map<Partition, Map<String, Long>>> updatedStartTimestamps,
+ boolean[] updateSuccess) {
+ ClusterStatusMonitor clusterStatusMonitor =
+ event.getAttribute(AttributeName.clusterStatusMonitor.name());
+ if (clusterStatusMonitor == null) {
+ return;
+ }
+ long curTime = System.currentTimeMillis();
+ String clusterName = event.getClusterName();
+ CustomizedViewMonitor customizedViewMonitor =
+ clusterStatusMonitor.getOrCreateCustomizedViewMonitor(clusterName);
+
+ for (int i = 0; i < updatedCustomizedViews.size(); i++) {
+ CustomizedView newCV = updatedCustomizedViews.get(i);
+ String resourceName = newCV.getResourceName();
+
+ if (!updateSuccess[i]) {
+ LOG.warn("Customized views are not updated successfully for resource
{} on cluster {}",
+ resourceName, clusterName);
+ continue;
+ }
+
+ CustomizedView oldCV =
Review comment:
If the resource is newly created, will the oldCV contain the object?
##########
File path:
helix-core/src/main/java/org/apache/helix/controller/stages/CustomizedStateOutput.java
##########
@@ -65,6 +70,11 @@ public void setCustomizedState(String stateType, String
resourceName, Partition
return Collections.emptyMap();
}
+ public Map<String, Map<Partition, Map<String, Long>>> getStartTimeMap(String
stateType) {
Review comment:
Seems no other callers, make it private? In that case, no need to make
it unmodifiableMap
##########
File path:
helix-core/src/main/java/org/apache/helix/controller/stages/CustomizedViewAggregationStage.java
##########
@@ -152,6 +162,68 @@ private void computeCustomizedStateView(final Resource
resource, final String st
if (curCustomizedView == null ||
!curCustomizedView.getRecord().equals(view.getRecord())) {
// Add customized view to the list which will be written to ZK later.
updatedCustomizedViews.add(view);
+ updatedStartTimestamps.put(resourceName,
+ customizedStateOutput.getResourceStartTimeMap(stateType,
resourceName));
+ }
+ }
+
+ private void reportLatency(ClusterEvent event, List<CustomizedView>
updatedCustomizedViews,
+ Map<String, CustomizedView> curCustomizedViews,
+ Map<String, Map<Partition, Map<String, Long>>> updatedStartTimestamps,
+ boolean[] updateSuccess) {
+ ClusterStatusMonitor clusterStatusMonitor =
+ event.getAttribute(AttributeName.clusterStatusMonitor.name());
+ if (clusterStatusMonitor == null) {
+ return;
+ }
+ long curTime = System.currentTimeMillis();
+ String clusterName = event.getClusterName();
+ CustomizedViewMonitor customizedViewMonitor =
+ clusterStatusMonitor.getOrCreateCustomizedViewMonitor(clusterName);
+
+ for (int i = 0; i < updatedCustomizedViews.size(); i++) {
+ CustomizedView newCV = updatedCustomizedViews.get(i);
+ String resourceName = newCV.getResourceName();
+
+ if (!updateSuccess[i]) {
+ LOG.warn("Customized views are not updated successfully for resource
{} on cluster {}",
+ resourceName, clusterName);
+ continue;
+ }
+
+ CustomizedView oldCV =
Review comment:
On the other hand, if the resource is removed, then this loop won't
check the latency of that resource? But I think this is OK, since there is no
start time to track anyway.
##########
File path:
helix-core/src/main/java/org/apache/helix/monitoring/mbeans/CustomizedViewMonitor.java
##########
@@ -0,0 +1,85 @@
+package org.apache.helix.monitoring.mbeans;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.concurrent.TimeUnit;
+import javax.management.JMException;
+import javax.management.MalformedObjectNameException;
+import javax.management.ObjectName;
+
+import com.codahale.metrics.Histogram;
+import com.codahale.metrics.SlidingTimeWindowArrayReservoir;
+import org.apache.helix.monitoring.mbeans.dynamicMBeans.DynamicMBeanProvider;
+import org.apache.helix.monitoring.mbeans.dynamicMBeans.DynamicMetric;
+import org.apache.helix.monitoring.mbeans.dynamicMBeans.HistogramDynamicMetric;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class CustomizedViewMonitor extends DynamicMBeanProvider {
+ private static final Logger LOG =
LoggerFactory.getLogger(CustomizedViewMonitor.class);
+
+ private static final String MBEAN_DESCRIPTION = "Helix Customized View
Aggregation Monitor";
+ private final String _clusterName;
+ private final String _sensorName;
+ private HistogramDynamicMetric _updateToAggregationLatencyGauge;
+ public static final String UPDATE_TO_AGGREGATION_LATENCY_GAUGE =
+ "UpdateToAggregationLatencyGauge";
+ private ClusterStatusMonitor _clusterStatusMonitor;
+ private static final String TYPE_KEY = "Type";
+ private static final String CLUSTER_KEY = "Cluster";
+ private static final String CUSTOMIZED_VIEW = "CustomizedView";
+
+ public CustomizedViewMonitor(String clusterName) {
+ _clusterName = clusterName;
+ _sensorName = String
+ .format("%s:%s=%s,%s=%s", MonitorDomainNames.AggregatedView.name(),
TYPE_KEY,
+ CUSTOMIZED_VIEW, CLUSTER_KEY, _clusterName);
+ _updateToAggregationLatencyGauge =
+ new HistogramDynamicMetric(UPDATE_TO_AGGREGATION_LATENCY_GAUGE, new
Histogram(
+ new SlidingTimeWindowArrayReservoir(getResetIntervalInMs(),
TimeUnit.MILLISECONDS)));
+ }
+
+ @Override
+ public DynamicMBeanProvider register() throws JMException {
+ List<DynamicMetric<?, ?>> attributeList = new ArrayList<>();
+ attributeList.add(_updateToAggregationLatencyGauge);
+ doRegister(attributeList, MBEAN_DESCRIPTION, getMBeanObjectName());
+ return this;
+ }
+
+ private ObjectName getMBeanObjectName() throws MalformedObjectNameException {
+ return new ObjectName(String
+ .format("%s:%s=%s,%s=%s", MonitorDomainNames.AggregatedView.name(),
TYPE_KEY,
+ CUSTOMIZED_VIEW, CLUSTER_KEY, _clusterName));
+ }
+
+ @Override
+ public String getSensorName() {
+ return _sensorName;
+ }
+
+ public void recordUpdateToAggregationLatency(long latency) {
+ if (_updateToAggregationLatencyGauge != null) {
Review comment:
nit, could it be null?
----------------------------------------------------------------
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:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
