Skip to content

Conversation

@gaobinlong
Copy link
Contributor

@gaobinlong gaobinlong commented Nov 26, 2025

Description

When there're multiple index templates matching data stream, the highest priority template will be used for the new created data stream, we can check the current used template by GET _data_stream/{name} API, but for the low priority template which are not used by any data streams, it cannot be deleted, this PR fixes this bug and add some tests for this case.

Related Issues

Resolves #20078

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.

Summary by CodeRabbit

  • Bug Fixes

    • Fixed an issue where deleting an index template could fail when the template matched a data stream pattern but had lower priority than another template.
  • Tests

    • Added tests validating template deletion behavior and priority-based template selection to prevent regressions.

✏️ Tip: You can customize this high-level summary in your review settings.

…tches a data stream but has low priority

Signed-off-by: Binlong Gao <gbinlong@amazon.com>
@gaobinlong gaobinlong requested a review from a team as a code owner November 26, 2025 06:09
@github-actions github-actions bot added bug Something isn't working Indexing Indexing, Bulk Indexing and anything related to indexing labels Nov 26, 2025
Signed-off-by: Binlong Gao <gbinlong@amazon.com>
@github-actions
Copy link
Contributor

✅ Gradle check result for 4d8295d: SUCCESS

@codecov
Copy link

codecov bot commented Nov 26, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 73.34%. Comparing base (97d3864) to head (90d582b).
⚠️ Report is 12 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff            @@
##               main   #20102   +/-   ##
=========================================
  Coverage     73.33%   73.34%           
- Complexity    71679    71710   +31     
=========================================
  Files          5790     5786    -4     
  Lines        327549   327611   +62     
  Branches      47181    47203   +22     
=========================================
+ Hits         240217   240287   +70     
- Misses        68080    68089    +9     
+ Partials      19252    19235   -17     

☔ 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.

@sandeshkr419 sandeshkr419 changed the title Fix delete not using index template failed when the index template matches a data stream but has low priority Fix deletion failure/error of unused index template when the index template matches a data stream but has low priority Nov 26, 2025
@sandeshkr419 sandeshkr419 changed the title Fix deletion failure/error of unused index template when the index template matches a data stream but has low priority Fix deletion failure/error of unused index template when the index template matches a data stream but has lower priority Nov 26, 2025
Copy link
Member

@sandeshkr419 sandeshkr419 left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for adding the test case.

Minor nit-pick on making the changelog more clarifying.

Signed-off-by: Binlong Gao <gbinlong@amazon.com>
@github-actions
Copy link
Contributor

❌ Gradle check result for a88ef78: 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
Copy link
Contributor

❌ Gradle check result for a88ef78: 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?

@peterzhuamazon
Copy link
Member

@coderabbitai review

@coderabbitai
Copy link

coderabbitai bot commented Nov 26, 2025

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai
Copy link

coderabbitai bot commented Nov 26, 2025

Walkthrough

Refactors template resolution to select a single highest-priority V2 template per pattern and restricts data-stream associations to that winner; adds tests and REST-spec coverage plus a changelog entry describing the deletion fix for an unused lower-priority data-stream index template.

Changes

Cohort / File(s) Summary
Changelog & REST test
CHANGELOG.md, rest-api-spec/src/main/resources/rest-api-spec/test/indices.delete_index_template/10_basic.yml
Adds a changelog "Fixed" entry for PR #20102; extends the indices.delete_index_template test with test_template_3 (data_stream template, priority 49), teardown cleanup for test-2 data stream and test_template_3, and a new scenario exercising deletion of a lower-priority template that matches a data stream pattern.
Core template resolution
server/src/main/java/org/opensearch/cluster/metadata/MetadataIndexTemplateService.java
Changes findV2Template() to pick a single winner (highest priority) instead of returning all matches; updates dataStreamsUsingTemplate() to only include data streams whose current highest-priority V2 template equals the specified template name.
Unit tests
server/src/test/java/org/opensearch/cluster/metadata/MetadataIndexTemplateServiceTests.java
Adds testRemoveIndexTemplateV2ForDataStream() to validate removal of a lower-priority template when a higher-priority template matches the same data stream pattern. Note: the patch contains two identical insertions of this test method.

Sequence Diagram(s)

sequenceDiagram
    participant Client
    participant Resolver as MetadataIndexTemplateService
    participant Cluster as ClusterState

    rect `#F8F9FB`
    note right of Resolver: Old behavior (bug)
    Client->>Resolver: resolve V2 templates for pattern "test*"
    Resolver->>Resolver: collect ALL matching templates (50, 51)
    Resolver->>Cluster: ask which data streams match any pattern
    Cluster-->>Resolver: data stream "test" matches
    Resolver-->>Client: report template "test" in use (blocks deletion)
    end

    rect `#F6FBF8`
    note right of Resolver: New behavior (fixed)
    Client->>Resolver: resolve V2 templates for pattern "test*"
    Resolver->>Resolver: select HIGHEST-priority winner (51)
    Resolver->>Cluster: ask which data streams use winner (51)
    Cluster-->>Resolver: "test" uses winner (51) only
    Resolver-->>Client: allow deletion of lower-priority template (50)
    end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

  • Pay attention to priority comparison and ties in findV2Template().
  • Verify dataStreamsUsingTemplate() correctly filters streams using the winner identity.
  • Inspect the duplicated test insertion and ensure only one instance remains.
  • Confirm REST test teardown/allowed_warnings entries are correct for CI.

Poem

🐰 I nibbled through templates, sniffed the priority trail,
The top one now stands tall, the rest can set sail,
A gentle hop, a tidy state, no more deletion fright,
Lower friends step softly — the highest takes the light. ✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately and clearly describes the main change: fixing a deletion failure for unused index templates with lower priority that match data streams.
Description check ✅ Passed The description follows the template structure with Description, Related Issues, and Check List sections; core sections are complete with technical details and issue reference.
Linked Issues check ✅ Passed The PR fulfills the linked issue #20078 objectives by fixing the deletion logic for lower-priority templates and adding comprehensive test coverage.
Out of Scope Changes check ✅ Passed All changes are directly scoped to fixing template deletion logic and adding related tests; no unrelated modifications detected.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a88ef78 and 90d582b.

📒 Files selected for processing (1)
  • CHANGELOG.md (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • CHANGELOG.md
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (21)
  • GitHub Check: gradle-check
  • GitHub Check: Mend Security Check
  • GitHub Check: precommit (25, ubuntu-24.04-arm)
  • GitHub Check: precommit (21, macos-15-intel)
  • GitHub Check: precommit (25, macos-15-intel)
  • GitHub Check: precommit (21, ubuntu-latest)
  • GitHub Check: precommit (21, windows-2025, true)
  • GitHub Check: precommit (25, macos-15)
  • GitHub Check: precommit (21, macos-15)
  • GitHub Check: precommit (21, ubuntu-24.04-arm)
  • GitHub Check: precommit (25, ubuntu-latest)
  • GitHub Check: precommit (25, windows-latest)
  • GitHub Check: precommit (21, windows-latest)
  • GitHub Check: assemble (25, ubuntu-latest)
  • GitHub Check: assemble (21, windows-latest)
  • GitHub Check: assemble (25, windows-latest)
  • GitHub Check: assemble (21, ubuntu-latest)
  • GitHub Check: assemble (25, ubuntu-24.04-arm)
  • GitHub Check: assemble (21, ubuntu-24.04-arm)
  • GitHub Check: detect-breaking-change
  • GitHub Check: Analyze (java)

Tip

📝 Customizable high-level summaries are now available in beta!

You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.

  • Provide your own instructions using the high_level_summary_instructions setting.
  • Format the summary however you like (bullet lists, tables, multi-section layouts, contributor stats, etc.).
  • Use high_level_summary_in_walkthrough to move the summary from the description to the walkthrough section.

Example instruction:

"Divide the high-level summary into five sections:

  1. 📝 Description — Summarize the main change in 50–60 words, explaining what was done.
  2. 📓 References — List relevant issues, discussions, documentation, or related PRs.
  3. 📦 Dependencies & Requirements — Mention any new/updated dependencies, environment variable changes, or configuration updates.
  4. 📊 Contributor Summary — Include a Markdown table showing contributions:
    | Contributor | Lines Added | Lines Removed | Files Changed |
  5. ✔️ Additional Notes — Add any extra reviewer context.
    Keep each section concise (under 200 words) and use bullet or numbered lists for clarity."

Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later.


Comment @coderabbitai help to get the list of available commands and usage tips.

Signed-off-by: Sandesh Kumar <sandeshkr419@gmail.com>
@github-actions
Copy link
Contributor

❌ Gradle check result for 90d582b: 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
Copy link
Contributor

✅ Gradle check result for 90d582b: SUCCESS

@gaobinlong gaobinlong merged commit bd24685 into opensearch-project:main Nov 27, 2025
36 of 38 checks passed
kkewwei pushed a commit to kkewwei/OpenSearch that referenced this pull request Nov 28, 2025
…mplate matches a data stream but has lower priority (opensearch-project#20102)

* Fix delete not using index template failed when the index template matches a data stream but has low priority

Signed-off-by: Binlong Gao <gbinlong@amazon.com>

* Modify change log

Signed-off-by: Binlong Gao <gbinlong@amazon.com>

* Optimize change log

Signed-off-by: Binlong Gao <gbinlong@amazon.com>

---------

Signed-off-by: Binlong Gao <gbinlong@amazon.com>
Signed-off-by: Sandesh Kumar <sandeshkr419@gmail.com>
Co-authored-by: Sandesh Kumar <sandeshkr419@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working Indexing Indexing, Bulk Indexing and anything related to indexing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] DataStreams: Can't delete index template that matches the data stream but is unused

3 participants