goiri commented on code in PR #4846: URL: https://github.com/apache/hadoop/pull/4846#discussion_r968842977
########## hadoop-yarn-project/hadoop-yarn/bin/FederationStateStore/MySQL/FederationStateStoreStoredProcs.sql: ########## @@ -122,10 +122,26 @@ BEGIN WHERE applicationId = applicationID_IN; END // -CREATE PROCEDURE sp_getApplicationsHomeSubCluster() -BEGIN - SELECT applicationId, homeSubCluster - FROM applicationsHomeSubCluster; +CREATE PROCEDURE sp_getApplicationsHomeSubCluster(IN limit_IN int, IN homeSubCluster_IN varchar(256)) +BEGIN + SELECT + applicationId, + homeSubCluster, + createTime + FROM + (SELECT + applicationId, + homeSubCluster, + createTime, + @app_rank := IF(@home_sc = homeSubCluster, @app_rank + 1, 1) AS app_rank, + @home_sc := homeSubCluster + FROM applicationshomesubcluster Review Comment: The tabs in line warnings are legit. Let's replace them with spaces. ########## hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common/src/main/java/org/apache/hadoop/yarn/server/federation/store/records/ApplicationHomeSubCluster.java: ########## @@ -51,6 +51,18 @@ public static ApplicationHomeSubCluster newInstance(ApplicationId appId, return appMapping; } + @Private + @Unstable + public static ApplicationHomeSubCluster newInstance(ApplicationId appId, long createTime, + SubClusterId homeSubCluster) { + ApplicationHomeSubCluster appMapping = Review Comment: Single line ########## hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common/src/test/java/org/apache/hadoop/yarn/server/federation/store/impl/FederationStateStoreBaseTest.java: ########## @@ -410,6 +414,89 @@ public void testGetApplicationsHomeSubCluster() throws Exception { Assert.assertTrue(result.getAppsHomeSubClusters().contains(ahsc2)); } + @Test + public void testGetApplicationsHomeSubClusterEmpty() throws Exception { + LambdaTestUtils.intercept(YarnException.class, + "Missing getApplicationsHomeSubCluster request", + () -> stateStore.getApplicationsHomeSubCluster(null)); + } + + @Test + public void testGetApplicationsHomeSubClusterFilter() throws Exception { + // Add ApplicationHomeSC - SC1 + long now = Time.now(); + + Set<ApplicationHomeSubCluster> appHomeSubClusters = new HashSet<>(); + + for (int i = 0; i < TEN_ROUNDS; i++) { Review Comment: TEN_ROUNDS is a weird variable. I was initially thinking more about something like NUM_APPS_X and NUM_APSS_Y ########## hadoop-yarn-project/hadoop-yarn/bin/FederationStateStore/MySQL/FederationStateStoreStoredProcs.sql: ########## @@ -122,10 +122,26 @@ BEGIN WHERE applicationId = applicationID_IN; END // -CREATE PROCEDURE sp_getApplicationsHomeSubCluster() -BEGIN - SELECT applicationId, homeSubCluster - FROM applicationsHomeSubCluster; +CREATE PROCEDURE sp_getApplicationsHomeSubCluster(IN limit_IN int, IN homeSubCluster_IN varchar(256)) +BEGIN + SELECT + applicationId, + homeSubCluster, + createTime + FROM + (SELECT + applicationId, + homeSubCluster, + createTime, + @app_rank := IF(@home_sc = homeSubCluster, @app_rank + 1, 1) AS app_rank, + @home_sc := homeSubCluster + FROM applicationshomesubcluster + ORDER BY createTime DESC + ) ranked + WHERE app_rank <= limit_IN + AND (CASE WHEN t.homeSubCluster_IN IS NULL THEN 1 = 1 Review Comment: Can we make this a regular boolean condition with no CASE? ########## hadoop-yarn-project/hadoop-yarn/bin/FederationStateStore/MySQL/FederationStateStoreStoredProcs.sql: ########## @@ -122,10 +122,26 @@ BEGIN WHERE applicationId = applicationID_IN; END // -CREATE PROCEDURE sp_getApplicationsHomeSubCluster() -BEGIN - SELECT applicationId, homeSubCluster - FROM applicationsHomeSubCluster; +CREATE PROCEDURE sp_getApplicationsHomeSubCluster(IN limit_IN int, IN homeSubCluster_IN varchar(256)) +BEGIN + SELECT + applicationId, + homeSubCluster, + createTime + FROM + (SELECT + applicationId, + homeSubCluster, + createTime, Review Comment: Shouldn't we sort this? ########## hadoop-yarn-project/hadoop-yarn/bin/FederationStateStore/SQLServer/FederationStateStoreStoreProcs.sql: ########## @@ -111,12 +111,27 @@ IF OBJECT_ID ( '[sp_getApplicationsHomeSubCluster]', 'P' ) IS NOT NULL GO CREATE PROCEDURE [dbo].[sp_getApplicationsHomeSubCluster] + @limit int, + @homeSubCluster VARCHAR(256) AS BEGIN DECLARE @errorMessage nvarchar(4000) BEGIN TRY - SELECT [applicationId], [homeSubCluster], [createTime] - FROM [dbo].[applicationsHomeSubCluster] + SELECT + [applicationId], + [homeSubCluster], + [createTime] + FROM + (SELECT + [applicationId], + [homeSubCluster], + [createTime], + row_number() over(partition by [homeSubCluster] order by [createTime] desc) AS row_num + FROM [dbo].[applicationsHomeSubCluster]) AS t + WHERE row_num <= @limit + AND (CASE WHEN @homeSubCluster IS NULL THEN 1 Review Comment: Do we really need to use case? We cannot do some boolean condition? -- 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