SearchBackPressure Service Collector added #483

CoderJeffrey · 2023-06-19T18:20:38Z

Signed-off-by: Jeffrey Liu [email protected]

Is your feature request related to a problem? Please provide an existing Issue # , or describe.
Add the functionality to gather the SearchBackPressure Service.

Describe the solution you are proposing
Add SearchBackPressureStatsCollector.java to collect SearchBackPressure related metrics, and added Unit Testing for SearchBackPressureStatsCollector.java.

Describe alternatives you've considered
N/A

Additional context

Changes in licenses/ are auto-created after running ./build gradlew
In order to access the Constants field in this collector, another pull & request for performance-analyzer-commons package has been made.

Example:
@JsonProperty(SearchBackPressureStatsValue.Constants.SEARCHBP_NODEID) public String getSearchBackPressureStats_NodeId() { return this.nodeId; }

Parallel Pull & Request link: SearchbackpressureStatsCollector Constants Added performance-analyzer-commons#33

Approving this pull & request will be needed in order for the collector to access the Performance-Analyzer-Commons Constants fields. Or else, error would occur.
Can verify the extracted search_back_pressure metrics in /dev/shm/performanceanalyzer directory, open any log file, and search for "search_back_pressure"

Check List

New functionality includes testing.
All tests pass
New functionality has been documented.
New functionality has javadoc added
Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

…[email protected]) Signed-off-by: CoderJeffrey <[email protected]>

codecov · 2023-06-19T18:54:20Z

Codecov Report

Merging #483 (59a78bc) into main (5428a52) will increase coverage by 0.18%.
The diff coverage is 75.92%.

❗ Current head 59a78bc differs from pull request most recent head 1a0b4ed. Consider uploading reports for the commit 1a0b4ed to get more accurate results

@@             Coverage Diff              @@
##               main     #483      +/-   ##
============================================
+ Coverage     73.67%   73.85%   +0.18%     
- Complexity      375      381       +6     
============================================
  Files            44       45       +1     
  Lines          2477     2693     +216     
  Branches        172      172              
============================================
+ Hits           1825     1989     +164     
- Misses          544      596      +52     
  Partials        108      108

Impacted Files	Coverage Δ
...r/collectors/SearchBackPressureStatsCollector.java	`75.70% <75.70%> (ø)`
...performanceanalyzer/PerformanceAnalyzerPlugin.java	`77.14% <100.00%> (+0.21%)`	⬆️
...org/opensearch/performanceanalyzer/util/Utils.java	`92.50% <100.00%> (+0.19%)`	⬆️

…y: Jeffrey Liu [email protected]) Signed-off-by: CoderJeffrey <[email protected]>

CoderJeffrey · 2023-06-19T23:00:11Z

In order to access the Constants field in this collector, another pull & request for performance-analyzer-commons package has been made.

Example:
@JsonProperty(SearchBackPressureStatsValue.Constants.SEARCHBP_NODEID) public String getSearchBackPressureStats_NodeId() { return this.nodeId; }

Parallel Pull & Request link: opensearch-project/performance-analyzer-commons#33
Approving this pull & request will be needed in order for the collector to access the commons constant fields. Or else, error would occur.

getsaurabh02 · 2023-06-20T23:00:45Z

Can we cleanup the bunch of jars which got along with the commit making the file changes as 37?

ansjcy · 2023-06-21T00:10:34Z

...ain/java/org/opensearch/performanceanalyzer/collectors/SearchBackPressureStatsCollector.java

+        private SearchShardTaskStats searchShardTaskStats;
+        private SearchTaskStats searchTaskStats;
+
+        // private double searchBackPressureStatsTest;


Is this searchBackPressureStatsTest for testing?

// private double searchBackPressureStatsTest; is not needed for the collector.
thank you so much for pointing out, just removed this piece of code (comment).

ansjcy · 2023-06-21T00:13:08Z

...ain/java/org/opensearch/performanceanalyzer/collectors/SearchBackPressureStatsCollector.java

+            this.searchbp_task_stats_cancellationCount = searchTaskStats.getCancellationCount();
+            this.searchbp_task_stats_limitReachedCount = searchTaskStats.getLimitReachedCount();
+            this.searchbp_task_stats_resource_heap_usage_cancellationCount =
+                    searchTaskStats


We can probably rewrite this part of code to make it more readable, for example, using some temp variables to replace

searchTaskStats .getResourceUsageTrackerStats() .get(HEAP_USAGE_TRACKER_FIELD_NAME) searchTaskStats .getResourceUsageTrackerStats() .get(CPU_USAGE_TRACKER_FIELD_NAME) searchTaskStats .getResourceUsageTrackerStats() .get(ELAPSED_TIME_USAGE_TRACKER_FIELD_NAME)

searchTaskStats .getResourceUsageTrackerStats() .get(HEAP_USAGE_TRACKER_FIELD_NAME)

Has now been simplified to
task_heap_stats

Similar simplification applies to other variables.

ansjcy · 2023-06-21T00:17:10Z

...ava/org/opensearch/performanceanalyzer/collectors/SearchBackPressureStatsCollectorTests.java

+import org.powermock.modules.junit4.PowerMockRunner;
+
+@RunWith(PowerMockRunner.class)
+public class SearchBackPressureStatsCollectorTests {


Besides the valid cases, we can add more tests for some invalid scenarios. For example, to test if an exception is correctly thrown under certain conditions.

In order to check for the exception for collectMetrics(), it is required to control the behavior of ObjectMapper, but since it is a private static field, and all exceptions are caught inside the collectMetrics() itself. Is it acceptable to only test for valid fields for now? i also check with other collect tests like ClusterManagerServiceEventMetricsTests.java, the additional exception testings are also omitted.

…com) Signed-off-by: CoderJeffrey <[email protected]>

…[email protected]) Signed-off-by: CoderJeffrey <[email protected]>

CoderJeffrey · 2023-06-21T05:07:10Z

Can we cleanup the bunch of jars which got along with the commit making the file changes as 37?

Just cleaned up all jars.

…d-off-by: Jeffrey Liu [email protected]) Signed-off-by: CoderJeffrey <[email protected]>

ansjcy

LGTM!

getsaurabh02 · 2023-06-23T18:41:26Z

...ain/java/org/opensearch/performanceanalyzer/collectors/SearchBackPressureStatsCollector.java

+import org.opensearch.search.backpressure.SearchBackpressureService;
+import org.opensearch.search.backpressure.stats.SearchShardTaskStats;
+import org.opensearch.search.backpressure.stats.SearchTaskStats;
+


nit: java doc

More Java doc has been added for SearchBackPressureStatsCollector and SearchBackPressureStatsCollectorTests

…y: Jeffrey Liu [email protected]) Signed-off-by: CoderJeffrey <[email protected]>

* SearchBackPressure Collector added Signed-off-by: CoderJeffrey <[email protected]> (cherry picked from commit e0ea1da)

* SearchBackPressure Collector added Signed-off-by: CoderJeffrey <[email protected]> (cherry picked from commit e0ea1da) Co-authored-by: Jeffrey Liu <[email protected]>

…t#483) (opensearch-project#558)" Signed-off-by: Khushboo Rajput <[email protected]>

Signed-off-by: Khushboo Rajput <[email protected]>

Signed-off-by: Khushboo Rajput <[email protected]> (cherry picked from commit fbef6f9)

…) (#570)

SearchBackPressure Collector added (Signed-off-by: Jeffrey Liu ujeffl…

861d738

…[email protected]) Signed-off-by: CoderJeffrey <[email protected]>

CoderJeffrey requested review from praveensameneni, khushbr, ansjcy, goyamegh, getsaurabh02, sbcd90 and sgup432 as code owners June 19, 2023 18:20

Replace String fields with Constants in Commons Package (Signed-off-b…

d222551

…y: Jeffrey Liu [email protected]) Signed-off-by: CoderJeffrey <[email protected]>

CoderJeffrey mentioned this pull request Jun 19, 2023

SearchbackpressureStatsCollector Constants Added opensearch-project/performance-analyzer-commons#33

Merged

5 tasks

CoderJeffrey mentioned this pull request Jun 20, 2023

[FEATURE] SearchBackPressure Reader opensearch-project/performance-analyzer-rca#424

Closed

ansjcy reviewed Jun 21, 2023

View reviewed changes

CoderJeffrey added 2 commits June 20, 2023 21:57

Remove jar files changes (Signed-off-by: Jeffrey Liu ujeffliu@amazon.…

1d17ee8

…com) Signed-off-by: CoderJeffrey <[email protected]>

Remove licenses/ jar files changes (Signed-off-by: Jeffrey Liu ujeffl…

5348a9f

…[email protected]) Signed-off-by: CoderJeffrey <[email protected]>

Refactor populate_task_stats() and populate_shard_task_stats() (Signe…

d451e22

…d-off-by: Jeffrey Liu [email protected]) Signed-off-by: CoderJeffrey <[email protected]>

This was referenced Jun 22, 2023

Searchbackpressure Service Reader Added opensearch-project/performance-analyzer-rca#426

Closed

Searchbackpressure Service Reader and UTs added opensearch-project/performance-analyzer-rca#427

Merged

ansjcy previously approved these changes Jun 22, 2023

View reviewed changes

getsaurabh02 previously approved these changes Jun 23, 2023

View reviewed changes

Add more Javadoc for SearchBackPressureCollector and UT (Signed-off-b…

1a0b4ed

…y: Jeffrey Liu [email protected]) Signed-off-by: CoderJeffrey <[email protected]>

CoderJeffrey dismissed stale reviews from getsaurabh02 and ansjcy via 1a0b4ed June 23, 2023 20:21

CoderJeffrey requested review from getsaurabh02 and ansjcy June 23, 2023 21:27

ansjcy approved these changes Jun 23, 2023

View reviewed changes

sbcd90 approved these changes Jun 24, 2023

View reviewed changes

sbcd90 merged commit e0ea1da into opensearch-project:main Jun 24, 2023

CoderJeffrey mentioned this pull request Jul 13, 2023

SearchBackPressure Policy/Decider Generic Framework CoderJeffrey/performance-analyzer-rca#2

Open

5 tasks

CoderJeffrey mentioned this pull request Jul 21, 2023

SearchBackPressure Policy/Decider Generic Framework Added opensearch-project/performance-analyzer-rca#461

Merged

5 tasks

khushbr added backport 2.x v2.11.0 Issues targeting release v2.11.0 labels Oct 5, 2023

opensearch-trigger-bot bot pushed a commit that referenced this pull request Oct 5, 2023

SearchBackPressure Service Collector added (#483)

0271ce8

* SearchBackPressure Collector added Signed-off-by: CoderJeffrey <[email protected]> (cherry picked from commit e0ea1da)

opensearch-trigger-bot bot mentioned this pull request Oct 5, 2023

[Backport 2.x] SearchBackPressure Service Collector added #558

Merged

peterzhuamazon mentioned this pull request Oct 5, 2023

[RELEASE] Release version 2.11.0 opensearch-project/opensearch-build#3998

Closed

71 tasks

khushbr added a commit to khushbr/performance-analyzer that referenced this pull request Oct 6, 2023

Revert "SearchBackPressure Service Collector added (opensearch-projec…

0d9ccf8

…t#483) (opensearch-project#558)" Signed-off-by: Khushboo Rajput <[email protected]>

khushbr added a commit that referenced this pull request Oct 6, 2023

Revert "SearchBackPressure Service Collector added (#483) (#558)" (#568)

fbef6f9

Signed-off-by: Khushboo Rajput <[email protected]>

opensearch-trigger-bot bot pushed a commit that referenced this pull request Oct 6, 2023

Revert "SearchBackPressure Service Collector added (#483) (#558)" (#568)

5cd870b

Signed-off-by: Khushboo Rajput <[email protected]> (cherry picked from commit fbef6f9)

ansjcy pushed a commit that referenced this pull request Oct 6, 2023

Revert "SearchBackPressure Service Collector added (#483) (#558)" (#568…

305194e

…) (#570)

ruai0511 mentioned this pull request Oct 10, 2023

Add Search Back Pressure PA metrics opensearch-project/documentation-website#5114

Closed

1 task

hdhalter mentioned this pull request Oct 10, 2023

Add new Performance Analyzer metrics and fix table formatting opensearch-project/documentation-website#5182

Merged

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

SearchBackPressure Service Collector added #483

SearchBackPressure Service Collector added #483

CoderJeffrey commented Jun 19, 2023 •

edited

Loading

codecov bot commented Jun 19, 2023 •

edited

Loading

CoderJeffrey commented Jun 19, 2023 •

edited

Loading

getsaurabh02 commented Jun 20, 2023

ansjcy Jun 21, 2023

CoderJeffrey Jun 21, 2023

ansjcy Jun 21, 2023

CoderJeffrey Jun 21, 2023

ansjcy Jun 21, 2023

CoderJeffrey Jun 21, 2023 •

edited

Loading

CoderJeffrey commented Jun 21, 2023

ansjcy left a comment

getsaurabh02 Jun 23, 2023

CoderJeffrey Jun 23, 2023

SearchBackPressure Service Collector added #483

SearchBackPressure Service Collector added #483

Conversation

CoderJeffrey commented Jun 19, 2023 • edited Loading

Check List

codecov bot commented Jun 19, 2023 • edited Loading

Codecov Report

CoderJeffrey commented Jun 19, 2023 • edited Loading

getsaurabh02 commented Jun 20, 2023

ansjcy Jun 21, 2023

Choose a reason for hiding this comment

CoderJeffrey Jun 21, 2023

Choose a reason for hiding this comment

ansjcy Jun 21, 2023

Choose a reason for hiding this comment

CoderJeffrey Jun 21, 2023

Choose a reason for hiding this comment

ansjcy Jun 21, 2023

Choose a reason for hiding this comment

CoderJeffrey Jun 21, 2023 • edited Loading

Choose a reason for hiding this comment

CoderJeffrey commented Jun 21, 2023

ansjcy left a comment

Choose a reason for hiding this comment

getsaurabh02 Jun 23, 2023

Choose a reason for hiding this comment

CoderJeffrey Jun 23, 2023

Choose a reason for hiding this comment

CoderJeffrey commented Jun 19, 2023 •

edited

Loading

codecov bot commented Jun 19, 2023 •

edited

Loading

CoderJeffrey commented Jun 19, 2023 •

edited

Loading

CoderJeffrey Jun 21, 2023 •

edited

Loading