Skip to content

Support mixed AND/OR rollover conditions via any_of - #1667

Merged
tandonks merged 10 commits into
opensearch-project:mainfrom
praladhe:feature/rollover-any_of-condition
Jul 9, 2026
Merged

Support mixed AND/OR rollover conditions via any_of#1667
tandonks merged 10 commits into
opensearch-project:mainfrom
praladhe:feature/rollover-any_of-condition

Conversation

@praladhe

@praladhe praladhe commented Jun 12, 2026

Copy link
Copy Markdown
Contributor

Description

Adds support for mixed AND/OR rollover conditions in the ISM rollover action through a
new, opt-in grouped syntax (any_of).

Today the rollover action's four conditions (min_index_age, min_size, min_doc_count,
min_primary_shard_size) are combined with pure OR — rollover triggers as soon as any single
condition is met. There is no way to express "roll over when (age >= 7d AND size >= 50gb) OR
(doc_count >= 100000000)".

This change introduces an alternative OR-of-ANDs form. The action can hold an ordered list
of condition groups under any_of; the conditions inside a group are AND-ed and the groups are
OR-ed. Rollover triggers when any one group is fully satisfied. Because any boolean combination of
the four conditions can be rewritten as OR-of-ANDs, this covers all practical rollover policies.

Example:

{
  "rollover": {
    "any_of": [
      { "min_index_age": "7d", "min_size": "50gb" },
      { "min_doc_count": 100000000 }
    ]
  }
}

Behavior and compatibility

  • The existing flat form is unchanged and remains the default (implicit OR), so all existing policies behave exactly as before.
  • The flat form and the grouped any_of form are mutually exclusive within a single rollover action; specifying both is rejected at parse/construction time.
  • An empty any_of, an empty group, non-positive thresholds, and unknown fields inside a group are all rejected with informative errors.
  • Stream serialization of the grouped data is version-gated (introduced in 3.7.0), following the existing prevent_empty_rollover / 3.4.0 pattern, so mixed-version clusters operate safely — older nodes never receive groups and fall back to flat behavior.
  • The prevent_empty_rollover empty-index guard is preserved unchanged and still runs before condition evaluation.
  • The ISM explain output reports grouped criteria under an any_of array; flat reporting is unchanged.

Changes

  • New RolloverConditionGroup model (parsing, validation, XContent + stream serialization).
  • RolloverAction: new optional conditionGroups field, mutual-exclusivity/empty validation, grouped XContent output, and version-gated stream write; new any_of constant and TARGET_VERSION.
  • RolloverActionParser: parse any_of from XContent and version-gated read from stream.
  • evaluateConditions in ManagedIndexUtils: refactored to a shared per-condition comparison helper reused by both flat (OR) and grouped (OR-of-ANDs) paths, keeping flat behavior byte-identical.
  • AttemptRolloverStep: report flat conditions or grouped any_of criteria.
  • Config index mapping: added any_of and bumped schema_version 29 → 30 (mapping and cached test copy kept in sync; test configSchemaVersion updated).

Testing

  • Unit tests: 796 passing, including example tests and property-based tests (kotest) covering evaluation, serialization round-trips, version gating, and validation.
  • Integration tests: RolloverActionIT (incl. a new end-to-end grouped any_of test) and IndexManagementIndicesIT pass against a real cluster.
  • Verified manually on a live 3.7.0 cluster: a grouped policy rolled over when one group's conditions were satisfied while another group's were not.

Public Documentation PR: #12740
API changes companion pull request: #1158

Related Issues

Resolves #1540

Check List

  • New functionality includes testing.
  • New functionality has been documented.
  • API changes companion pull request created.
  • Commits are signed per the DCO using --signoff.
  • Public documentation issue/PR created.

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.

praladhe added 2 commits June 12, 2026 14:29
Add an opt-in grouped rollover syntax (any_of) to the ISM rollover
action. Conditions within a group are AND-ed and groups are OR-ed
(OR-of-ANDs); rollover triggers when any one group is fully satisfied.
The existing flat form is unchanged and remains the default. Grouped
stream serialization is version-gated at 3.7.0 for mixed-version
cluster safety.

Signed-off-by: Prathamesh Ladhe <praladhe@amazon.com>
@codecov

codecov Bot commented Jun 12, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 97.76119% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 76.85%. Comparing base (bcbb9dd) to head (39996e2).

Files with missing lines Patch % Lines
...ment/indexstatemanagement/action/RolloverAction.kt 94.44% 0 Missing and 1 partial ⚠️
...atemanagement/step/rollover/AttemptRolloverStep.kt 97.61% 0 Missing and 1 partial ⚠️
...ent/indexstatemanagement/util/ManagedIndexUtils.kt 94.73% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1667      +/-   ##
==========================================
+ Coverage   76.65%   76.85%   +0.20%     
==========================================
  Files         387      388       +1     
  Lines       18566    18655      +89     
  Branches     2601     2619      +18     
==========================================
+ Hits        14231    14337     +106     
+ Misses       3028     3025       -3     
+ Partials     1307     1293      -14     

☔ View full report in Codecov by Harness.
📢 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.

@praladhe
praladhe marked this pull request as ready for review June 12, 2026 15:19
Comment on lines +82 to +85
if (out.version.onOrAfter(TARGET_VERSION)) {
out.writeBoolean(conditionGroups != null)
if (conditionGroups != null) out.writeList(conditionGroups)
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what happens if a grouped policy is sent to a pre-3.7.0 node? suppose we created this policy in new version and Index was created before 3.7.0, what will the behaviour then?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It never runs there. During a mixed-version upgrade, SkipExecution pauses all ISM (node versions differ), and the transport gate removes the groups before they reach the old node; if the old node parses the JSON directly it just rejects it (Invalid field: [any_of]).
For index created before 3.7.0 it doesn't matter as ISM evaluates live index stats (age/size/docs), not the creation version. Once the cluster is on 3.7+, the grouped policy works exactly as normal.

@github-actions

github-actions Bot commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

PR Code Analyzer ❗

AI-powered 'Code-Diff-Analyzer' found issues on commit 9c002de.

PathLineSeverityDescription
build.gradle242highNew test dependency added: io.kotest:kotest-property:5.9.1. Dependency changes must be verified by maintainers regardless of perceived legitimacy.
build.gradle243highNew test dependency added: io.kotest:kotest-common:5.9.1. Dependency changes must be verified by maintainers regardless of perceived legitimacy.
build.gradle244highNew test dependency added: io.kotest:kotest-assertions-shared:5.9.1. Dependency changes must be verified by maintainers regardless of perceived legitimacy.

The table above displays the top 10 most important findings.

Total: 3 | Critical: 0 | High: 3 | Medium: 0 | Low: 0


Pull Requests Author(s): Please update your Pull Request according to the report above.

Repository Maintainer(s): You can bypass diff analyzer by adding label skip-diff-analyzer after reviewing the changes carefully, then re-run failed actions. To re-enable the analyzer, remove the label, then re-run all actions.


⚠️ Note: The Code-Diff-Analyzer helps protect against potentially harmful code patterns. Please ensure you have thoroughly reviewed the changes beforehand.

Thanks.

Signed-off-by: praladhe <praladhe@amazon.com>
@praladhe
praladhe requested a review from Tarun-kishore as a code owner July 7, 2026 04:51
Remove io.kotest test dependencies and rewrite the three rollover
any_of property-based tests on OpenSearch's built-in randomized testing.
Resolves the Code-Diff-Analyzer dependency flag.

Signed-off-by: praladhe <praladhe@amazon.com>
@github-actions

github-actions Bot commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🧪 PR contains tests
🔒 No security concerns identified
✅ No TODO sections
🔀 Multiple PR themes

Sub-PR theme: Bump ISM config schema version to 30 and add any_of mapping

Relevant files:

  • src/main/resources/mappings/opendistro-ism-config.json
  • src/test/resources/mappings/cached-opendistro-ism-config.json
  • src/test/kotlin/org/opensearch/indexmanagement/IndexManagementRestTestCase.kt

Sub-PR theme: Introduce any_of grouped rollover conditions (model + evaluation)

Relevant files:

  • src/main/kotlin/org/opensearch/indexmanagement/indexstatemanagement/action/RolloverAction.kt
  • src/main/kotlin/org/opensearch/indexmanagement/indexstatemanagement/action/RolloverActionParser.kt
  • src/main/kotlin/org/opensearch/indexmanagement/indexstatemanagement/action/RolloverConditionGroup.kt
  • src/main/kotlin/org/opensearch/indexmanagement/indexstatemanagement/util/ManagedIndexUtils.kt

Sub-PR theme: Report grouped criteria via any_of in AttemptRolloverStep explain output

Relevant files:

  • src/main/kotlin/org/opensearch/indexmanagement/indexstatemanagement/step/rollover/AttemptRolloverStep.kt
  • src/test/kotlin/org/opensearch/indexmanagement/indexstatemanagement/step/AttemptRolloverStepReportingTests.kt

⚡ Recommended focus areas for review

Unused parameter

executeRollover was changed to accept a new conditions: Map<String, Any?> parameter, but the diff does not show it being used inside the function, and there is no visible call site passing the computed conditions map. If the parameter is unused or callers are not updated, either the reported conditions are never propagated to the rollover result, or the code will not compile. Verify the call sites and that the parameter is actually consumed.

@Suppress("CyclomaticComplexMethod")
private suspend fun executeRollover(
    context: StepContext,
    rolloverTarget: String,
    isDataStream: Boolean,
    conditions: Map<String, Any?>,
) {
Behavior change for empty group list

When conditionGroups is non-null but empty, groups.any { ... } returns false, meaning rollover never triggers. The RolloverAction init block rejects empty any_of, but deserialization from older code paths or direct construction should be double-checked. Also note that the previous flat semantics returned true when no conditions were specified; with results.isEmpty() returning true this is preserved for flat, but the grouped branch does not have an equivalent fallback for an accidentally empty group list.

val groups = this.conditionGroups
if (!groups.isNullOrEmpty()) {
    return groups.any { group ->
        val results =
            rolloverConditionResults(
                group.minSize, group.minDocs, group.minAge, group.minPrimaryShardSize,
                indexAgeTimeValue, numDocs, indexSize, primaryShardSize,
            )
        results.all { it }
    }
}

@github-actions

github-actions Bot commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Fail loud on unsupported version serialization

When serializing to a node older than V_3_7_0, conditionGroups is silently dropped.
This can lead to a grouped rollover action reaching an old node as an action with no
conditions, which per evaluateConditions returns true (immediate rollover). Consider
throwing an explicit error in writeTo when conditionGroups is set but the target
version does not support it, to prevent unsafe behavior on mixed-version clusters.

src/main/kotlin/org/opensearch/indexmanagement/indexstatemanagement/action/RolloverAction.kt [82-85]

 if (out.version.onOrAfter(Version.V_3_7_0)) {
     out.writeBoolean(conditionGroups != null)
     if (conditionGroups != null) out.writeList(conditionGroups)
+} else if (!conditionGroups.isNullOrEmpty()) {
+    throw IllegalStateException(
+        "Cannot serialize RolloverAction with [$ANY_OF_FIELD] to a node on version [${out.version}] " +
+            "which does not support grouped rollover conditions",
+    )
 }
Suggestion importance[1-10]: 7

__

Why: Valid concern: silently dropping conditionGroups when serializing to an older node could result in a grouped action being interpreted as an unconditional rollover, causing unsafe behavior in mixed-version clusters. Failing explicitly is safer.

Medium
General
Handle empty groups list defensively

When conditionGroups is non-null but empty (which the init block already rejects),
the code falls through to flat evaluation and returns true because no flat
conditions exist. Since conditionGroups.isNullOrEmpty() treats an empty list as "no
groups", a defensive check would prevent an accidental "always rollover" behavior if
validation ever regresses. Consider distinguishing null vs empty explicitly.

src/main/kotlin/org/opensearch/indexmanagement/indexstatemanagement/util/ManagedIndexUtils.kt [294-312]

+val groups = this.conditionGroups
+if (groups != null) {
+    if (groups.isEmpty()) return false
+    return groups.any { group ->
+        val results =
+            rolloverConditionResults(
+                group.minSize, group.minDocs, group.minAge, group.minPrimaryShardSize,
+                indexAgeTimeValue, numDocs, indexSize, primaryShardSize,
+            )
+        results.all { it }
+    }
+}
+
 val results =
     rolloverConditionResults(
         this.minSize, this.minDocs, this.minAge, this.minPrimaryShardSize,
         indexAgeTimeValue, numDocs, indexSize, primaryShardSize,
     )
 if (results.isEmpty()) return true
 return results.any { it }
Suggestion importance[1-10]: 4

__

Why: Defensive coding suggestion. The init block already validates against empty groups, so this is a defense-in-depth improvement with minor practical impact.

Low
Provide clearer error for malformed any_of

The ensureExpectedToken call here uses XContentParser.Token.START_ARRAY but the
parser is already positioned on the value token via xcp.nextToken() before the when
block (as it is for other fields). Double-check this actually works; if the current
token is the field-name-advanced-to-value token, this is fine, but be aware that if
any_of is provided as something other than an array (e.g., an object), the error
message will be a raw parser exception. Consider a clearer message referencing the
field name.

src/main/kotlin/org/opensearch/indexmanagement/indexstatemanagement/action/RolloverActionParser.kt [72-79]

 RolloverAction.ANY_OF_FIELD -> {
-    ensureExpectedToken(XContentParser.Token.START_ARRAY, xcp.currentToken(), xcp)
+    require(xcp.currentToken() == XContentParser.Token.START_ARRAY) {
+        "Expected an array for [${RolloverAction.ANY_OF_FIELD}] but got [${xcp.currentToken()}]"
+    }
     val groups = mutableListOf<RolloverConditionGroup>()
     while (xcp.nextToken() != XContentParser.Token.END_ARRAY) {
         groups.add(RolloverConditionGroup.parse(xcp))
     }
     conditionGroups = groups
 }
Suggestion importance[1-10]: 3

__

Why: Minor improvement to error messaging. The existing ensureExpectedToken call already throws a reasonable error, so this is a marginal readability enhancement.

Low

if (out.version.onOrAfter(Version.V_3_4_0)) {
out.writeBoolean(preventEmptyRollover)
}
if (out.version.onOrAfter(Version.V_3_7_0)) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since 3.7 is already released, we should change it to 3.8

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed — if this ships in 3.8, V_3_8_0 is the right gate. I've gated on V_3_7_0 for now and will flip these to V_3_8_0 as soon as we bump the dependency to 3.8.0-SNAPSHOT.

@tandonks tandonks left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool looks good. Please make sure to followup on the documentation as this is a big change. Thanks for your first contribution :)

@tandonks
tandonks merged commit ec4d114 into opensearch-project:main Jul 9, 2026
25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[FEATURE] Support AND operations on Rollover conditions

4 participants