Skip to content

[Benchmark PR Configs] Add id_15 with big5 force intra segment partition strategy#20557

Merged
prudhvigodithi merged 1 commit into
opensearch-project:mainfrom
prudhvigodithi:benchmark
Feb 9, 2026
Merged

[Benchmark PR Configs] Add id_15 with big5 force intra segment partition strategy#20557
prudhvigodithi merged 1 commit into
opensearch-project:mainfrom
prudhvigodithi:benchmark

Conversation

@prudhvigodithi
Copy link
Copy Markdown
Member

@prudhvigodithi prudhvigodithi commented Feb 5, 2026

Description

  • Add id_15 with big5 force intra segment partition strategy.
  • "ADDITIONAL_CONFIG": "search.concurrent_segment_search.partition_strategy:force",
  • To test intra segment benchmark numbers with force partition strategy. Example Intra segment support for single-value metric aggregations #20503 where we can compare numbers with default balanced and force partition strategy.

Related Issues

Part of #20202

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.

Signed-off-by: Prudhvi Godithi <pgodithi@amazon.com>
@prudhvigodithi prudhvigodithi requested review from a team, jed326 and peternied as code owners February 5, 2026 18:47
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Feb 5, 2026

📝 Walkthrough

Walkthrough

This PR adds a new benchmark configuration entry (id_15) to .github/benchmark-configs.json for an intra-segment search test targeting the big5 workload with concurrent segment search mode set to auto and partition_strategy:force parameter.

Changes

Cohort / File(s) Summary
Benchmark Configuration
.github/benchmark-configs.json
Adds new benchmark entry id_15 with intra-segment search configuration for big5 workload, mirroring existing cluster setup with additional partition_strategy:force parameter.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~2 minutes

Possibly related PRs

Suggested labels

skip-changelog

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: adding a new benchmark configuration id_15 with big5 workload and force intra segment partition strategy.
Description check ✅ Passed The description includes the required Description section explaining the change and Related Issues linking to issue #20202, meeting the template requirements.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

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

@prudhvigodithi
Copy link
Copy Markdown
Member Author

Adding @asimmahmood1 @jainankitk @rishabh6788

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In @.github/benchmark-configs.json:
- Around line 245-252: The ADDITIONAL_CONFIG entry currently sets only
search.concurrent_segment_search.partition_strategy but omits the mode,
contradicting the description; update the ADDITIONAL_CONFIG value to include
search.concurrent_segment_search.mode:auto alongside
search.concurrent_segment_search.partition_strategy:force so the configuration
string contains both keys (refer to the ADDITIONAL_CONFIG property and the
existing search.concurrent_segment_search.partition_strategy setting) to match
the "mode as auto" description.
📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3ba2f37 and e9911a1.

📒 Files selected for processing (1)
  • .github/benchmark-configs.json
⏰ 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). (19)
  • GitHub Check: precommit (21, ubuntu-24.04-arm)
  • GitHub Check: precommit (25, windows-latest)
  • GitHub Check: precommit (21, windows-latest)
  • GitHub Check: precommit (21, windows-2025, true)
  • GitHub Check: precommit (21, macos-15-intel)
  • GitHub Check: precommit (25, ubuntu-latest)
  • GitHub Check: detect-breaking-change
  • GitHub Check: precommit (25, ubuntu-24.04-arm)
  • GitHub Check: precommit (25, macos-15-intel)
  • GitHub Check: precommit (25, macos-15)
  • GitHub Check: precommit (21, macos-15)
  • GitHub Check: precommit (21, ubuntu-latest)
  • GitHub Check: assemble (21, ubuntu-latest)
  • GitHub Check: assemble (25, windows-latest)
  • GitHub Check: assemble (21, windows-latest)
  • GitHub Check: assemble (25, ubuntu-latest)
  • GitHub Check: assemble (21, ubuntu-24.04-arm)
  • GitHub Check: assemble (25, ubuntu-24.04-arm)
  • GitHub Check: Analyze (java)

✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.

Comment thread .github/benchmark-configs.json
Comment thread .github/benchmark-configs.json
@prudhvigodithi
Copy link
Copy Markdown
Member Author

Thanks @asimmahmood1, @jainankitk are we good with this change?

Copy link
Copy Markdown
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.

@prudhvigodithi Let's add a changelog as well. Rest LGTM!

@prudhvigodithi
Copy link
Copy Markdown
Member Author

@prudhvigodithi Let's add a changelog as well. Rest LGTM!

In past for these changes we usually skip the changelog, but the unresolved comments was preventing this PR from merged.

@prudhvigodithi prudhvigodithi merged commit ee0a9be into opensearch-project:main Feb 9, 2026
45 of 46 checks passed
tanyabti pushed a commit to tanyabti/OpenSearch that referenced this pull request Feb 24, 2026
Signed-off-by: Prudhvi Godithi <pgodithi@amazon.com>
pradeep-L pushed a commit to pradeep-L/OpenSearch that referenced this pull request Apr 21, 2026
Signed-off-by: Prudhvi Godithi <pgodithi@amazon.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants