Add new APIs for recommendations summary at Clusters and Namespaces level #893

khansaad · 2023-07-14T05:20:30Z

This PR fixes #892

New APIs - /listClusters and /summarize are added which provides summary of the recommendations at cluster and namespace level

Signed-off-by: saakhan <saakhan@redhat.com>

…od in listRecomm API to fetch data in summarizeAPI, add converter method for summarizeAPI and other relevant changes Signed-off-by: saakhan <saakhan@redhat.com>

# Conflicts: # src/main/java/com/autotune/analyzer/serviceObjects/Converters.java

Signed-off-by: saakhan <saakhan@redhat.com>

…y and namespaces Signed-off-by: saakhan <saakhan@redhat.com>

Signed-off-by: saakhan <saakhan@redhat.com>

… recomm and filtering data locally Signed-off-by: saakhan <saakhan@redhat.com>

… same cluster name issue and other minor changes Signed-off-by: saakhan <saakhan@redhat.com>

# Conflicts: # src/main/java/com/autotune/database/dao/ExperimentDAO.java # src/main/java/com/autotune/database/dao/ExperimentDAOImpl.java # src/main/java/com/autotune/database/helper/DBConstants.java

Signed-off-by: saakhan <saakhan@redhat.com>

# Conflicts: # src/main/java/com/autotune/database/dao/ExperimentDAO.java # src/main/java/com/autotune/database/dao/ExperimentDAOImpl.java # src/main/java/com/autotune/database/helper/DBConstants.java # src/main/java/com/autotune/database/service/ExperimentDBService.java

Signed-off-by: saakhan <saakhan@redhat.com>

… flag to force pull from the DB when required Signed-off-by: saakhan <saakhan@redhat.com>

# Conflicts: # src/main/java/com/autotune/database/dao/ExperimentDAOImpl.java # src/main/java/com/autotune/database/helper/DBConstants.java

# Conflicts: # src/main/java/com/autotune/analyzer/services/ListRecommendations.java

…d new error and info category for action summary Signed-off-by: saakhan <saakhan@redhat.com>

src/main/java/com/autotune/analyzer/services/ListClusters.java

src/main/java/com/autotune/analyzer/services/Summarize.java

msvinaykumar · 2023-08-28T07:34:12Z

src/main/java/com/autotune/analyzer/services/Summarize.java

+            if (summarizeType == null || summarizeType.isEmpty())
+                summarizeType = KruizeConstants.JSONKeys.CLUSTER;
+            // by default, db_flag will be false so the data will be fetched from cache only
+            if (fetchFromDB == null || fetchFromDB.isEmpty())


based on description should be renamed
as fetchFromCache
fetchFrom(DB or Thanos) varaible can be used when we move to Thanos server
Just a thought so you can ignore this comment .

src/main/java/com/autotune/database/dao/ExperimentDAOImpl.java

src/main/java/com/autotune/analyzer/services/Summarize.java

msvinaykumar

Obtain a preliminary assessment of the data volume that Kruize needs to handle, and verify the proper functionality of the summarization API, both with and without pagination. Currently, the summary API operates in either of two modes: it retrieves and summarizes data for either
(1) all clusters and namespaces, or
(2) a specified cluster and namespace. It would be advantageous to introduce additional options, such as
(3) fetching data for all cluster names that begin with 'xyz*', or allowing a list of comma-separated cluster names to be specified.

msvinaykumar · 2023-08-28T08:52:21Z

src/main/java/com/autotune/analyzer/services/Summarize.java

+
+public class Summarize extends HttpServlet {
+    private static final Logger LOGGER = LoggerFactory.getLogger(Summarize.class);
+    HashMap<String, SummarizeAPIObject> clusterSummaryCacheMap = new HashMap<>();


move initialization to init() method and leave only declaration here. And evaluate if any parallel request to this API cause any issues here.

msvinaykumar · 2023-08-28T08:53:13Z

src/main/java/com/autotune/analyzer/services/Summarize.java

+public class Summarize extends HttpServlet {
+    private static final Logger LOGGER = LoggerFactory.getLogger(Summarize.class);
+    HashMap<String, SummarizeAPIObject> clusterSummaryCacheMap = new HashMap<>();
+    HashMap<String, SummarizeAPIObject> namespaceSummaryCacheMap = new HashMap<>();


Transfer the initialization process to the init() method and retain only the declaration here. Then, assess whether any potential problems arise from concurrent requests to this API.

I suggest to make use of ServletContext
provides a more controlled and managed way to share data among different servlets, and it handles the potential concurrency issues that may arise in a multithreaded environment.

msvinaykumar · 2023-08-28T09:14:04Z

src/main/java/com/autotune/database/service/ExperimentDBService.java

+        }
+    }
+
+    public void loadExperimentsAndRecommendationsByClusterName(Map<String, KruizeObject> mKruizeExperimentMap, String clusterName) throws Exception {


Implement a try-catch block to capture potential failures while loading experiment details or experiment recommendations. In case of failure for either or both of these tasks, provide appropriate user notification. Additionally, ensure that the loop continues to execute even if a few corrupt entries are encountered. It's important that the failure of a few entries doesn't disrupt the entire process.

msvinaykumar · 2023-08-28T09:21:19Z

src/main/java/com/autotune/database/helper/DBConstants.java

@@ -15,9 +17,13 @@ public static final class SQLQUERY {
        public static final String SELECT_FROM_RESULTS_BY_EXP_NAME_AND_MAX_END_TIME = String.format("from KruizeResultsEntry k WHERE k.experiment_name = :%s and k.interval_end_time = (SELECT MAX(e.interval_end_time) FROM KruizeResultsEntry e  where e.experiment_name = :%s )", KruizeConstants.JSONKeys.EXPERIMENT_NAME, KruizeConstants.JSONKeys.EXPERIMENT_NAME);
        public static final String SELECT_FROM_RECOMMENDATIONS_BY_EXP_NAME = "from KruizeRecommendationEntry k WHERE k.experiment_name = :experimentName";
        public static final String SELECT_FROM_RECOMMENDATIONS_BY_EXP_NAME_AND_END_TIME = String.format("from KruizeRecommendationEntry k WHERE k.experiment_name = :%s and k.interval_end_time= :%s", KruizeConstants.JSONKeys.EXPERIMENT_NAME, KruizeConstants.JSONKeys.INTERVAL_END_TIME);
+        public static final String SELECT_FROM_RECOMMENDATIONS_BY_CLUSTER_NAME = "from KruizeRecommendationEntry k WHERE k.cluster_name = :clusterName and k.experiment_name IN :experimentList ORDER by interval_end_time DESC LIMIT :limit";


Refrain from employing the 'limit' statement on its own...
Instead, employ the 'limit' statement in conjunction with carefully optimized 'where' conditions. The 'limit' statement processes the entire content and then returns a specified count. However, we can enhance efficiency by strategically utilizing 'where' conditions along with the 'limit' statement, thus mitigating the need to process the entire content.

Signed-off-by: saakhan <saakhan@redhat.com>

…per the review Signed-off-by: saakhan <saakhan@redhat.com>

Signed-off-by: saakhan <saakhan@redhat.com>

dinogun · 2023-08-31T09:46:33Z

src/main/java/com/autotune/analyzer/serviceObjects/SummarizeAPIObject.java

+import java.util.HashMap;
+
+public class SummarizeAPIObject {
+


Needs to have a API version similar to all the other APIs

# Conflicts: # src/main/java/com/autotune/utils/KruizeConstants.java

# Conflicts: # src/main/java/com/autotune/database/dao/ExperimentDAOImpl.java # src/main/java/com/autotune/database/helper/DBConstants.java # src/main/java/com/autotune/utils/KruizeConstants.java

Signed-off-by: saakhan <saakhan@redhat.com>

dinogun · 2024-10-03T17:24:19Z

Closing stale PRs

add listClusters API

2380d56

Signed-off-by: saakhan <saakhan@redhat.com>

khansaad added the enhancement New feature or request label Jul 14, 2023

khansaad added this to the Autotune 0.0.8 Release milestone Jul 14, 2023

khansaad self-assigned this Jul 14, 2023

khansaad requested review from msvinaykumar and chandrams July 14, 2023 05:26

khansaad added 3 commits July 26, 2023 15:35

add summarize API(in-progress), add corr. API object, add common meth…

e7c13f2

…od in listRecomm API to fetch data in summarizeAPI, add converter method for summarizeAPI and other relevant changes Signed-off-by: saakhan <saakhan@redhat.com>

Merge remote-tracking branch 'upstream/mvp_demo' into add_cluster_apis

3f13a2b

# Conflicts: # src/main/java/com/autotune/analyzer/serviceObjects/Converters.java

fix NPE, add variation logic

07737af

Signed-off-by: saakhan <saakhan@redhat.com>

khansaad linked an issue Jul 26, 2023 that may be closed by this pull request

Add new APIs to support cluster and namespace level recommendations #892

Open

dinogun modified the milestones: Autotune 0.0.8 Release, Kruize 0.0.18_rm Release Jul 26, 2023

khansaad added 11 commits August 1, 2023 15:55

update logic to sum up the recommendations data, notifications-summar…

bc17b4b

…y and namespaces Signed-off-by: saakhan <saakhan@redhat.com>

add workloads class, validation for wrong params, refactoring, etc

1d2d8ed

Signed-off-by: saakhan <saakhan@redhat.com>

Merge remote-tracking branch 'upstream/mvp_demo' into add_cluster_apis

bdb91c2

update logic to fix the json response issue, temporarily fetching all…

4e8d755

… recomm and filtering data locally Signed-off-by: saakhan <saakhan@redhat.com>

update DB query to fetch data from the DB, fix duplicate records with…

6aea0d6

… same cluster name issue and other minor changes Signed-off-by: saakhan <saakhan@redhat.com>

Merge remote-tracking branch 'upstream/mvp_demo' into add_cluster_apis

bd8561e

# Conflicts: # src/main/java/com/autotune/database/dao/ExperimentDAO.java # src/main/java/com/autotune/database/dao/ExperimentDAOImpl.java # src/main/java/com/autotune/database/helper/DBConstants.java

updated logic to improve DB response time

0e440c2

Signed-off-by: saakhan <saakhan@redhat.com>

handle case when current config is missing, add more try-catch blocks

44152c5

Signed-off-by: saakhan <saakhan@redhat.com>

add ActionSummary object to classify the workloads, add 'fetchFromDB'…

912046c

… flag to force pull from the DB when required Signed-off-by: saakhan <saakhan@redhat.com>

Merge remote-tracking branch 'upstream/mvp_demo' into add_cluster_apis

7b6e852

# Conflicts: # src/main/java/com/autotune/database/dao/ExperimentDAOImpl.java # src/main/java/com/autotune/database/helper/DBConstants.java

rbadagandi1 modified the milestones: Kruize 0.0.18_rm Release, Kruize 0.0.19_rm Release Aug 23, 2023

khansaad added 2 commits August 24, 2023 00:08

Merge remote-tracking branch 'upstream/mvp_demo' into add_cluster_apis

9d5faad

# Conflicts: # src/main/java/com/autotune/analyzer/services/ListRecommendations.java

fix issues with variation and total DB count, API objects names. Adde…

66618a7

…d new error and info category for action summary Signed-off-by: saakhan <saakhan@redhat.com>

khansaad marked this pull request as ready for review August 28, 2023 06:50

msvinaykumar reviewed Aug 28, 2023

View reviewed changes

src/main/java/com/autotune/analyzer/services/ListClusters.java Outdated Show resolved Hide resolved

msvinaykumar reviewed Aug 28, 2023

View reviewed changes

src/main/java/com/autotune/analyzer/services/Summarize.java Show resolved Hide resolved

msvinaykumar reviewed Aug 28, 2023

View reviewed changes

src/main/java/com/autotune/database/dao/ExperimentDAOImpl.java Outdated Show resolved Hide resolved

msvinaykumar reviewed Aug 28, 2023

View reviewed changes

src/main/java/com/autotune/analyzer/services/Summarize.java Outdated Show resolved Hide resolved

msvinaykumar reviewed Aug 28, 2023

View reviewed changes

src/main/java/com/autotune/analyzer/services/Summarize.java Outdated Show resolved Hide resolved

msvinaykumar requested changes Aug 28, 2023

View reviewed changes

khansaad added 3 commits August 28, 2023 22:26

fix issues reported in testing related to caching and JSON structure

82ddd0c

Signed-off-by: saakhan <saakhan@redhat.com>

add missed javadocs for new classes and methods and other changes as …

1f1f8d0

…per the review Signed-off-by: saakhan <saakhan@redhat.com>

minor update to change info object name

6a68292

Signed-off-by: saakhan <saakhan@redhat.com>

dinogun reviewed Aug 31, 2023

View reviewed changes

rbadagandi1 modified the milestones: Kruize 0.0.19_rm Release, Kruize 0.0.20_rm Release, Kruize 0.0.21_rm Release Sep 4, 2023

khansaad added 3 commits September 6, 2023 18:44

Merge remote-tracking branch 'upstream/mvp_demo' into add_cluster_apis

33bcb0e

# Conflicts: # src/main/java/com/autotune/utils/KruizeConstants.java

Merge remote-tracking branch 'upstream/mvp_demo' into add_cluster_apis

62f5071

# Conflicts: # src/main/java/com/autotune/database/dao/ExperimentDAOImpl.java # src/main/java/com/autotune/database/helper/DBConstants.java # src/main/java/com/autotune/utils/KruizeConstants.java

changes made wrt the new listRecommendation API and minor refactoring

b189df5

Signed-off-by: saakhan <saakhan@redhat.com>

rbadagandi1 modified the milestones: Kruize 0.0.21_rm Release, Kruize 0.0.22_rm Release, Kruize 0.0.23_rm Release Nov 20, 2023

rbadagandi1 removed this from the Kruize 0.0.24_rm Release milestone Jul 10, 2024

dinogun closed this Oct 3, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add new APIs for recommendations summary at Clusters and Namespaces level #893

Add new APIs for recommendations summary at Clusters and Namespaces level #893

khansaad commented Jul 14, 2023

msvinaykumar Aug 28, 2023

msvinaykumar left a comment

msvinaykumar Aug 28, 2023

msvinaykumar Aug 28, 2023

msvinaykumar Aug 28, 2023

msvinaykumar Aug 28, 2023

msvinaykumar Aug 28, 2023

dinogun Aug 31, 2023

dinogun commented Oct 3, 2024

Add new APIs for recommendations summary at Clusters and Namespaces level #893

Add new APIs for recommendations summary at Clusters and Namespaces level #893

Conversation

khansaad commented Jul 14, 2023

msvinaykumar Aug 28, 2023

Choose a reason for hiding this comment

msvinaykumar left a comment

Choose a reason for hiding this comment

msvinaykumar Aug 28, 2023

Choose a reason for hiding this comment

msvinaykumar Aug 28, 2023

Choose a reason for hiding this comment

msvinaykumar Aug 28, 2023

Choose a reason for hiding this comment

msvinaykumar Aug 28, 2023

Choose a reason for hiding this comment

msvinaykumar Aug 28, 2023

Choose a reason for hiding this comment

dinogun Aug 31, 2023

Choose a reason for hiding this comment

dinogun commented Oct 3, 2024