Fix closed-index reroute handling#20648
Fix closed-index reroute handling#20648srikanthpadakanti wants to merge 5 commits intoopensearch-project:mainfrom
Conversation
Signed-off-by: Srikanth Padakanti <srikanth_padakanti@apple.com>
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis PR fixes a bug where shard routings for closed indexes were being allocated without the index being reopened. The fix adds guards to skip allocation and batching when shards have an INDEX_CLOSED unassigned reason, along with comprehensive test coverage. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Comment |
Signed-off-by: Srikanth Padakanti <srikanth_padakanti@apple.com>
Signed-off-by: Srikanth Padakanti <srikanth_padakanti@apple.com>
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
CHANGELOG.mdserver/src/main/java/org/opensearch/gateway/GatewayAllocator.javaserver/src/main/java/org/opensearch/gateway/ShardsBatchGatewayAllocator.javaserver/src/test/java/org/opensearch/gateway/GatewayAllocatorTests.java
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2026-01-13T17:40:27.167Z
Learnt from: reta
Repo: opensearch-project/OpenSearch PR: 20411
File: server/src/main/java/org/opensearch/index/codec/CodecService.java:112-133
Timestamp: 2026-01-13T17:40:27.167Z
Learning: Avoid capturing or evaluating a supplier (e.g., this::defaultCodec) upfront when passing it to a registry during object construction. If registries may replace defaults during iteration (as in EnginePlugin.getAdditionalCodecs), pass the supplier itself and only resolve it at use time. This ensures dynamic behavior is preserved during initialization and prevents premature binding of defaults in codecs/registry setup. This pattern should apply to similar initialization paths in Java server code where registries may mutate defaults during construction.
Applied to files:
server/src/main/java/org/opensearch/gateway/GatewayAllocator.javaserver/src/main/java/org/opensearch/gateway/ShardsBatchGatewayAllocator.java
🧬 Code graph analysis (1)
server/src/test/java/org/opensearch/gateway/GatewayAllocatorTests.java (1)
server/src/main/java/org/opensearch/gateway/GatewayAllocator.java (1)
GatewayAllocator(74-369)
⏰ 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). (18)
- GitHub Check: precommit (21, ubuntu-24.04-arm)
- GitHub Check: precommit (25, ubuntu-24.04-arm)
- GitHub Check: precommit (21, windows-2025, true)
- GitHub Check: precommit (21, ubuntu-latest)
- GitHub Check: precommit (21, macos-15-intel)
- GitHub Check: precommit (25, windows-latest)
- GitHub Check: precommit (25, macos-15-intel)
- GitHub Check: precommit (25, macos-15)
- GitHub Check: precommit (25, ubuntu-latest)
- GitHub Check: Analyze (java)
- GitHub Check: assemble (21, windows-latest)
- GitHub Check: assemble (21, macos-15)
- GitHub Check: assemble (21, ubuntu-24.04-arm)
- GitHub Check: assemble (25, windows-latest)
- GitHub Check: assemble (25, macos-15)
- GitHub Check: assemble (25, ubuntu-latest)
- GitHub Check: assemble (25, ubuntu-24.04-arm)
- GitHub Check: detect-breaking-change
🔇 Additional comments (8)
CHANGELOG.md (1)
32-32: Changelog entry matches the fix scope.server/src/main/java/org/opensearch/gateway/ShardsBatchGatewayAllocator.java (1)
23-23: Import aligns with INDEX_CLOSED checks.server/src/main/java/org/opensearch/gateway/GatewayAllocator.java (2)
46-46: Import supports the new INDEX_CLOSED guard.
183-187: Correctly skips allocation for closed indices.server/src/test/java/org/opensearch/gateway/GatewayAllocatorTests.java (4)
26-33: Imports support the new test scenarios.
671-745: Good coverage for batch mode with INDEX_CLOSED shards.
747-881: Non-batch mode guard is well validated.
883-946: Control test for OPEN index batching is solid.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@server/src/main/java/org/opensearch/gateway/ShardsBatchGatewayAllocator.java`:
- Around line 404-412: Move the INDEX_CLOSED guard so closed-index shards are
handled before checking currentBatchedShards: in the unassigned.forEach(...)
lambda (the shardRouting loop), first check shardRouting.unassignedInfo() !=
null && shardRouting.unassignedInfo().getReason() ==
UnassignedInfo.Reason.INDEX_CLOSED and if true remove/evict that shardId from
currentBatchedShards (so stale batches are cleared, e.g., by calling the same
logic used by refreshShardBatches) and return; only after that test perform the
existing containsKey(shardRouting.shardId()) / primary checks and add to
newShardsToBatch as before. This ensures previously-batched INDEX_CLOSED shards
are not retained.
server/src/main/java/org/opensearch/gateway/ShardsBatchGatewayAllocator.java
Show resolved
Hide resolved
…hards Signed-off-by: Srikanth Padakanti <srikanth_padakanti@apple.com>
|
❌ Gradle check result for 24281a0: 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? |
|
❌ Gradle check result for 5fac3a4: 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? |
Description
Prevents allocation of shards from closed indices during cluster reroute operations. Added a check in
AllocationService.allocateExistingUnassignedShards()to skip shards belonging to closed indices, ensuring they remain unassigned until the index is explicitly reopened.Related Issues
Resolves #20200
Check List
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.