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: [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]
