Fix listBlobsByPrefixInSortedOrder in EncryptedBlobContainer to adhere limit by Pranshu-S · Pull Request #20514 · opensearch-project/OpenSearch

Pranshu-S · 2026-01-31T13:20:24Z

Description

As S3BlobContainer already provides an implementation to directly get the latest blob after sorting in Lexicographic order, we do not need to load all the files to memory, sort and return the latest. This will help in not only improving bootstrapping time in cases when there is a manifest pile-up but also prevent JVM exhaustion

Before the change (~500k files in S3)

After the change

Related Issues

Resolves #20543 20543

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.

…e limit Signed-off-by: Pranshu Shukla <pranshushukla06@gmail.com>

coderabbitai · 2026-01-31T13:20:33Z

Important

Review skipped

Auto incremental reviews are disabled on this repository.

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.

🔍 Trigger a full review

📝 Walkthrough

Walkthrough

The changes introduce a synchronous variant of the listBlobsByPrefixInSortedOrder method to EncryptedBlobContainer that delegates to the underlying blob container and wraps results in encrypted metadata. A warning log is added to the base BlobContainer class when sorting blobs in memory. Comprehensive test coverage validates the new method's behavior and integration with RemoteManifestManager.

Changes

Cohort / File(s)	Summary
Logging & Core Implementation `server/src/main/java/org/opensearch/common/blobstore/BlobContainer.java`, `server/src/main/java/org/opensearch/common/blobstore/EncryptedBlobContainer.java`	Added LogManager import and warning log for in-memory blob sorting in BlobContainer. Added new public synchronous `listBlobsByPrefixInSortedOrder` method to EncryptedBlobContainer that delegates to underlying container and wraps each BlobMetadata in EncryptedBlobMetadata using existing cryptoHandler.
Test Coverage `server/src/test/java/org/opensearch/common/blobstore/EncryptedBlobContainerTests.java`, `server/src/test/java/org/opensearch/gateway/remote/RemoteManifestManagerTests.java`	Added three test methods validating the new synchronous method: basic functionality, null handling, and delegation verification. Added integration test verifying RemoteManifestManager properly delegates through EncryptedBlobContainer wrapper with correct parameters.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check	⚠️ Warning	The pull request description lacks essential details. It contains mostly placeholder template text and does not clearly explain what the PR achieves, the actual implementation changes, or how they solve the stated problem.	Replace template placeholder text with a clear description of: (1) what the PR accomplishes and why (performance optimization for limit adherence), (2) the specific implementation changes made to EncryptedBlobContainer and BlobContainer, and (3) how the changes prevent memory exhaustion and improve bootstrapping time.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately describes the main fix: improving listBlobsByPrefixInSortedOrder in EncryptedBlobContainer to respect the limit parameter.

_{✏️ 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

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

github-actions · 2026-01-31T14:50:44Z

✅ Gradle check result for 6872dc5: SUCCESS

codecov · 2026-01-31T14:51:23Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 73.31%. Comparing base (93ae8db) to head (5ca8325).
⚠️ Report is 8 commits behind head on main.

Additional details and impacted files

@@             Coverage Diff              @@
##               main   #20514      +/-   ##
============================================
- Coverage     73.33%   73.31%   -0.02%     
+ Complexity    72125    72119       -6     
============================================
  Files          5798     5798              
  Lines        329654   329731      +77     
  Branches      47491    47519      +28     
============================================
+ Hits         241741   241743       +2     
- Misses        68504    68549      +45     
- Partials      19409    19439      +30

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

rajiv-kv

Looks good. Some minor comments.

server/src/main/java/org/opensearch/common/blobstore/EncryptedBlobContainer.java

server/src/test/java/org/opensearch/gateway/remote/RemoteManifestManagerTests.java

…entation if it defaults to listing all blobs and sorting in memory Signed-off-by: Pranshu Shukla <pranshushukla06@gmail.com>

github-actions · 2026-02-03T08:11:19Z

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

Pranshu-S · 2026-02-03T08:13:02Z

Result -
Test Result (2 failures / +2)
org.opensearch.index.engine.CompositeIndexWriterForAppendTests.testConcurrentIndexAndDeleteDuringRefresh

Flaky - #20005

Signed-off-by: Pranshu Shukla <pranshushukla06@gmail.com>

github-actions · 2026-02-03T09:46:37Z

✅ Gradle check result for 7660098: SUCCESS

coderabbitai

Actionable comments posted: 1

🤖 Fix all issues with AI agents

In
`@server/src/test/java/org/opensearch/gateway/remote/RemoteManifestManagerTests.java`:
- Around line 128-137: The test is stubbing a real EncryptedBlobContainer
instance (encryptedBlobContainer.path()) which causes NotAMockException; replace
that stub with one on the underlying mock (mockBlobContainer.path()).
Specifically, remove the when(encryptedBlobContainer.path()) stub and add
when(mockBlobContainer.path()).thenReturn(blobPath); keep the
EncryptedBlobContainer constructed with new
EncryptedBlobContainer<>(mockBlobContainer, cryptoHandler) and continue
returning that instance from blobStore.blobContainer(any()) so
encryptedBlobContainer.path() will delegate to the mocked
mockBlobContainer.path().

📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 732c20f and 7660098.

📒 Files selected for processing (4)

server/src/main/java/org/opensearch/common/blobstore/BlobContainer.java
server/src/main/java/org/opensearch/common/blobstore/EncryptedBlobContainer.java
server/src/test/java/org/opensearch/common/blobstore/EncryptedBlobContainerTests.java
server/src/test/java/org/opensearch/gateway/remote/RemoteManifestManagerTests.java

🧰 Additional context used

🧠 Learnings (3)

📓 Common learnings

Learnt from: udabhas
Repo: opensearch-project/OpenSearch PR: 20095
File: server/src/main/java/org/opensearch/repositories/blobstore/BlobStoreRepository.java:4146-4165
Timestamp: 2026-01-14T09:31:12.773Z
Learning: For OpenSearch’s S3 path, index-level CryptoMetadata is passed from BlobStoreRepository to S3BlobContainer, and the final KMS key/context merge with repository settings is performed in SseKmsUtil.configureEncryptionSettings. Do not merge in BlobStoreRepository to avoid double-merging.

📚 Learning: 2026-01-14T09:31:12.773Z

Learnt from: udabhas
Repo: opensearch-project/OpenSearch PR: 20095
File: server/src/main/java/org/opensearch/repositories/blobstore/BlobStoreRepository.java:4146-4165
Timestamp: 2026-01-14T09:31:12.773Z
Learning: For OpenSearch’s S3 path, index-level CryptoMetadata is passed from BlobStoreRepository to S3BlobContainer, and the final KMS key/context merge with repository settings is performed in SseKmsUtil.configureEncryptionSettings. Do not merge in BlobStoreRepository to avoid double-merging.

Applied to files:

server/src/main/java/org/opensearch/common/blobstore/BlobContainer.java
server/src/test/java/org/opensearch/gateway/remote/RemoteManifestManagerTests.java
server/src/test/java/org/opensearch/common/blobstore/EncryptedBlobContainerTests.java

📚 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/common/blobstore/BlobContainer.java
server/src/main/java/org/opensearch/common/blobstore/EncryptedBlobContainer.java

🔇 Additional comments (5)

server/src/main/java/org/opensearch/common/blobstore/BlobContainer.java (1)

355-363: Helpful visibility into the in-memory fallback.

server/src/main/java/org/opensearch/common/blobstore/EncryptedBlobContainer.java (1)

251-268: Clean delegation and wrapping for the sync list API.

server/src/test/java/org/opensearch/common/blobstore/EncryptedBlobContainerTests.java (2)

12-25: Imports align with the new list-based tests.

255-313: Good coverage for the sync list-by-prefix behavior.

server/src/test/java/org/opensearch/gateway/remote/RemoteManifestManagerTests.java (1)

16-17: Imports are consistent with the new encrypted-wrapper test.

Also applies to: 42-43

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

server/src/test/java/org/opensearch/gateway/remote/RemoteManifestManagerTests.java

Signed-off-by: Pranshu Shukla <pranshushukla06@gmail.com>

…ntainer-list-all-objects Signed-off-by: Pranshu Shukla <pranshushukla06@gmail.com>

Signed-off-by: Pranshu Shukla <pranshushukla06@gmail.com>

github-actions · 2026-02-04T13:25:55Z

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

Signed-off-by: Pranshu Shukla <pranshushukla06@gmail.com>

Pranshu-S · 2026-02-04T14:53:01Z

Appears to be stuck

Job not started yet. Waiting for 60 seconds before next attempt.
time passed: 4680
Use queue information to find build number in Jenkins if available
jq: parse error: Invalid numeric literal at line 1, column 7

Signed-off-by: Pranshu Shukla <pranshushukla06@gmail.com>

github-actions · 2026-02-04T16:43:48Z

✅ Gradle check result for 5ca8325: SUCCESS

…e limit (opensearch-project#20514) * Fix listBlobsByPrefixInSortedOrder in EncryptedBlobContainer to adhere limit Signed-off-by: Pranshu Shukla <pranshushukla06@gmail.com>

Fix listBlobsByPrefixInSortedOrder in EncryptedBlobContainer to adher…

6872dc5

…e limit Signed-off-by: Pranshu Shukla <pranshushukla06@gmail.com>

Pranshu-S changed the title ~~Fix listBlobsByPrefixInSortedOrder in EncryptedBlobContainer to adher…~~ Fix listBlobsByPrefixInSortedOrder in EncryptedBlobContainer to adhere limit Jan 31, 2026

rajiv-kv reviewed Feb 2, 2026

View reviewed changes

chore: adding warn logs in listBlobsByPrefixInSortedOrder base implem…

7426255

…entation if it defaults to listing all blobs and sorting in memory Signed-off-by: Pranshu Shukla <pranshushukla06@gmail.com>

Retry Build

7660098

Signed-off-by: Pranshu Shukla <pranshushukla06@gmail.com>

Pranshu-S marked this pull request as ready for review February 3, 2026 10:27

Pranshu-S requested a review from a team as a code owner February 3, 2026 10:27

coderabbitai bot reviewed Feb 3, 2026

View reviewed changes

server/src/test/java/org/opensearch/gateway/remote/RemoteManifestManagerTests.java Show resolved Hide resolved

github-actions bot added bug Something isn't working Cluster Manager labels Feb 4, 2026

github-project-automation bot added this to Cluster Manager Project Board Feb 4, 2026

Pranshu-S added 3 commits February 4, 2026 17:30

Addressing comments

6b7fc06

Signed-off-by: Pranshu Shukla <pranshushukla06@gmail.com>

Merge remote-tracking branch 'origin/main' into fix-encrpyted-blob-co…

8cc42bf

…ntainer-list-all-objects Signed-off-by: Pranshu Shukla <pranshushukla06@gmail.com>

Add entry in CHANGELOG.md

8b785d1

Signed-off-by: Pranshu Shukla <pranshushukla06@gmail.com>

Retry Build

d3851a4

Signed-off-by: Pranshu Shukla <pranshushukla06@gmail.com>

Retry Build

5ca8325

Signed-off-by: Pranshu Shukla <pranshushukla06@gmail.com>

shwetathareja approved these changes Feb 5, 2026

View reviewed changes

github-project-automation bot moved this to 👀 In review in Cluster Manager Project Board Feb 5, 2026

shwetathareja merged commit ea01bb5 into opensearch-project:main Feb 5, 2026
34 checks passed

github-project-automation bot moved this from 👀 In review to ✅ Done in Cluster Manager Project Board Feb 5, 2026

Pranshu-S mentioned this pull request Feb 6, 2026

[BUG] Remote Cluster State cleanup failures due to deletions timeouts resulting in stale-metadata pile-ups #20564

Closed

Conversation

Pranshu-S commented Jan 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Related Issues

Check List

Uh oh!

coderabbitai bot commented Jan 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review skipped

Walkthrough

Changes

Estimated code review effort

Uh oh!

github-actions bot commented Jan 31, 2026

Uh oh!

codecov bot commented Jan 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

rajiv-kv left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented Feb 3, 2026

Uh oh!

Pranshu-S commented Feb 3, 2026

Uh oh!

github-actions bot commented Feb 3, 2026

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

github-actions bot commented Feb 4, 2026

Uh oh!

Pranshu-S commented Feb 4, 2026

Uh oh!

github-actions bot commented Feb 4, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Pranshu-S commented Jan 31, 2026 •

edited

Loading

coderabbitai bot commented Jan 31, 2026 •

edited

Loading

codecov bot commented Jan 31, 2026 •

edited

Loading