[GitHub] [hadoop] slfan1989 commented on a diff in pull request #4738: YARN-11250. Capture the Performance Metrics of ZookeeperFederationStateStore.
slfan1989 commented on code in PR #4738:
URL: https://github.com/apache/hadoop/pull/4738#discussion_r949069282
##
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common/src/main/java/org/apache/hadoop/yarn/server/federation/store/impl/ZKFederationStateStoreOpDurations.java:
##
@@ -0,0 +1,155 @@
+/**
+ * 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.
+ */
+package org.apache.hadoop.yarn.server.federation.store.impl;
+
+import org.apache.hadoop.classification.InterfaceAudience;
+import org.apache.hadoop.classification.InterfaceStability;
+import org.apache.hadoop.metrics2.MetricsCollector;
+import org.apache.hadoop.metrics2.MetricsInfo;
+import org.apache.hadoop.metrics2.MetricsSource;
+import org.apache.hadoop.metrics2.MetricsSystem;
+import org.apache.hadoop.metrics2.annotation.Metric;
+import org.apache.hadoop.metrics2.annotation.Metrics;
+import org.apache.hadoop.metrics2.lib.DefaultMetricsSystem;
+import org.apache.hadoop.metrics2.lib.MetricsRegistry;
+import org.apache.hadoop.metrics2.lib.MutableRate;
+
+import static org.apache.hadoop.metrics2.lib.Interns.info;
+
+@InterfaceAudience.Private
+@InterfaceStability.Unstable
+@Metrics(context="ZKFederationStateStore-op-durations")
+public final class ZKFederationStateStoreOpDurations implements MetricsSource {
+
+ @Metric("Duration for a add application homeSubcluster call")
+ private MutableRate addAppHomeSubCluster;
+
+ @Metric("Duration for a update application homeSubcluster call")
+ private MutableRate updateAppHomeSubCluster;
+
+ @Metric("Duration for a get application homeSubcluster call")
+ private MutableRate getAppHomeSubCluster;
+
+ @Metric("Duration for a get applications homeSubcluster call")
+ private MutableRate getAppsHomeSubCluster;
+
+ @Metric("Duration for a delete applications homeSubcluster call")
+ private MutableRate deleteAppHomeSubCluster;
+
+ @Metric("Duration for a register subCluster call")
+ private MutableRate registerSubCluster;
+
+ @Metric("Duration for a deregister subCluster call")
+ private MutableRate deregisterSubCluster;
+
+ @Metric("Duration for a subCluster Heartbeat call")
+ private MutableRate subClusterHeartbeat;
+
+ @Metric("Duration for a get SubCluster call")
+ private MutableRate getSubCluster;
+
+ @Metric("Duration for a get SubClusters call")
+ private MutableRate getSubClusters;
+
+ @Metric("Duration for a get PolicyConfiguration call")
+ private MutableRate getPolicyConfiguration;
+
+ @Metric("Duration for a set PolicyConfiguration call")
+ private MutableRate setPolicyConfiguration;
+
+ @Metric("Duration for a get PolicyConfigurations call")
+ private MutableRate getPoliciesConfigurations;
+
+ protected static final MetricsInfo RECORD_INFO =
+ info("ZKFederationStateStoreOpDurations", "Durations of
ZKFederationStateStore calls");
+
+ private final MetricsRegistry registry;
+
+ private static final ZKFederationStateStoreOpDurations INSTANCE =
+ new ZKFederationStateStoreOpDurations();
+
+ public static ZKFederationStateStoreOpDurations getInstance() {
+return INSTANCE;
+ }
+
+ private ZKFederationStateStoreOpDurations() {
+registry = new MetricsRegistry(RECORD_INFO);
+registry.tag(RECORD_INFO, "ZKFederationStateStoreOpDurations");
+
+MetricsSystem ms = DefaultMetricsSystem.instance();
+if (ms != null) {
+ ms.register(RECORD_INFO.name(), RECORD_INFO.description(), this);
+}
+ }
+
+ @Override
+ public synchronized void getMetrics(MetricsCollector collector, boolean all)
{
+registry.snapshot(collector.addRecord(registry.info()), all);
+ }
+
+ public void addAppHomeSubClusterDuration(long endTime, long startTime) {
+addAppHomeSubCluster.add(endTime - startTime);
+ }
+
+ public void addUpdateAppHomeSubClusterDuration(long endTime, long startTime)
{
+updateAppHomeSubCluster.add(endTime - startTime);
+ }
+
+ public void addGetAppHomeSubClusterDuration(long endTime, long startTime) {
+getAppHomeSubCluster.add(endTime - startTime);
+ }
+
+ public void addGetAppsHomeSubClusterDuration(long endTime, long startTime) {
+getAppsHomeSubCluster.add(endTime - startTime);
+ }
+
+ public void addDeleteAppHomeSubClusterDuration(long
[GitHub] [hadoop] slfan1989 commented on a diff in pull request #4738: YARN-11250. Capture the Performance Metrics of ZookeeperFederationStateStore.
slfan1989 commented on code in PR #4738:
URL: https://github.com/apache/hadoop/pull/4738#discussion_r948298660
##
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common/src/main/java/org/apache/hadoop/yarn/server/federation/store/impl/ZookeeperFederationStateStore.java:
##
@@ -336,14 +354,14 @@ public SubClusterHeartbeatResponse subClusterHeartbeat(
subClusterInfo.setCapability(request.getCapability());
putSubclusterInfo(subClusterId, subClusterInfo, true);
-
+opDurations.addSubClusterHeartbeatDuration(clock.getTime() - start);
return SubClusterHeartbeatResponse.newInstance();
}
@Override
public GetSubClusterInfoResponse getSubCluster(
- GetSubClusterInfoRequest request) throws YarnException {
-
+ GetSubClusterInfoRequest request) throws YarnException {
Review Comment:
I will fix it.
--
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.
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
[GitHub] [hadoop] slfan1989 commented on a diff in pull request #4738: YARN-11250. Capture the Performance Metrics of ZookeeperFederationStateStore.
slfan1989 commented on code in PR #4738:
URL: https://github.com/apache/hadoop/pull/4738#discussion_r948298488
##
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common/src/main/java/org/apache/hadoop/yarn/server/federation/store/impl/ZookeeperFederationStateStore.java:
##
@@ -203,53 +213,58 @@ public AddApplicationHomeSubClusterResponse
addApplicationHomeSubCluster(
FederationStateStoreUtils.logAndThrowStoreException(LOG, errMsg);
}
SubClusterId newSubClusterId =
-request.getApplicationHomeSubCluster().getHomeSubCluster();
+ request.getApplicationHomeSubCluster().getHomeSubCluster();
putApp(appId, newSubClusterId, true);
+
+opDurations.addUpdateAppHomeSubClusterDuration(clock.getTime() - start);
return UpdateApplicationHomeSubClusterResponse.newInstance();
}
@Override
public GetApplicationHomeSubClusterResponse getApplicationHomeSubCluster(
- GetApplicationHomeSubClusterRequest request) throws YarnException {
+ GetApplicationHomeSubClusterRequest request) throws YarnException {
+long start = clock.getTime();
FederationApplicationHomeSubClusterStoreInputValidator.validate(request);
ApplicationId appId = request.getApplicationId();
SubClusterId homeSubCluster = getApp(appId);
if (homeSubCluster == null) {
String errMsg = "Application " + appId + " does not exist";
FederationStateStoreUtils.logAndThrowStoreException(LOG, errMsg);
}
+opDurations.addGetAppHomeSubClusterDuration(clock.getTime() - start);
return GetApplicationHomeSubClusterResponse.newInstance(
-ApplicationHomeSubCluster.newInstance(appId, homeSubCluster));
+ApplicationHomeSubCluster.newInstance(appId, homeSubCluster));
Review Comment:
I will fix it.
##
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common/src/main/java/org/apache/hadoop/yarn/server/federation/store/impl/ZookeeperFederationStateStore.java:
##
@@ -203,53 +213,58 @@ public AddApplicationHomeSubClusterResponse
addApplicationHomeSubCluster(
FederationStateStoreUtils.logAndThrowStoreException(LOG, errMsg);
}
SubClusterId newSubClusterId =
-request.getApplicationHomeSubCluster().getHomeSubCluster();
+ request.getApplicationHomeSubCluster().getHomeSubCluster();
putApp(appId, newSubClusterId, true);
+
+opDurations.addUpdateAppHomeSubClusterDuration(clock.getTime() - start);
return UpdateApplicationHomeSubClusterResponse.newInstance();
}
@Override
public GetApplicationHomeSubClusterResponse getApplicationHomeSubCluster(
- GetApplicationHomeSubClusterRequest request) throws YarnException {
+ GetApplicationHomeSubClusterRequest request) throws YarnException {
+long start = clock.getTime();
FederationApplicationHomeSubClusterStoreInputValidator.validate(request);
ApplicationId appId = request.getApplicationId();
SubClusterId homeSubCluster = getApp(appId);
if (homeSubCluster == null) {
String errMsg = "Application " + appId + " does not exist";
FederationStateStoreUtils.logAndThrowStoreException(LOG, errMsg);
}
+opDurations.addGetAppHomeSubClusterDuration(clock.getTime() - start);
return GetApplicationHomeSubClusterResponse.newInstance(
-ApplicationHomeSubCluster.newInstance(appId, homeSubCluster));
+ApplicationHomeSubCluster.newInstance(appId, homeSubCluster));
}
@Override
public GetApplicationsHomeSubClusterResponse getApplicationsHomeSubCluster(
- GetApplicationsHomeSubClusterRequest request) throws YarnException {
+ GetApplicationsHomeSubClusterRequest request) throws YarnException {
Review Comment:
I will fix it.
--
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.
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
[GitHub] [hadoop] slfan1989 commented on a diff in pull request #4738: YARN-11250. Capture the Performance Metrics of ZookeeperFederationStateStore.
slfan1989 commented on code in PR #4738:
URL: https://github.com/apache/hadoop/pull/4738#discussion_r948298265
##
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common/src/main/java/org/apache/hadoop/yarn/server/federation/store/impl/ZookeeperFederationStateStore.java:
##
@@ -161,8 +169,9 @@ public void close() throws Exception {
@Override
public AddApplicationHomeSubClusterResponse addApplicationHomeSubCluster(
- AddApplicationHomeSubClusterRequest request) throws YarnException {
+ AddApplicationHomeSubClusterRequest request) throws YarnException {
Review Comment:
I will fix the indentation problem.
##
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common/src/main/java/org/apache/hadoop/yarn/server/federation/store/impl/ZookeeperFederationStateStore.java:
##
@@ -203,53 +213,58 @@ public AddApplicationHomeSubClusterResponse
addApplicationHomeSubCluster(
FederationStateStoreUtils.logAndThrowStoreException(LOG, errMsg);
}
SubClusterId newSubClusterId =
Review Comment:
I will fix it.
--
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.
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
[GitHub] [hadoop] slfan1989 commented on a diff in pull request #4738: YARN-11250. Capture the Performance Metrics of ZookeeperFederationStateStore.
slfan1989 commented on code in PR #4738:
URL: https://github.com/apache/hadoop/pull/4738#discussion_r948295097
##
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common/src/main/java/org/apache/hadoop/yarn/server/federation/store/impl/ZookeeperFederationStateStore.java:
##
@@ -203,53 +213,58 @@ public AddApplicationHomeSubClusterResponse
addApplicationHomeSubCluster(
FederationStateStoreUtils.logAndThrowStoreException(LOG, errMsg);
}
SubClusterId newSubClusterId =
-request.getApplicationHomeSubCluster().getHomeSubCluster();
+ request.getApplicationHomeSubCluster().getHomeSubCluster();
putApp(appId, newSubClusterId, true);
+
+opDurations.addUpdateAppHomeSubClusterDuration(clock.getTime() - start);
return UpdateApplicationHomeSubClusterResponse.newInstance();
}
@Override
public GetApplicationHomeSubClusterResponse getApplicationHomeSubCluster(
- GetApplicationHomeSubClusterRequest request) throws YarnException {
+ GetApplicationHomeSubClusterRequest request) throws YarnException {
+long start = clock.getTime();
FederationApplicationHomeSubClusterStoreInputValidator.validate(request);
ApplicationId appId = request.getApplicationId();
SubClusterId homeSubCluster = getApp(appId);
if (homeSubCluster == null) {
String errMsg = "Application " + appId + " does not exist";
FederationStateStoreUtils.logAndThrowStoreException(LOG, errMsg);
}
+opDurations.addGetAppHomeSubClusterDuration(clock.getTime() - start);
return GetApplicationHomeSubClusterResponse.newInstance(
Review Comment:
Your suggestion is reasonable, I will modify the code.
--
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.
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
[GitHub] [hadoop] slfan1989 commented on a diff in pull request #4738: YARN-11250. Capture the Performance Metrics of ZookeeperFederationStateStore.
slfan1989 commented on code in PR #4738:
URL: https://github.com/apache/hadoop/pull/4738#discussion_r947366439
##
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common/src/main/java/org/apache/hadoop/yarn/server/federation/store/impl/ZookeeperFederationStateStore.java:
##
@@ -104,29 +107,39 @@
public class ZookeeperFederationStateStore implements FederationStateStore {
private static final Logger LOG =
- LoggerFactory.getLogger(ZookeeperFederationStateStore.class);
+ LoggerFactory.getLogger(ZookeeperFederationStateStore.class);
private final static String ROOT_ZNODE_NAME_MEMBERSHIP = "memberships";
private final static String ROOT_ZNODE_NAME_APPLICATION = "applications";
private final static String ROOT_ZNODE_NAME_POLICY = "policies";
- /** Interface to Zookeeper. */
+ /**
+ * Interface to Zookeeper.
+ */
private ZKCuratorManager zkManager;
- /** Directory to store the state store data. */
+ /**
+ * Directory to store the state store data.
+ */
private String baseZNode;
private String appsZNode;
private String membershipZNode;
private String policiesZNode;
+ private volatile Clock clock = SystemClock.getInstance();
+
+ @VisibleForTesting
+ private ZKFederationStateStoreOpDurations opDurations =
+ ZKFederationStateStoreOpDurations.getInstance();
+
@Override
public void init(Configuration conf) throws YarnException {
LOG.info("Initializing ZooKeeper connection");
baseZNode = conf.get(
-YarnConfiguration.FEDERATION_STATESTORE_ZK_PARENT_PATH,
-YarnConfiguration.DEFAULT_FEDERATION_STATESTORE_ZK_PARENT_PATH);
+YarnConfiguration.FEDERATION_STATESTORE_ZK_PARENT_PATH,
Review Comment:
I will fix it.
##
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common/src/main/java/org/apache/hadoop/yarn/server/federation/store/impl/ZookeeperFederationStateStore.java:
##
@@ -203,53 +217,58 @@ public AddApplicationHomeSubClusterResponse
addApplicationHomeSubCluster(
FederationStateStoreUtils.logAndThrowStoreException(LOG, errMsg);
}
SubClusterId newSubClusterId =
-request.getApplicationHomeSubCluster().getHomeSubCluster();
+request.getApplicationHomeSubCluster().getHomeSubCluster();
Review Comment:
I will fix it.
--
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.
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
[GitHub] [hadoop] slfan1989 commented on a diff in pull request #4738: YARN-11250. Capture the Performance Metrics of ZookeeperFederationStateStore.
slfan1989 commented on code in PR #4738:
URL: https://github.com/apache/hadoop/pull/4738#discussion_r947345430
##
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common/src/main/java/org/apache/hadoop/yarn/server/federation/store/impl/ZookeeperFederationStateStore.java:
##
@@ -104,29 +107,39 @@
public class ZookeeperFederationStateStore implements FederationStateStore {
private static final Logger LOG =
- LoggerFactory.getLogger(ZookeeperFederationStateStore.class);
+ LoggerFactory.getLogger(ZookeeperFederationStateStore.class);
private final static String ROOT_ZNODE_NAME_MEMBERSHIP = "memberships";
private final static String ROOT_ZNODE_NAME_APPLICATION = "applications";
private final static String ROOT_ZNODE_NAME_POLICY = "policies";
- /** Interface to Zookeeper. */
+ /**
+ * Interface to Zookeeper.
Review Comment:
I will fix it.
--
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.
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
[GitHub] [hadoop] slfan1989 commented on a diff in pull request #4738: YARN-11250. Capture the Performance Metrics of ZookeeperFederationStateStore.
slfan1989 commented on code in PR #4738:
URL: https://github.com/apache/hadoop/pull/4738#discussion_r947344793
##
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common/src/main/java/org/apache/hadoop/yarn/server/federation/store/impl/ZookeeperFederationStateStore.java:
##
@@ -104,29 +107,39 @@
public class ZookeeperFederationStateStore implements FederationStateStore {
private static final Logger LOG =
- LoggerFactory.getLogger(ZookeeperFederationStateStore.class);
+ LoggerFactory.getLogger(ZookeeperFederationStateStore.class);
Review Comment:
Thanks for your help reviewing the code, I will fix it.
--
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.
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
[GitHub] [hadoop] slfan1989 commented on a diff in pull request #4738: YARN-11250. Capture the Performance Metrics of ZookeeperFederationStateStore.
slfan1989 commented on code in PR #4738:
URL: https://github.com/apache/hadoop/pull/4738#discussion_r946163145
##
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common/src/main/java/org/apache/hadoop/yarn/server/federation/store/impl/ZKFederationStateStoreOpDurations.java:
##
@@ -54,25 +54,25 @@ public final class ZKFederationStateStoreOpDurations
implements MetricsSource {
private MutableRate registerSubClusterCall;
@Metric("Duration for a deregister subCluster call")
- private MutableRate deregisterSubCluster;
+ private MutableRate deregisterSubClusterCall;
Review Comment:
Thanks for your suggestion, there is no need to add the word Call after the
variable, I will modify the code.
--
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.
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
