feat(wlm): Add additional search settings and override_request_values to WLM groups

[dzane17]

Description

Adds additional WLM search settings and introduces override_request_values to control how workload group settings interact with request-level parameters.

New search settings

Workload groups can now configure the following settings in addition to the existing search.default_search_timeout:

• search.cancel_after_time_interval — time after which a search request is cancelled
• search.max_concurrent_shard_requests — max concurrent shard requests per node (>= 1)
• search.batched_reduce_size — number of shard results to reduce at once (>= 2)
• override_request_values — boolean (default false) controlling whether WLM settings override explicitly set request-level values

Why override_request_values is needed

OpenSearch convention is that nothing should override request-level parameters — they represent the caller's explicit intent. However, WLM groups are configured by system administrators to protect the cluster. There should be a mechanism for WLM group settings to take precedence over a potentially malicious or uninformed individual who sets aggressive request parameters (e.g., unbounded timeouts or excessive concurrency).

At the same time, WLM group settings should not immediately override request values by default because that can cause unpredictable response behavior which breaks existing clients. Therefore override_request_values defaults to false — admins must explicitly opt in to the override behavior after understanding the impact on their workloads.

Settings update semantics

Settings use merge semantics on update:

• Settings key absent from request — existing settings preserved
• "settings": {} or "settings": null — clears all search settings and sets override_request_values to false
• "settings": {"key": "value"} — merges with existing (adds/updates the key)
• "settings": {"key": null} — removes that specific key from existing settings

The override_request_values field is always present in the GET response (defaults to "false" when not explicitly set).

Related Issues

Part of #20555

Check List

• Functionality includes testing.
• API changes companion pull request created, if applicable.
• Public documentation issue/PR created, if applicable.

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.

[github-actions]

PR Reviewer Guide 🔍

(Review updated until commit 6a38df0)

Here are some key observations to aid the review process:

🧪 PR contains tests

🔒 No security concerns identified

✅ No TODO sections

🔀 No multiple PR themes

⚡ Recommended focus areas for review Possible Issue When override_request_values is false and a user explicitly sets batched_reduce_size to 512 (the default), WLM will still override it because the code cannot distinguish between "not set" and "explicitly set to 512". This violates the documented behavior that request-level parameters represent explicit intent and should not be overridden when override_request_values is false. private void applyBatchedReduceSize(Settings wlmSettings, SearchRequest searchRequest, boolean overrideRequestValues) { if (wlmSettings.hasValue(WorkloadGroupSearchSettings.WLM_BATCHED_REDUCE_SIZE.getKey()) == false) { return; } try { int batchedReduceSize = WorkloadGroupSearchSettings.WLM_BATCHED_REDUCE_SIZE.get(wlmSettings); // Only apply WLM batched reduce size when the request uses the default value. // Note: batchedReduceSize is a primitive int with no sentinel value, so we cannot // distinguish between "not set" and "explicitly set to 512 (the default)". If a user // explicitly sets batched_reduce_size=512, WLM will still override it when // override_request_values is false. if (overrideRequestValues || searchRequest.getBatchedReduceSize() == SearchRequest.DEFAULT_BATCHED_REDUCE_SIZE) { searchRequest.setBatchedReduceSize(batchedReduceSize); }

[github-actions]

PR Code Suggestions ✨

Latest suggestions up to 6a38df0

Explore these optional code suggestions:

Category	Suggestion	Impact
General	Preserve exception cause in validation The validation logic allows null values to bypass type validation, but Settings.builder().put(key, value) will throw an exception if value is null. This creates an inconsistency where null values are intended to clear settings but may cause unexpected errors during validation. server/src/main/java/org/opensearch/wlm/WorkloadGroupSearchSettings.java [110-115] for (String key : settings.keySet()) { String value = settings.get(key); Setting<?> setting = REGISTERED_SETTINGS.get(key); if (setting == null) { throw new IllegalArgumentException("Unknown WLM setting: " + key); } // null value means "clear this setting" — skip type validation if (value == null) { continue; } try { Settings testSettings = Settings.builder().put(key, value).build(); setting.get(testSettings); } catch (Exception e) { - throw new IllegalArgumentException("Invalid value '" + value + "' for " + key + ": " + e.getMessage()); + throw new IllegalArgumentException("Invalid value '" + value + "' for " + key + ": " + e.getMessage(), e); } } Suggestion importance[1-10]: 3 __ Why: The suggestion to preserve the exception cause by adding e to the IllegalArgumentException constructor is a minor improvement for debugging. However, the concern about Settings.builder().put(key, value) throwing an exception when value is null is incorrect—the code explicitly checks for null and skips validation before calling put(). The improvement is valid but has minimal impact.	Low
	Clarify batched reduce size override logic The logic cannot distinguish between a user explicitly setting batched_reduce_size=512 and the default value. When override_request_values=false, WLM will incorrectly override an explicit user value of 512, violating the intended behavior where user values should be preserved. server/src/main/java/org/opensearch/wlm/listeners/WorkloadGroupRequestOperationListener.java [136-152] private void applyBatchedReduceSize(Settings wlmSettings, SearchRequest searchRequest, boolean overrideRequestValues) { if (wlmSettings.hasValue(WorkloadGroupSearchSettings.WLM_BATCHED_REDUCE_SIZE.getKey()) == false) { return; } try { int batchedReduceSize = WorkloadGroupSearchSettings.WLM_BATCHED_REDUCE_SIZE.get(wlmSettings); - // Only apply WLM batched reduce size when the request uses the default value. - // Note: batchedReduceSize is a primitive int with no sentinel value, so we cannot - // distinguish between "not set" and "explicitly set to 512 (the default)". If a user - // explicitly sets batched_reduce_size=512, WLM will still override it when - // override_request_values is false. - if (overrideRequestValues || searchRequest.getBatchedReduceSize() == SearchRequest.DEFAULT_BATCHED_REDUCE_SIZE) { + // When override is false, only apply if request uses default. Note: we cannot + // distinguish "not set" from "explicitly set to default (512)", so WLM may + // override explicit user values that match the default. + if (overrideRequestValues) { + searchRequest.setBatchedReduceSize(batchedReduceSize); + } else if (searchRequest.getBatchedReduceSize() == SearchRequest.DEFAULT_BATCHED_REDUCE_SIZE) { searchRequest.setBatchedReduceSize(batchedReduceSize); } } catch (Exception e) { logger.error("Failed to apply workload group setting [search.batched_reduce_size]", e); } } Suggestion importance[1-10]: 2 __ Why: The suggestion restructures the conditional logic but doesn't change the behavior or fix any issue. The existing code already handles the logic correctly with the same semantics. The comment in the existing code already acknowledges the limitation about distinguishing explicit vs. default values. This is a stylistic change with no functional improvement.	Low

---

Previous suggestions

Suggestions up to commit 5bdf642

Category	Suggestion	Impact
General	Cannot distinguish default from explicit values The logic for applyBatchedReduceSize has a flaw: when overrideRequestValues is false and a user explicitly sets batched_reduce_size=512 (the default), WLM will incorrectly override it. This violates the expected behavior where user-provided values should be preserved when override is disabled. Consider adding a raw accessor method similar to getMaxConcurrentShardRequestsRaw() to distinguish between default and explicitly-set values. server/src/main/java/org/opensearch/wlm/listeners/WorkloadGroupRequestOperationListener.java [136-152] private void applyBatchedReduceSize(Settings wlmSettings, SearchRequest searchRequest, boolean overrideRequestValues) { if (wlmSettings.hasValue(WorkloadGroupSearchSettings.WLM_BATCHED_REDUCE_SIZE.getKey()) == false) { return; } try { int batchedReduceSize = WorkloadGroupSearchSettings.WLM_BATCHED_REDUCE_SIZE.get(wlmSettings); - // Only apply WLM batched reduce size when the request uses the default value. - // Note: batchedReduceSize is a primitive int with no sentinel value, so we cannot - // distinguish between "not set" and "explicitly set to 512 (the default)". If a user - // explicitly sets batched_reduce_size=512, WLM will still override it when - // override_request_values is false. + // TODO: Add getBatchedReduceSizeRaw() method to SearchRequest to properly detect + // when the value is explicitly set vs. using the default if (overrideRequestValues || searchRequest.getBatchedReduceSize() == SearchRequest.DEFAULT_BATCHED_REDUCE_SIZE) { searchRequest.setBatchedReduceSize(batchedReduceSize); } } catch (Exception e) { logger.error("Failed to apply workload group setting [search.batched_reduce_size]", e); } } Suggestion importance[1-10]: 5 __ Why: The suggestion correctly identifies a limitation where batched_reduce_size=512 (the default) cannot be distinguished from an unset value. However, the code already documents this limitation in comments (lines 143-146), and the suggestion only adds a TODO without providing a concrete solution.	Low

Suggestions up to commit b01f234

Category	Suggestion	Impact
Possible issue	Verify null key iteration behavior The merge logic iterates over mutableFragmentSettings.keySet() and checks if value is null. However, Settings.keySet() typically excludes keys with null values, so the null check may never be triggered. Verify that null-valued keys are actually present in the keyset or use an alternative approach to detect removal intent. server/src/main/java/org/opensearch/cluster/metadata/WorkloadGroup.java [130-141] +} else { + // Merge: start with existing settings, overlay new values, remove null-valued keys + Settings.Builder builder = Settings.builder().put(existingGroup.getSettings()); + for (String key : mutableFragmentSettings.keySet()) { + String value = mutableFragmentSettings.get(key); + if (value == null) { + // null value means "clear this setting" + builder.remove(key); + } else { + builder.put(key, value); + } + } + updatedSettings = builder.build(); +} - Suggestion importance[1-10]: 8 __ Why: This is a valid concern. The Settings.keySet() behavior with null values needs verification. If keySet() excludes null-valued keys, the removal logic would never execute, breaking the intended "clear this setting" functionality. This could be a critical bug affecting the merge semantics.	Medium

Suggestions up to commit 368789a

Category	Suggestion	Impact
General	Fragile sentinel value exposed in public API The sentinel value 0 for "not explicitly set" is an internal implementation detail of SearchRequest that is now being exposed as part of a public (experimental) API contract. If the internal default sentinel ever changes (e.g., to -1), the WLM logic in applyMaxConcurrentShardRequests that checks == 0 would silently break. Consider documenting this contract more explicitly or adding a dedicated isMaxConcurrentShardRequestsSet() boolean method to make the "not set" check more robust and less coupled to the internal sentinel value. server/src/main/java/org/opensearch/action/search/SearchRequest.java [657-660] +/** + * Returns whether maxConcurrentShardRequests has been explicitly set by the user. + * Returns {@code false} if the field holds the sentinel value {@code 0} (not set). + */ +@ExperimentalApi +public boolean isMaxConcurrentShardRequestsSet() { + return maxConcurrentShardRequests != 0; +} + +/** + * Returns the raw value of maxConcurrentShardRequests without applying the default. + * A value of {@code 0} means the user has not explicitly set this parameter. + */ @ExperimentalApi public int getMaxConcurrentShardRequestsRaw() { return maxConcurrentShardRequests; } Suggestion importance[1-10]: 5 __ Why: Adding a dedicated isMaxConcurrentShardRequestsSet() method is a valid API design improvement that decouples the WLM logic from the internal sentinel value 0. This makes the "not set" check more robust and self-documenting, reducing the risk of silent breakage if the sentinel changes.	Low
	Dead code guard may mask future bugs The WLM_OVERRIDE_REQUEST_VALUES setting is always injected into the group's settings by the WorkloadGroup constructor, so wlmSettings.isEmpty() will never be true when override_request_values is present. However, if wlmSettings only contains override_request_values and no actual search settings, the early-return guard wlmSettings.isEmpty() would incorrectly skip applying settings. More critically, calling WLM_OVERRIDE_REQUEST_VALUES.get(wlmSettings) when the key is absent (e.g., legacy data) will use the default, which is fine, but the early return on wlmSettings.isEmpty() means a group with only override_request_values=true and no other settings would still return early — this is acceptable. The real issue is that the guard wlmSettings.isEmpty() will never trigger now since override_request_values is always injected, making the guard dead code and potentially masking future bugs. Consider removing the isEmpty() check or adjusting it to check for actual search settings. server/src/main/java/org/opensearch/wlm/listeners/WorkloadGroupRequestOperationListener.java [82] +boolean overrideRequestValues = WorkloadGroupSearchSettings.WLM_OVERRIDE_REQUEST_VALUES.get(wlmSettings); - Suggestion importance[1-10]: 3 __ Why: The suggestion correctly identifies that wlmSettings.isEmpty() is effectively dead code since override_request_values is always injected by the constructor. However, the improved_code is identical to the existing_code, making this more of an observation than an actionable fix. The impact is low as the current behavior is still correct.	Low
	Fragile reliance on constructor side-effect when clearing settings When mutableFragmentSettings is empty (explicit clear), updatedSettings is set to Settings.EMPTY. However, the WorkloadGroup constructor will then re-inject override_request_values=false into the normalized settings. This means clearing settings via an empty object will always result in override_request_values=false being present, which is the intended behavior per the test testUpdateWithEmptySettingsClearsExisting. This is consistent, but it's worth noting that passing Settings.EMPTY here relies on the constructor normalization — if the constructor logic changes, this could break. The current code is correct but fragile; consider explicitly building the settings with only the default override_request_values when clearing, to make the intent explicit. server/src/main/java/org/opensearch/cluster/metadata/WorkloadGroup.java [137-140] } else if (mutableFragmentSettings.isEmpty()) { - // Explicitly empty - clear all settings + // Explicitly empty - clear all search settings (override_request_values will be re-injected by constructor) updatedSettings = Settings.EMPTY; } else { Suggestion importance[1-10]: 2 __ Why: The suggestion only proposes a comment change to the existing_code, and the improved_code is functionally identical to the existing_code. While the observation about fragility is valid, the suggestion offers no concrete code improvement beyond a comment clarification.	Low

Suggestions up to commit 8e64571

Category	Suggestion	Impact
General	Add raw accessor to detect unset batched reduce size The same ambiguity problem exists for maxConcurrentShardRequests — the raw value 0 is used as a sentinel for "not set", but if the WLM setting value equals 5 (the default returned by getMaxConcurrentShardRequests()), a user who explicitly set max_concurrent_shard_requests=5 would have their value preserved (since raw != 0). However, if the WLM setting is also 5, the behavior is correct by coincidence. The getMaxConcurrentShardRequestsRaw() approach is sound, but the same sentinel approach should be applied to batchedReduceSize for consistency. Consider adding a getBatchedReduceSizeRaw() method (returning the raw field before default substitution) to SearchRequest to properly detect "not explicitly set" for batchedReduceSize, similar to getMaxConcurrentShardRequestsRaw(). server/src/main/java/org/opensearch/wlm/listeners/WorkloadGroupRequestOperationListener.java [136-152] -private void applyBatchedReduceSize(Settings wlmSettings, SearchRequest searchRequest, boolean overrideRequestValues) { - if (wlmSettings.hasValue(WorkloadGroupSearchSettings.WLM_BATCHED_REDUCE_SIZE.getKey()) == false) { - return; - } - try { - int batchedReduceSize = WorkloadGroupSearchSettings.WLM_BATCHED_REDUCE_SIZE.get(wlmSettings); - // Only apply WLM batched reduce size when the request uses the default value. - // Note: batchedReduceSize is a primitive int with no sentinel value, so we cannot - // distinguish between "not set" and "explicitly set to 512 (the default)". If a user - // explicitly sets batched_reduce_size=512, WLM will still override it when - // override_request_values is false. - if (overrideRequestValues || searchRequest.getBatchedReduceSize() == SearchRequest.DEFAULT_BATCHED_REDUCE_SIZE) { - searchRequest.setBatchedReduceSize(batchedReduceSize); - } +// In SearchRequest.java, add: +@ExperimentalApi +public int getBatchedReduceSizeRaw() { + return batchedReduceSize; // returns 0 or actual value before default substitution +} +// Then in applyBatchedReduceSize: +if (overrideRequestValues || searchRequest.getBatchedReduceSizeRaw() == 0) { + searchRequest.setBatchedReduceSize(batchedReduceSize); +} + Suggestion importance[1-10]: 6 __ Why: This is a valid correctness concern — the current batchedReduceSize detection using DEFAULT_BATCHED_REDUCE_SIZE as a sentinel cannot distinguish between "not set" and "explicitly set to default". The improved_code correctly proposes adding a getBatchedReduceSizeRaw() method similar to getMaxConcurrentShardRequestsRaw(), though the implementation assumes batchedReduceSize field defaults to 0 which needs verification.	Low
	Use consistent naming prefix for settings The override_request_values key does not have the search. prefix unlike all other WLM search settings. This inconsistency could cause confusion and potential conflicts with other non-search settings in the future. Consider using a consistent naming convention such as search.override_request_values to make it clear this setting belongs to the search settings namespace. server/src/main/java/org/opensearch/wlm/WorkloadGroupSearchSettings.java [71] -public static final Setting<Boolean> WLM_OVERRIDE_REQUEST_VALUES = Setting.boolSetting("override_request_values", false); +public static final Setting<Boolean> WLM_OVERRIDE_REQUEST_VALUES = Setting.boolSetting("search.override_request_values", false); Suggestion importance[1-10]: 5 __ Why: The inconsistency in naming (override_request_values vs search.* prefix) is a valid concern for API consistency and future maintainability. However, this is a design decision that would require changes across many files and tests, and the suggestion doesn't account for the cascading impact.	Low
	Skip apply methods when no search settings exist The WLM_OVERRIDE_REQUEST_VALUES setting is always injected into the group's settings by the WorkloadGroup constructor, so wlmSettings.isEmpty() will never be true when override_request_values is present. However, if wlmSettings somehow doesn't contain the key, Setting.get() will return the default (false), which is safe. The real issue is the early return on wlmSettings.isEmpty() — since override_request_values is always injected, a group with only that key will have non-empty settings, but none of the applyXxx methods will find their keys and will all return early, which is correct. This logic is fine, but the guard wlmSettings.isEmpty() will now never short-circuit for groups that only have override_request_values. Consider checking whether any search settings are present before calling the apply methods, to avoid unnecessary work. server/src/main/java/org/opensearch/wlm/listeners/WorkloadGroupRequestOperationListener.java [82-87] boolean overrideRequestValues = WorkloadGroupSearchSettings.WLM_OVERRIDE_REQUEST_VALUES.get(wlmSettings); +// Only proceed if at least one search setting (beyond override_request_values) is present +boolean hasSearchSettings = wlmSettings.keySet().stream() + .anyMatch(k -> !k.equals(WorkloadGroupSearchSettings.WLM_OVERRIDE_REQUEST_VALUES.getKey())); +if (!hasSearchSettings) { + return; +} + +applyTimeout(wlmSettings, searchRequest, overrideRequestValues); +applyCancelAfterTimeInterval(wlmSettings, searchRequest, overrideRequestValues); +applyMaxConcurrentShardRequests(wlmSettings, searchRequest, overrideRequestValues); +applyBatchedReduceSize(wlmSettings, searchRequest, overrideRequestValues); + Suggestion importance[1-10]: 3 __ Why: The suggestion is logically sound but offers only a minor optimization. Each applyXxx method already checks hasValue() and returns early, so the additional stream check adds complexity without meaningful benefit. The improved_code also changes the structure of the existing code rather than just adding the guard.	Low

Suggestions up to commit 42afadf

Category	Suggestion	Impact
Possible issue	Use safe fallback for boolean setting retrieval The WLM_OVERRIDE_REQUEST_VALUES setting is always injected into the group's settings by the WorkloadGroup constructor, so wlmSettings.isEmpty() will never be true for a properly constructed group that has override_request_values set. However, if wlmSettings only contains override_request_values and no actual search settings, the early return on wlmSettings.isEmpty() would correctly skip processing. The real issue is that WLM_OVERRIDE_REQUEST_VALUES.get(wlmSettings) will throw if the key is absent (e.g., for legacy persisted groups), so you should use wlmSettings.getAsBoolean with a default fallback to be safe. server/src/main/java/org/opensearch/wlm/listeners/WorkloadGroupRequestOperationListener.java [82] -boolean overrideRequestValues = WorkloadGroupSearchSettings.WLM_OVERRIDE_REQUEST_VALUES.get(wlmSettings); +boolean overrideRequestValues = wlmSettings.getAsBoolean( + WorkloadGroupSearchSettings.WLM_OVERRIDE_REQUEST_VALUES.getKey(), false +); Suggestion importance[1-10]: 5 __ Why: The suggestion raises a valid concern about legacy persisted groups that may not have override_request_values injected. Using getAsBoolean with a default fallback is safer than Setting.get() which could throw for absent keys. However, the constructor normalization should handle most cases, making this a minor defensive improvement.	Low
General	Skip processing when no search settings are configured Since the WorkloadGroup constructor always injects override_request_values into settings, wlmSettings will never be truly empty for a valid group — it will always contain at least override_request_values. This means the wlmSettings.isEmpty() guard will never trigger, and the method will always proceed to call all four apply methods even when no actual search settings are configured. Consider checking whether any actual search settings (beyond override_request_values) are present before proceeding.* server/src/main/java/org/opensearch/wlm/listeners/WorkloadGroupRequestOperationListener.java [78-80] if (wlmSettings == null || wlmSettings.isEmpty()) { return; } +// Check if any actionable search settings are present (not just override_request_values) +boolean hasSearchSettings = wlmSettings.keySet().stream() + .anyMatch(k -> !k.equals(WorkloadGroupSearchSettings.WLM_OVERRIDE_REQUEST_VALUES.getKey())); +if (!hasSearchSettings) { + return; +} Suggestion importance[1-10]: 4 __ Why: The suggestion correctly identifies that wlmSettings.isEmpty() will never be true since the constructor always injects override_request_values. Adding a check for actionable settings avoids unnecessary method calls, but the individual apply* methods already guard with hasValue checks, making this an optimization rather than a correctness fix.	Low
	Clarify comment about settings clearing behavior When settings are explicitly cleared with an empty object, updatedSettings is set to Settings.EMPTY, but the WorkloadGroup constructor will then re-inject override_request_values with its default value. This is the intended behavior per the test testUpdateWithEmptySettingsClearsExisting, but the comment says "clear all settings" which is misleading — it should note that override_request_values will be re-added by the constructor. More critically, the merge branch does not ensure override_request_values is preserved if it was explicitly removed via a null value in the update fragment, which could leave the group without the required field until the constructor re-normalizes it. server/src/main/java/org/opensearch/cluster/metadata/WorkloadGroup.java [137-140] } else if (mutableFragmentSettings.isEmpty()) { - // Explicitly empty - clear all settings + // Explicitly empty - clear all search settings (override_request_values will be re-injected by constructor) updatedSettings = Settings.EMPTY; } else { Suggestion importance[1-10]: 2 __ Why: This is purely a comment clarification with no functional change. The existing_code and improved_code are functionally identical, differing only in the comment text, which warrants a low score.	Low
	Verify sentinel detection works with setting defaults The default value of WLM_MAX_CONCURRENT_SHARD_REQUESTS is set to 5, which matches SearchRequest's default. However, when this setting is stored in WLM group settings and later retrieved via wlmSettings.hasValue(...), the default value 5 will be applied even if the user never explicitly set it — because Settings.builder().put("search.max_concurrent_shard_requests", "5") stores the value. The sentinel-based detection using getMaxConcurrentShardRequestsRaw() == 0 in the listener relies on the WLM setting only being present when explicitly configured. The default in the Setting definition should not affect stored values, but it's worth verifying that hasValue correctly returns false when the key was never stored. server/src/main/java/org/opensearch/wlm/WorkloadGroupSearchSettings.java [53-57] +public static final Setting<Integer> WLM_MAX_CONCURRENT_SHARD_REQUESTS = Setting.intSetting( + "search.max_concurrent_shard_requests", + 5, + 1 +); - Suggestion importance[1-10]: 1 __ Why: The existing_code and improved_code are identical, making this a verification suggestion rather than an actual code change. It only asks the user to verify behavior, warranting a very low score.	Low

[github-actions]

❌ Gradle check result for 9a8c5ae: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

[github-actions]

Persistent review updated to latest commit 42afadf

[github-actions]

❌ Gradle check result for 42afadf: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

[github-actions]

Persistent review updated to latest commit 8e64571

[github-actions]

❌ Gradle check result for 8e64571: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

[github-actions]

Persistent review updated to latest commit 368789a

[github-actions]

❌ Gradle check result for 368789a: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

[cwperks]

Thank you @dzane17. Overall the change looks good to me, but is it possible to add any integ tests to demonstrate that the overrides are taking when a request is explicitly given values but maps to a workload group that specifies to override?

idk we can can do anything contrived and cancel a request after a few seconds even if the timeout passed to a request is 1m?

[github-actions]

✅ Gradle check result for 368789a: SUCCESS

[codecov]

Codecov Report

❌ Patch coverage is 81.15942% with 13 lines in your changes missing coverage. Please review.
✅ Project coverage is 73.43%. Comparing base (3bea28a) to head (6a38df0).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
...steners/WorkloadGroupRequestOperationListener.java	79.54%	8 Missing and 1 partial ⚠️
...org/opensearch/cluster/metadata/WorkloadGroup.java	84.61%	0 Missing and 2 partials ⚠️
...g/opensearch/wlm/MutableWorkloadGroupFragment.java	50.00%	0 Missing and 2 partials ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##               main   #21523      +/-   ##
============================================
- Coverage     73.48%   73.43%   -0.05%     
+ Complexity    74736    74701      -35     
============================================
  Files          5983     5983              
  Lines        339062   339120      +58     
  Branches      48882    48895      +13     
============================================
- Hits         249162   249042     -120     
- Misses        70064    70289     +225     
+ Partials      19836    19789      -47     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[dzane17]

@cwperks A true end-to-end functional test is tricky.

The integ test testSearchSettingsOverrideRequestValues now covers the full request flow but does not assert the WLM settings are actually injected. For the timeout example, doing so would require controlling request duration deterministically, which isn't really possible — CI machine variability plus the fact that painless scripts can't use Thread.sleep for security reasons means we'd need a test-only plugin exposing a blocking query type just to make one assertion reliable.

Override behavior is already thoroughly covered in unit tests WorkloadGroupRequestOperationListenerTests with both positive and negative cases per setting. Each test creates a real SearchRequest, invokes the real listener, and asserts on the post-listener state of the request. That tells us WLM properly injects the setting values into the SearchRequest object. From that point it's just a matter of OpenSearch honoring attributes on the request, which is an existing workflow.

[github-actions]

Persistent review updated to latest commit b01f234

[github-actions]

❌ Gradle check result for b01f234: null

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

[github-actions]

❌ Gradle check result for b01f234: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

[github-actions]

Persistent review updated to latest commit 5bdf642

[github-actions]

✅ Gradle check result for 5bdf642: SUCCESS

[github-actions]

Persistent review updated to latest commit 6a38df0

[github-actions]

✅ Gradle check result for 6a38df0: SUCCESS