Skip to content

Conversation

Gagan6164
Copy link
Contributor

Description

Replace ChildMemoryCircuitBreaker with NoopCircuitBreaker for File Cache.

Related Issues

Resolves #19137

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.

@Gagan6164 Gagan6164 requested a review from a team as a code owner August 28, 2025 06:23
@github-actions github-actions bot added enhancement Enhancement or improvement to existing feature or request Search:Searchable Snapshots Storage Issues and PRs relating to data and metadata storage Storage:Resiliency Issues and PRs related to the storage resiliency labels Aug 28, 2025
Copy link
Contributor

❌ Gradle check result for 1fe8933: 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

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

@andrross
Copy link
Member

I think we should remove the circuit breaker from the code if it is not used in practice. You will have to keep the existing constructor, but that should be pretty simple, something like:

/**
 * @deprecated Use {@link FileCache(SegmentedCache<Path, CachedIndexInput>)}. CircuitBreaker parameter is not used.
 */
@Deprecated(forRemoval = true)
public FileCache(SegmentedCache<Path, CachedIndexInput> cache, CircuitBreaker circuitBreaker) {
    this(cache);
}

public FileCache(SegmentedCache<Path, CachedIndexInput> cache) {
    this.theCache = cache;
}

Copy link
Contributor

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

github-actions bot commented Sep 1, 2025

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

github-actions bot commented Sep 2, 2025

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

github-actions bot commented Sep 3, 2025

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

github-actions bot commented Sep 4, 2025

✅ Gradle check result for fa98373: SUCCESS

Copy link

codecov bot commented Sep 4, 2025

Codecov Report

❌ Patch coverage is 77.77778% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 72.87%. Comparing base (e0aebd9) to head (fa98373).
⚠️ Report is 3 commits behind head on main.

Files with missing lines Patch % Lines
...search/index/store/remote/filecache/FileCache.java 60.00% 2 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##               main   #19166      +/-   ##
============================================
+ Coverage     72.81%   72.87%   +0.06%     
- Complexity    69631    69674      +43     
============================================
  Files          5658     5658              
  Lines        320087   320079       -8     
  Branches      46345    46345              
============================================
+ Hits         233057   233246     +189     
+ Misses        68118    67914     -204     
- Partials      18912    18919       +7     

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

@gbbafna gbbafna merged commit fdf9f4e into opensearch-project:main Sep 4, 2025
31 checks passed
@github-project-automation github-project-automation bot moved this from 👀 In review to ✅ Done in Storage Project Board Sep 4, 2025
kh3ra pushed a commit to kh3ra/OpenSearch that referenced this pull request Sep 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Enhancement or improvement to existing feature or request Search:Searchable Snapshots skip-changelog Storage:Resiliency Issues and PRs related to the storage resiliency Storage Issues and PRs relating to data and metadata storage
Projects
Status: ✅ Done
Development

Successfully merging this pull request may close these issues.

[RFC] Remove Circuit Breaker check from File Cache
3 participants