Skip to content

Introducing indexing & deletion strategy planner interfaces#20585

Open
shank9918 wants to merge 1 commit intoopensearch-project:mainfrom
shank9918:refactored-engine
Open

Introducing indexing & deletion strategy planner interfaces#20585
shank9918 wants to merge 1 commit intoopensearch-project:mainfrom
shank9918:refactored-engine

Conversation

@shank9918
Copy link

@shank9918 shank9918 commented Feb 10, 2026

Description

  • Introduce Strategy Planner Interfaces

Refactor indexing and deletion planning logic into dedicated planner interfaces responsible for primary and non-primary execution. Default planner implementations would encapsulate the existing InternalEngine behavior, while enabling alternative engines to provide custom planners or reuse shared implementations.

  • Delegate Planning via Composition

Update Engine implementations to delegate planning decisions to injected planner instances. The default behavior for InternalEngine should remain unchanged, ensuring backward compatibility.

Related Issues

Resolves #[Issue number to be closed when this PR is merged]

[Feature Request] Refactor Engine internals to decouple operation/result types and introduce pluggable planning strategies

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.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 10, 2026

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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

@shank9918 shank9918 changed the title Introducing indexing & deletion strategy planner interfaces and refac… Decouple operation/result types and introduce pluggable planning strategies Feb 10, 2026
@shank9918 shank9918 force-pushed the refactored-engine branch 3 times, most recently from 344f4b8 to 944cf1a Compare February 11, 2026 05:26
@github-actions
Copy link
Contributor

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

@shank9918 shank9918 force-pushed the refactored-engine branch 3 times, most recently from 9513ca5 to 479e74f Compare February 11, 2026 05:49
@github-actions
Copy link
Contributor

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

@shank9918 shank9918 changed the title Decouple operation/result types and introduce pluggable planning strategies Introducing indexing & deletion strategy planner interfaces Feb 11, 2026
@github-actions
Copy link
Contributor

❌ Gradle check result for eecb2d0: 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 6be6a79: 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?

@shank9918 shank9918 force-pushed the refactored-engine branch 2 times, most recently from 3c3a9cd to 83bcfc7 Compare February 12, 2026 06:05
@github-actions
Copy link
Contributor

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

@shank9918 shank9918 closed this Feb 12, 2026
@shank9918 shank9918 reopened this Feb 12, 2026
@github-actions
Copy link
Contributor

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

Copy link
Contributor

@Bukhtawar Bukhtawar left a comment

Choose a reason for hiding this comment

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

Thanks for the changes, left minor comment, lets tease out Strategy classes into a dedicated Java package

@shank9918
Copy link
Author

Thanks for the changes, left minor comment, lets tease out Strategy classes into a dedicated Java package

The Planner classes rely on the package-private LiveVersionMap and VersionValue. Relocating strategy and planner classes to a dedicated package would force us to modify their access levels which I want to avoid.

@github-actions
Copy link
Contributor

Persistent review updated to latest commit 2c61040

@github-actions
Copy link
Contributor

Persistent review updated to latest commit 135ed2c

@github-actions
Copy link
Contributor

Persistent review updated to latest commit 614ddcb

@github-actions
Copy link
Contributor

✅ Gradle check result for 614ddcb: SUCCESS

@github-actions
Copy link
Contributor

Persistent review updated to latest commit cf08e29

@github-actions
Copy link
Contributor

Persistent review updated to latest commit a681e1e

Signed-off-by: Shashank Gowri <shnkgo@amazon.com>
@github-actions
Copy link
Contributor

Persistent review updated to latest commit 22e6d16

@github-actions
Copy link
Contributor

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

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.

3 participants