SearchBackPressure Policy/Decider Generic Framework

[CoderJeffrey]

Is your feature request related to a problem? Please provide an existing Issue # , or describe.

Previous work related to this PR:
For Search Back Pressure Service, previous PR (opensearch-project/performance-analyzer#483) (opensearch-project#427) (opensearch-project#437) have established a pipeline using Collector to read SearchBackPressure Cancellation Stats, using Reader to consume the metrics and populate to the sqlitedb, using Cluster/Node level RCA to consume stats in sqlitedb and compare stats with thresholds to generate Healthy/Unhealthy Flow Unit.

Problem:
We nee a Policy/Decider framework to suggest actions based on cluster level RCAs to tune the thresholds for SearchBackPressure Service.

Describe the solution you are proposing
This PR added the Policy/Decider Framework that consumed the cluster level RCA and suggestion actions to downstream classes (e.g. Users).

Task Breakdown:

• Policy Class to determine when we need to suggest actions to user to auto-tune the thresholds for Search Back Pressure Service.
• Decider Class to consume the cluster-level RCA and use the Policy to send the actions to downstream classes.
• PolicyConfig file to read key thresholds/settings from Config files to improve extensibility for SearchBackPressureDecider.
• (Generic) Action Class for actions that will be sent to downstream classes to suggest user to tune the thresholds.

Describe alternatives you've considered
N/A

Additional context
Currently, the framework only suggest actions to tune heap related thresholds for Search Back Pressure Service.
Final policy logic to determine when to send out actions to users have not been decided yet. This PR mainly offers an extensible Decider/Policy framework.

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.

[kaushalmahi12]

I think keeping the var name as INCREASE should be sufficient.

[CoderJeffrey]

changed to INCREASE

[kaushalmahi12]

I think we don't need these percentages individually for all the heap related settings. We can keep a single value which implies the percentage increase in all of the heap related SBP settings.

[CoderJeffrey]

    private static final String SEARCHBP_HEAP_STEPSIZE_IN_PERCENTAGE =
            "searchbp-heap-stepsize-in-percentage";

is now the value to imply the percentage increase in all of heap related SBP settings. Comments (javadoc) alsol changed accordingly.

[kaushalmahi12]

This will be redundant.

[CoderJeffrey]

removed and now getMonitorWindowSizeMinutes() and getHourMonitorBucketSizeMinutes() return default values.

public int getHourMonitorWindowSizeMinutes() {
        return DEFAULT_HOUR_MONITOR_WINDOW_SIZE_MINUTES;
    }

    public int getHourMonitorBucketSizeMinutes() {
        return DEFAULT_HOUR_MONITOR_BUCKET_SIZE_MINUTES;
    }

[kaushalmahi12]

This information is redundant since we are not using it anywhere, I would suggest to remove this member variable from the class.

[CoderJeffrey]

The currentIteration makes sure new decision will be emitted only per period

[kaushalmahi12]

I think we can refactor the nested if blocks into meaningful methods here. e,g; recordShardTaskIssue

[kaushalmahi12]

Why are we logging this here ?

[CoderJeffrey]

removed.

[kaushalmahi12]

This should be an enum to limit the possible values for this var.

[CoderJeffrey]

Do you mean an enum like this? but in the constructor, the field should still be a String?

public enum ThresholdName {
     HEAP_THRESHOLD("searchbp-heap-threshold"),
     // ...

     private final String value;
     ThresholdName(final String value) {
         this.value = value;
     }
     @Override
     public String toString() {
         return value;
     }
 }

[kaushalmahi12]

Shard and SearchTask related breach should be done separately to avoid a potential bug when both of them needs to be adjusted.

[kaushalmahi12]

This logic can be refactored into a lambda function and have that lambda as a member variable.

[kaushalmahi12]

Lets discuss about these