Fix AsyncSearchIndexServiceTests.testCircuitBreaker failure#144058
Merged
chrisparrinello merged 8 commits intoelastic:mainfrom Mar 13, 2026
Merged
Fix AsyncSearchIndexServiceTests.testCircuitBreaker failure#144058chrisparrinello merged 8 commits intoelastic:mainfrom
chrisparrinello merged 8 commits intoelastic:mainfrom
Conversation
elasticsearchmachine
added
the
needs:triage
chrisparrinello
added
>test
Collaborator
|
Pinging @elastic/es-search-foundations (Team:Search Foundations) |
drempapis
reviewed
Mar 12, 2026
| * ByteArrayWrapper shallow size (object header + fields) + RamUsageEstimator.sizeOf(array), which exceeds PAGE_SIZE_IN_BYTES. | ||
| */ | ||
| private static final int MIN_CIRCUIT_BREAKER_LIMIT_FOR_INITIAL_BUFFER = Math.toIntExact( | ||
| RamUsageEstimator.sizeOf(new byte[PageCacheRecycler.PAGE_SIZE_IN_BYTES]) + 64L |
Contributor
There was a problem hiding this comment.
Originally, we had 16* 1024 = 16384
In the new implementation, PAGE_SIZE_IN_BYTES = 1 << 14 -> 16384 (I guess that was the reason added the 16*1024 in the tests)
The RamUsageEstimator.sizeOf(new byte[PageCacheRecycler.PAGE_SIZE_IN_BYTES]) returns 16400
Why do we add +64 in the formula?
Contributor
Author
There was a problem hiding this comment.
That buffer is wrapped in a ByteArrayWrapper whose size is also taken into account in the circuit breaker. If the random circuit breaker size is on the lower end between PAGE_SIZE_IN_BYTES and PAGE_SIZE_IN_BYTES plus the overhead, the test fails.
I dug into the code some more and 64 might be a little high. Running RamUsageEstimator.shallowSizeOfInstance(ByteArrayWrapper.class) shows a size of 40 bytes. I've changed to that and ran 1k iterations without a failure.
drempapis
approved these changes
Mar 12, 2026
Contributor
drempapis
left a comment
drempapis
left a comment
There was a problem hiding this comment.
LGTM! Thank you Chris for the details
szybia
added a commit
to szybia/elasticsearch
that referenced
this pull request
Mar 13, 2026
…elocations * upstream/main: (72 commits) [Test] Randomly disable sequence numbers in CcrTimeSeriesDataStreamsIT (elastic#143930) Fix AsyncSearchIndexServiceTests.testCircuitBreaker failure (elastic#144058) Refine GenerativeIT some more, this time with accounting for some added (elastic#144220) ESQL: Physical Planning on the Lookup Node (elastic#143707) Mute org.elasticsearch.xpack.esql.CsvIT test {csv-spec:approximation.Approximate stats by with zero variance} elastic#144240 Trigger counter metrics in test for delta temporality measurements (elastic#144193) fix capabiltiy approximation_v3 (elastic#144230) [ci] Add PR pipeline for testing ipv6 and fix tests not working with ipv6 (elastic#140473) update (elastic#144095) Make from/to optional in TBUCKET when Kibana timestamp filter is present (elastic#144057) Extract reroute behavior from create-index request classes (elastic#144140) ESQL: Fix release build only failures (elastic#144122) ES|QL query approximation: move sample correction to data node (elastic#144005) Add indexing pressure tracking to OTLP endpoints (elastic#144009) Fix replica writes after _seq_no doc values are pruned (elastic#144180) allow tests to configure supportsLoadingConfig (elastic#144061) [ES|QL] Unmute testGiantTextFieldInSubqueryIntermediateResultsWithSort (elastic#144126) [ESQL][DOCS] Add CPS page (unpublished for moment) (elastic#144206) ESQL: Forbid "load" unmapped_fields for certain commands (elastic#144115) Add CCS Remote Views Detection (elastic#143384) ...
ncordon
pushed a commit
to ncordon/elasticsearch
that referenced
this pull request
Mar 16, 2026
…144058) Closes elastic#143411. The circuit breaker test did not take into account the overhead in ByteArrayWrapper when picking a size of the circuit breaker to be large enough to accommodate the page and ByteArrayWrapper. With an unlucky random seed, the test would fail immediately for large enough pages.
michalborek
pushed a commit
to michalborek/elasticsearch
that referenced
this pull request
Mar 23, 2026
…144058) Closes elastic#143411. The circuit breaker test did not take into account the overhead in ByteArrayWrapper when picking a size of the circuit breaker to be large enough to accommodate the page and ByteArrayWrapper. With an unlucky random seed, the test would fail immediately for large enough pages.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #143411.
The circuit breaker test did not take into account the overhead in
ByteArrayWrapperwhen picking a size of the circuit breaker to be large enough to accommodate the page andByteArrayWrapper. With an unlucky random seed, the test would fail immediately for large enough pages.