goiri commented on code in PR #6016:
URL: https://github.com/apache/hadoop/pull/6016#discussion_r1344369027
##########
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:
##########
@@ -1674,4 +1813,194 @@ public int incrementCurrentKeyId() {
}
return keyIdSeqCounter.getCount();
}
+
+ /**
+ * Get parent app node path based on full path and split index supplied.
+ * @param appIdPath App id path for which parent needs to be returned.
+ * @param splitIndex split index.
+ * @return parent app node path.
+ */
+ private String getSplitAppNodeParent(String appIdPath, int splitIndex) {
+ // Calculated as string upto index (appIdPath Length - split index - 1). We
+ // deduct 1 to exclude path separator.
+ return appIdPath.substring(0, appIdPath.length() - splitIndex - 1);
+ }
+
+ /**
+ * Checks if parent app node has no leaf nodes and if it does not have,
+ * removes it. Called while removing application.
+ *
+ * @param appIdPath path of app id to be removed.
+ * @param splitIndex split index.
+ * @throws Exception if any problem occurs while performing ZK operation.
+ */
+ private void checkRemoveParentAppNode(String appIdPath, int splitIndex)
+ throws Exception {
+ if (splitIndex == 0) {
+ return;
+ }
+
+ String parentAppNode = getSplitAppNodeParent(appIdPath, splitIndex);
+ List<String> children;
+ try {
+ children = getChildren(parentAppNode);
+ } catch (KeeperException.NoNodeException ke) {
+ // It should be fine to swallow this exception as the parent app node we
+ // intend to delete is already deleted.
+ LOG.debug("Unable to remove app parent node {} as it does not exist.",
+ parentAppNode);
+ return;
+ }
+
+ // If children==null or children is not empty, we cannot delete the parent
path.
+ if (children == null || !children.isEmpty()) {
+ return;
+ }
+
+ // No apps stored under parent path.
+ try {
+ zkManager.delete(parentAppNode);
+ LOG.debug("No leaf app node exists. Removing parent node {}.",
parentAppNode);
+ } catch (KeeperException.NotEmptyException ke) {
+ // It should be fine to swallow this exception as the parent app node
+ // has to be deleted only if it has no children. And this node has.
+ LOG.debug("Unable to remove app parent node {} as it has children.",
+ parentAppNode);
+ }
+ }
+
+ List<String> getChildren(final String path) throws Exception {
+ return zkManager.getChildren(path);
+ }
+
+ /**
+ * Get alternate path for app id if path according to configured split index
+ * does not exist. We look for path based on all possible split indices.
+ * @param appId
+ * @return a {@link AppNodeSplitInfo} object containing the path and split
+ * index if it exists, null otherwise.
+ * @throws Exception if any problem occurs while performing ZK operation.
+ */
+ private AppNodeSplitInfo getAlternatePath(String appId) throws Exception {
+ for (Map.Entry<Integer, String> entry :
routerAppRootHierarchies.entrySet()) {
+ // Look for other paths
+ int splitIndex = entry.getKey();
+ if (splitIndex != appIdNodeSplitIndex) {
+ String alternatePath =
+ getLeafAppIdNodePath(appId, entry.getValue(), splitIndex, false);
+ if (exists(alternatePath)) {
+ return new AppNodeSplitInfo(alternatePath, splitIndex);
+ }
+ }
+ }
+ return null;
+ }
+
+ /**
+ * Returns leaf app node path based on app id and passed split index. If the
+ * passed flag createParentIfNotExists is true, also creates the parent app
+ * node if it does not exist.
+ * @param appId application id.
+ * @param rootNode app root node based on split index.
+ * @param appIdNodeSplitIdx split index.
+ * @param createParentIfNotExists flag which determines if parent app node
+ * needs to be created(as per split) if it does not exist.
+ * @return leaf app node path.
+ * @throws Exception if any problem occurs while performing ZK operation.
+ */
+ private String getLeafAppIdNodePath(String appId, String rootNode,
+ int appIdNodeSplitIdx, boolean createParentIfNotExists) throws
Exception {
+ if (appIdNodeSplitIdx == 0) {
+ return getNodePath(rootNode, appId);
+ }
+ String nodeName = appId;
+ int splitIdx = nodeName.length() - appIdNodeSplitIdx;
+ String rootNodePath = getNodePath(rootNode, nodeName.substring(0,
splitIdx));
+ if (createParentIfNotExists && !exists(rootNodePath)) {
+ try {
+ zkManager.create(rootNodePath);
+ } catch (KeeperException.NodeExistsException e) {
+ LOG.debug("Unable to create app parent node {} as it already exists.",
rootNodePath);
+ }
+ }
+ return getNodePath(rootNodePath, nodeName.substring(splitIdx));
+ }
+
+ /**
+ * Returns leaf app node path based on app id and configured split index. If
+ * the passed flag createParentIfNotExists is true, also creates the parent
+ * app node if it does not exist.
+ * @param appId application id.
+ * @param createParentIfNotExists flag which determines if parent app node
+ * needs to be created(as per split) if it does not exist.
+ * @return leaf app node path.
+ * @throws YarnException if any problem occurs while performing ZK operation.
+ */
+ private String getLeafAppIdNodePath(String appId,
+ boolean createParentIfNotExists) throws YarnException {
+ try {
+ String rootNode = routerAppRootHierarchies.get(appIdNodeSplitIndex);
+ return getLeafAppIdNodePath(appId, rootNode, appIdNodeSplitIndex,
createParentIfNotExists);
+ } catch (Exception e) {
+ throw new YarnException(e);
+ }
+ }
+
+ private ApplicationHomeSubCluster loadRouterAppStateFromAppNode(String
appNodePath)
+ throws Exception {
+ byte[] data = get(appNodePath);
+ LOG.debug("Loading application from znode: {}", appNodePath);
+ ApplicationHomeSubCluster appHomeSubCluster = null;
+ if (data != null) {
Review Comment:
You could do early return.
```
if (data == null) {
return appHomeSubCluster;
}
```
##########
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:
##########
@@ -761,11 +867,44 @@ private ApplicationHomeSubCluster
getApplicationHomeSubCluster(
private void storeOrUpdateApplicationHomeSubCluster(final ApplicationId
applicationId,
final ApplicationHomeSubCluster applicationHomeSubCluster, boolean
update)
throws YarnException {
- String appZNode = getNodePath(appsZNode, applicationId.toString());
- ApplicationHomeSubClusterProto proto =
- ((ApplicationHomeSubClusterPBImpl)
applicationHomeSubCluster).getProto();
- byte[] data = proto.toByteArray();
- put(appZNode, data, update);
+ try {
+ ApplicationHomeSubClusterProto proto =
+ ((ApplicationHomeSubClusterPBImpl)
applicationHomeSubCluster).getProto();
+ byte[] data = proto.toByteArray();
+ if (update) {
+ updateApplicationStateInternal(applicationId, data);
+ } else {
+ storeApplicationStateInternal(applicationId, data);
+ }
+ } catch (Exception e) {
+ throw new YarnException(e);
+ }
+ }
+
+ protected void storeApplicationStateInternal(final ApplicationId
applicationId, byte[] data)
+ throws Exception {
+ String nodeCreatePath = getLeafAppIdNodePath(applicationId.toString(),
true);
+ LOG.debug("Storing info for app: {} at: {}.", applicationId,
nodeCreatePath);
+ put(nodeCreatePath, data, false);
+ }
+
+ protected void updateApplicationStateInternal(final ApplicationId
applicationId, byte[] data)
+ throws Exception {
+ String nodeUpdatePath = getLeafAppIdNodePath(applicationId.toString(),
false);
+ if (!exists(nodeUpdatePath)) {
+ AppNodeSplitInfo alternatePathInfo =
getAlternatePath(applicationId.toString());
+ if (alternatePathInfo != null) {
+ nodeUpdatePath = alternatePathInfo.path;
+ } else if (appIdNodeSplitIndex != 0) {
+ // No alternate path exists. Create path as per configured split
index.
+ String rootNode = getSplitAppNodeParent(nodeUpdatePath,
appIdNodeSplitIndex);
Review Comment:
checkstyle is complaining about this line
##########
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:
##########
@@ -1674,4 +1813,194 @@ public int incrementCurrentKeyId() {
}
return keyIdSeqCounter.getCount();
}
+
+ /**
+ * Get parent app node path based on full path and split index supplied.
+ * @param appIdPath App id path for which parent needs to be returned.
+ * @param splitIndex split index.
+ * @return parent app node path.
+ */
+ private String getSplitAppNodeParent(String appIdPath, int splitIndex) {
+ // Calculated as string upto index (appIdPath Length - split index - 1). We
+ // deduct 1 to exclude path separator.
+ return appIdPath.substring(0, appIdPath.length() - splitIndex - 1);
+ }
+
+ /**
+ * Checks if parent app node has no leaf nodes and if it does not have,
+ * removes it. Called while removing application.
+ *
+ * @param appIdPath path of app id to be removed.
+ * @param splitIndex split index.
+ * @throws Exception if any problem occurs while performing ZK operation.
+ */
+ private void checkRemoveParentAppNode(String appIdPath, int splitIndex)
+ throws Exception {
+ if (splitIndex == 0) {
+ return;
+ }
+
+ String parentAppNode = getSplitAppNodeParent(appIdPath, splitIndex);
+ List<String> children;
+ try {
+ children = getChildren(parentAppNode);
+ } catch (KeeperException.NoNodeException ke) {
+ // It should be fine to swallow this exception as the parent app node we
+ // intend to delete is already deleted.
+ LOG.debug("Unable to remove app parent node {} as it does not exist.",
+ parentAppNode);
+ return;
+ }
+
+ // If children==null or children is not empty, we cannot delete the parent
path.
+ if (children == null || !children.isEmpty()) {
+ return;
+ }
+
+ // No apps stored under parent path.
+ try {
+ zkManager.delete(parentAppNode);
+ LOG.debug("No leaf app node exists. Removing parent node {}.",
parentAppNode);
+ } catch (KeeperException.NotEmptyException ke) {
+ // It should be fine to swallow this exception as the parent app node
+ // has to be deleted only if it has no children. And this node has.
+ LOG.debug("Unable to remove app parent node {} as it has children.",
+ parentAppNode);
+ }
+ }
+
+ List<String> getChildren(final String path) throws Exception {
+ return zkManager.getChildren(path);
+ }
+
+ /**
+ * Get alternate path for app id if path according to configured split index
+ * does not exist. We look for path based on all possible split indices.
+ * @param appId
+ * @return a {@link AppNodeSplitInfo} object containing the path and split
+ * index if it exists, null otherwise.
+ * @throws Exception if any problem occurs while performing ZK operation.
+ */
+ private AppNodeSplitInfo getAlternatePath(String appId) throws Exception {
+ for (Map.Entry<Integer, String> entry :
routerAppRootHierarchies.entrySet()) {
+ // Look for other paths
+ int splitIndex = entry.getKey();
+ if (splitIndex != appIdNodeSplitIndex) {
+ String alternatePath =
+ getLeafAppIdNodePath(appId, entry.getValue(), splitIndex, false);
+ if (exists(alternatePath)) {
+ return new AppNodeSplitInfo(alternatePath, splitIndex);
+ }
+ }
+ }
+ return null;
+ }
+
+ /**
+ * Returns leaf app node path based on app id and passed split index. If the
+ * passed flag createParentIfNotExists is true, also creates the parent app
+ * node if it does not exist.
+ * @param appId application id.
+ * @param rootNode app root node based on split index.
+ * @param appIdNodeSplitIdx split index.
+ * @param createParentIfNotExists flag which determines if parent app node
+ * needs to be created(as per split) if it does not exist.
+ * @return leaf app node path.
+ * @throws Exception if any problem occurs while performing ZK operation.
+ */
+ private String getLeafAppIdNodePath(String appId, String rootNode,
+ int appIdNodeSplitIdx, boolean createParentIfNotExists) throws
Exception {
+ if (appIdNodeSplitIdx == 0) {
+ return getNodePath(rootNode, appId);
+ }
+ String nodeName = appId;
+ int splitIdx = nodeName.length() - appIdNodeSplitIdx;
+ String rootNodePath = getNodePath(rootNode, nodeName.substring(0,
splitIdx));
+ if (createParentIfNotExists && !exists(rootNodePath)) {
+ try {
+ zkManager.create(rootNodePath);
+ } catch (KeeperException.NodeExistsException e) {
+ LOG.debug("Unable to create app parent node {} as it already exists.",
rootNodePath);
+ }
+ }
+ return getNodePath(rootNodePath, nodeName.substring(splitIdx));
+ }
+
+ /**
+ * Returns leaf app node path based on app id and configured split index. If
+ * the passed flag createParentIfNotExists is true, also creates the parent
+ * app node if it does not exist.
+ * @param appId application id.
+ * @param createParentIfNotExists flag which determines if parent app node
+ * needs to be created(as per split) if it does not exist.
+ * @return leaf app node path.
+ * @throws YarnException if any problem occurs while performing ZK operation.
+ */
+ private String getLeafAppIdNodePath(String appId,
+ boolean createParentIfNotExists) throws YarnException {
+ try {
+ String rootNode = routerAppRootHierarchies.get(appIdNodeSplitIndex);
+ return getLeafAppIdNodePath(appId, rootNode, appIdNodeSplitIndex,
createParentIfNotExists);
+ } catch (Exception e) {
+ throw new YarnException(e);
+ }
+ }
+
+ private ApplicationHomeSubCluster loadRouterAppStateFromAppNode(String
appNodePath)
+ throws Exception {
+ byte[] data = get(appNodePath);
+ LOG.debug("Loading application from znode: {}", appNodePath);
+ ApplicationHomeSubCluster appHomeSubCluster = null;
+ if (data != null) {
+ try {
+ appHomeSubCluster = new ApplicationHomeSubClusterPBImpl(
+ ApplicationHomeSubClusterProto.parseFrom(data));
+ } catch (InvalidProtocolBufferException e) {
+ String errMsg = "Cannot parse application at " + appNodePath;
+ FederationStateStoreUtils.logAndThrowStoreException(LOG, errMsg);
+ }
+ }
+ return appHomeSubCluster;
+ }
+
+ private List<ApplicationHomeSubCluster> loadRouterApplications() throws
Exception {
+ List<ApplicationHomeSubCluster> applicationHomeSubClusters = new
ArrayList<>();
+ for (int splitIndex = 0; splitIndex <= 4; splitIndex++) {
Review Comment:
Again can we have the 4 defined somewhere?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
