[
https://issues.apache.org/jira/browse/YARN-11290?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17599706#comment-17599706
]
ASF GitHub Bot commented on YARN-11290:
---------------------------------------
goiri commented on code in PR #4846:
URL: https://github.com/apache/hadoop/pull/4846#discussion_r961990524
##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common/src/main/java/org/apache/hadoop/yarn/server/federation/store/impl/MemoryFederationStateStore.java:
##########
@@ -255,14 +261,33 @@ public GetApplicationHomeSubClusterResponse
getApplicationHomeSubCluster(
@Override
public GetApplicationsHomeSubClusterResponse getApplicationsHomeSubCluster(
GetApplicationsHomeSubClusterRequest request) throws YarnException {
- List<ApplicationHomeSubCluster> result =
- new ArrayList<ApplicationHomeSubCluster>();
- for (Entry<ApplicationId, SubClusterId> e : applications.entrySet()) {
- result
- .add(ApplicationHomeSubCluster.newInstance(e.getKey(),
e.getValue()));
+
+ if (request == null) {
+ throw new YarnException("Missing getApplicationsHomeSubCluster request");
+ }
+
+ List<ApplicationHomeSubCluster> result = new ArrayList<>();
+ List<ApplicationId> applicationIdList =
+ applications.keySet().stream().collect(Collectors.toList());
+
+ SubClusterId requestSubClusterId = request.getSubClusterId();
+ int appCount = 0;
+ for (int i = 0; i < applicationIdList.size(); i++) {
+ if (appCount >= maxAppsInStateStore) {
Review Comment:
Move this to the for condition.
##########
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:
##########
@@ -255,23 +260,41 @@ public GetApplicationHomeSubClusterResponse
getApplicationHomeSubCluster(
@Override
public GetApplicationsHomeSubClusterResponse getApplicationsHomeSubCluster(
GetApplicationsHomeSubClusterRequest request) throws YarnException {
+
+ if (request == null) {
+ throw new YarnException("Missing getApplicationsHomeSubCluster request");
+ }
+
long start = clock.getTime();
List<ApplicationHomeSubCluster> result = new ArrayList<>();
+ SubClusterId requestSubClusterId = request.getSubClusterId();
+ int appCount = 0;
try {
- for (String child : zkManager.getChildren(appsZNode)) {
+ List<String> childrens = zkManager.getChildren(appsZNode);
+ for (String child : childrens) {
+ if (appCount >= maxAppsInStateStore) {
+ break;
+ }
ApplicationId appId = ApplicationId.fromString(child);
SubClusterId homeSubCluster = getApp(appId);
- ApplicationHomeSubCluster app =
- ApplicationHomeSubCluster.newInstance(appId, homeSubCluster);
+ // If the requestSubClusterId that needs to be filtered in the request
+ // is inconsistent with the SubClusterId in the data, continue to the
next round
+ if (requestSubClusterId != null &&
!requestSubClusterId.equals(homeSubCluster)) {
+ continue;
Review Comment:
reverse the if
##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common/src/main/java/org/apache/hadoop/yarn/server/federation/store/impl/SQLFederationStateStore.java:
##########
@@ -726,13 +731,23 @@ public GetApplicationHomeSubClusterResponse
getApplicationHomeSubCluster(
@Override
public GetApplicationsHomeSubClusterResponse getApplicationsHomeSubCluster(
GetApplicationsHomeSubClusterRequest request) throws YarnException {
+
+ if (request == null) {
+ throw new YarnException("Missing getApplicationsHomeSubCluster request");
+ }
+
CallableStatement cstmt = null;
ResultSet rs = null;
- List<ApplicationHomeSubCluster> appsHomeSubClusters =
- new ArrayList<ApplicationHomeSubCluster>();
+ List<ApplicationHomeSubCluster> appsHomeSubClusters = new ArrayList<>();
try {
cstmt = getCallableStatement(CALL_SP_GET_APPLICATIONS_HOME_SUBCLUSTER);
+ cstmt.setInt("limit_IN", maxAppsInStateStore);
+ String homeSubClusterIN = null;;
Review Comment:
Extra ;
##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common/src/main/java/org/apache/hadoop/yarn/server/federation/store/impl/MemoryFederationStateStore.java:
##########
@@ -255,14 +261,33 @@ public GetApplicationHomeSubClusterResponse
getApplicationHomeSubCluster(
@Override
public GetApplicationsHomeSubClusterResponse getApplicationsHomeSubCluster(
GetApplicationsHomeSubClusterRequest request) throws YarnException {
- List<ApplicationHomeSubCluster> result =
- new ArrayList<ApplicationHomeSubCluster>();
- for (Entry<ApplicationId, SubClusterId> e : applications.entrySet()) {
- result
- .add(ApplicationHomeSubCluster.newInstance(e.getKey(),
e.getValue()));
+
+ if (request == null) {
+ throw new YarnException("Missing getApplicationsHomeSubCluster request");
+ }
+
+ List<ApplicationHomeSubCluster> result = new ArrayList<>();
+ List<ApplicationId> applicationIdList =
+ applications.keySet().stream().collect(Collectors.toList());
+
+ SubClusterId requestSubClusterId = request.getSubClusterId();
+ int appCount = 0;
+ for (int i = 0; i < applicationIdList.size(); i++) {
+ if (appCount >= maxAppsInStateStore) {
+ break;
+ }
+ ApplicationId applicationId = applicationIdList.get(i);
+ SubClusterId subClusterId = applications.get(applicationId);
+ // If the requestSubClusterId that needs to be filtered in the request
+ // is inconsistent with the SubClusterId in the data, continue to the
next round
+ if (requestSubClusterId != null &&
!requestSubClusterId.equals(subClusterId)){
+ continue;
+ }
+ result.add(ApplicationHomeSubCluster.newInstance(applicationId,
subClusterId));
+ appCount++;
}
- GetApplicationsHomeSubClusterResponse.newInstance(result);
Review Comment:
This was just overlooked?
##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common/src/main/java/org/apache/hadoop/yarn/server/federation/store/impl/MemoryFederationStateStore.java:
##########
@@ -255,14 +261,33 @@ public GetApplicationHomeSubClusterResponse
getApplicationHomeSubCluster(
@Override
public GetApplicationsHomeSubClusterResponse getApplicationsHomeSubCluster(
GetApplicationsHomeSubClusterRequest request) throws YarnException {
- List<ApplicationHomeSubCluster> result =
- new ArrayList<ApplicationHomeSubCluster>();
- for (Entry<ApplicationId, SubClusterId> e : applications.entrySet()) {
- result
- .add(ApplicationHomeSubCluster.newInstance(e.getKey(),
e.getValue()));
+
+ if (request == null) {
+ throw new YarnException("Missing getApplicationsHomeSubCluster request");
+ }
+
+ List<ApplicationHomeSubCluster> result = new ArrayList<>();
+ List<ApplicationId> applicationIdList =
+ applications.keySet().stream().collect(Collectors.toList());
+
+ SubClusterId requestSubClusterId = request.getSubClusterId();
+ int appCount = 0;
+ for (int i = 0; i < applicationIdList.size(); i++) {
+ if (appCount >= maxAppsInStateStore) {
Review Comment:
Actually it would be good to do a foreach
##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common/src/main/java/org/apache/hadoop/yarn/server/federation/store/impl/MemoryFederationStateStore.java:
##########
@@ -255,14 +261,33 @@ public GetApplicationHomeSubClusterResponse
getApplicationHomeSubCluster(
@Override
public GetApplicationsHomeSubClusterResponse getApplicationsHomeSubCluster(
GetApplicationsHomeSubClusterRequest request) throws YarnException {
- List<ApplicationHomeSubCluster> result =
- new ArrayList<ApplicationHomeSubCluster>();
- for (Entry<ApplicationId, SubClusterId> e : applications.entrySet()) {
- result
- .add(ApplicationHomeSubCluster.newInstance(e.getKey(),
e.getValue()));
+
+ if (request == null) {
+ throw new YarnException("Missing getApplicationsHomeSubCluster request");
+ }
+
+ List<ApplicationHomeSubCluster> result = new ArrayList<>();
+ List<ApplicationId> applicationIdList =
+ applications.keySet().stream().collect(Collectors.toList());
+
+ SubClusterId requestSubClusterId = request.getSubClusterId();
+ int appCount = 0;
+ for (int i = 0; i < applicationIdList.size(); i++) {
+ if (appCount >= maxAppsInStateStore) {
+ break;
+ }
+ ApplicationId applicationId = applicationIdList.get(i);
+ SubClusterId subClusterId = applications.get(applicationId);
+ // If the requestSubClusterId that needs to be filtered in the request
+ // is inconsistent with the SubClusterId in the data, continue to the
next round
+ if (requestSubClusterId != null &&
!requestSubClusterId.equals(subClusterId)){
+ continue;
+ }
+ result.add(ApplicationHomeSubCluster.newInstance(applicationId,
subClusterId));
Review Comment:
do the reverse if to avoid the continue
> Improve Query Condition of FederationStateStore#getApplicationsHomeSubCluster
> -----------------------------------------------------------------------------
>
> Key: YARN-11290
> URL: https://issues.apache.org/jira/browse/YARN-11290
> Project: Hadoop YARN
> Issue Type: Improvement
> Components: federation
> Affects Versions: 3.4.0
> Reporter: fanshilun
> Assignee: fanshilun
> Priority: Minor
> Labels: pull-request-available
>
> 1. Currently this interface returns the number of apps in all sub-clusters,
> increasing the limit on the number of query apps, limited to 1000 apps.
> 2. Allows to query the App based on the specified HomeSubCluster.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
