[
https://issues.apache.org/jira/browse/YARN-9048?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17771769#comment-17771769
]
ASF GitHub Bot commented on YARN-9048:
--------------------------------------
slfan1989 commented on code in PR #6016:
URL: https://github.com/apache/hadoop/pull/6016#discussion_r1345450428
##########
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:
Thank you very much for your help in reviewing the code! I will improve this
part of the code.
##########
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:
I will improve this part of the code.
> Add znode hierarchy in Federation ZK State Store
> ------------------------------------------------
>
> Key: YARN-9048
> URL: https://issues.apache.org/jira/browse/YARN-9048
> Project: Hadoop YARN
> Issue Type: Sub-task
> Reporter: Bibin Chundatt
> Assignee: Shilun Fan
> Priority: Major
> Labels: pull-request-available
>
> Similar to YARN-2962 consider having hierarchy in ZK federation store for
> applications
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
