-
Notifications
You must be signed in to change notification settings - Fork 25.6k
Periodically check the available memory when fetching search hits source #121920
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
elasticsearchmachine
merged 70 commits into
elastic:main
from
andreidan:source-mem-accounting
Feb 24, 2025
Merged
Changes from 2 commits
Commits
Show all changes
70 commits
Select commit
Hold shift + click to select a range
8a37ca9
Account for the SearchHit source in circuit breaker
andreidan 9b524a3
Add source memory accounting for enrich source
andreidan 64a696a
spotless
andreidan 19592ef
test compile
andreidan 40fc178
Test compilation
andreidan 72d2c0e
test compile
andreidan 55f40ef
no sysout
andreidan 8b1f8c1
DecRef for newly created hit on circuit breaking exception
andreidan edaeebe
FetchPhase doc iterator lets CircuitBreakingException bubble up
andreidan 4e3d55f
Merge branch 'main' into source-mem-accounting
andreidan ff6ab24
DecRef after hit creation
andreidan c103451
Purge hits on CBE
andreidan d4b22e1
Enrich purge hits on CBE
andreidan 667873d
Merge branch 'main' into source-mem-accounting
andreidan 86dccd6
Merge branch 'main' into source-mem-accounting
elasticmachine fcf7711
Merge branch 'main' into source-mem-accounting
andreidan ffe9053
Use a mem accountign ref counted for the parent, unfiltered, source
andreidan 4083d64
Use unfiltered parent source ref counted
andreidan 8831a33
Compile
andreidan 6ffec2d
Compile
andreidan f4e111b
Use the existing refCounted field in SearchHit
andreidan b64bed5
Leaktracker.wrap
andreidan 69644ba
compile
andreidan 9aef36b
remove sout
andreidan 87308c9
Only call the breaker if bytes != 0
andreidan e5c366a
Merge branch 'main' into source-mem-accounting
andreidan 5a9c770
Local accounting up to 32kb
andreidan e3ea850
Merge branch 'main' into source-mem-accounting
elasticmachine 558d4af
Cosmetics and enhance test
andreidan b793459
Revert enrich modifications
andreidan 1062778
Revert enrich stuff
andreidan fbc4bb7
Release things before asserting in finally block
andreidan c89d103
Remove unused field
andreidan 35e8dce
Remove needless changes
andreidan d926d33
Update docs/changelog/121920.yaml
andreidan 3c57ced
Assert only if last ref in finally block
andreidan d3d2205
Update assert to account for fetch phase tripping the cb
andreidan 23f3794
Merge branch 'main' into source-mem-accounting
andreidan ee6d444
Default accounting buffer to 1M, drop int boxing.
andreidan a8e842a
Unify the in consumer and supplier in one interface
andreidan 64959b2
use lambda
andreidan 6c3a59d
Line length
andreidan f366e7f
Use volatile int in `MemoryAccountingBytesRefCounted`
andreidan 49df2a7
Remove field volatile and add some docs as to why
andreidan c0bdf15
Merge branch 'main' into source-mem-accounting
elasticmachine 86da7fd
Some docs for the functional interface
andreidan ebcba60
Move the mem accounting buffer size in aggregationcontext
andreidan 83e37e6
AssertBusy to allow for the decRefs to be dec-ed to 0
andreidan a8cf2c5
Merge branch 'main' into source-mem-accounting
andreidan 4e47ef6
Merge branch 'main' into source-mem-accounting
andreidan 9d8c238
Merge branch 'main' into source-mem-accounting
andreidan 31694f6
Merge branch 'main' into source-mem-accounting
andreidan de7492d
Use the real memory circuit breaker in the fetch phase
andreidan e7f2a3c
Compile
andreidan 1b0ca70
static
andreidan 979203a
Drop bool
andreidan 403720c
Fix mocks
andreidan 06dcb68
Have a checkRealMemoryCB method on the SearchContext
andreidan 41b74cb
Revert use of fetch phase source loader trip
andreidan 6e60c6e
Add unit test for the tracking memory in the fetch phase
andreidan 1469c33
[CI] Auto commit changes from spotless
39a0cc0
Unit test the search context checkRealMemoryCB method
andreidan abacdb1
Only check the breaker at the end of the segment if the source is req…
andreidan acf2f63
Merge branch 'main' into source-mem-accounting
andreidan fab8b9b
Account per fetch phase and add min value for the local acocunting bu…
andreidan 01f4c90
Spotless
andreidan 0be02e5
Drop the flag on finalizing local accounting
andreidan cff3ad8
Drop leftover
andreidan 47aac2a
Merge branch 'main' into source-mem-accounting
andreidan 13715a5
Renamings
andreidan File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -13,6 +13,8 @@ | |
| import org.elasticsearch.ElasticsearchParseException; | ||
| import org.elasticsearch.TransportVersions; | ||
| import org.elasticsearch.common.Strings; | ||
| import org.elasticsearch.common.breaker.CircuitBreaker; | ||
| import org.elasticsearch.common.breaker.NoopCircuitBreaker; | ||
| import org.elasticsearch.common.bytes.BytesArray; | ||
| import org.elasticsearch.common.bytes.BytesReference; | ||
| import org.elasticsearch.common.compress.CompressorFactory; | ||
|
|
@@ -83,6 +85,8 @@ public final class SearchHit implements Writeable, ToXContentObject, RefCounted | |
| private long primaryTerm; | ||
|
|
||
| private BytesReference source; | ||
| @Nullable | ||
| private BytesReference unfilteredSourceRef; | ||
|
|
||
| private final Map<String, DocumentField> documentFields; | ||
| private final Map<String, DocumentField> metaFields; | ||
|
|
@@ -110,21 +114,27 @@ public final class SearchHit implements Writeable, ToXContentObject, RefCounted | |
| private Map<String, SearchHits> innerHits; | ||
|
|
||
| private final RefCounted refCounted; | ||
| private final CircuitBreaker circuitBreaker; | ||
|
|
||
| // used only in tests | ||
| public SearchHit(int docId) { | ||
| this(docId, null); | ||
| public SearchHit(int docId, CircuitBreaker circuitBreaker) { | ||
| this(docId, null, circuitBreaker); | ||
| } | ||
|
|
||
| public SearchHit(int docId, String id) { | ||
| this(docId, id, null); | ||
| public SearchHit(int docId, String id, CircuitBreaker circuitBreaker) { | ||
| this(docId, id, null, circuitBreaker); | ||
| } | ||
|
|
||
| public SearchHit(int nestedTopDocId, String id, NestedIdentity nestedIdentity) { | ||
| this(nestedTopDocId, id, nestedIdentity, null); | ||
| public SearchHit(int nestedTopDocId, String id, NestedIdentity nestedIdentity, CircuitBreaker circuitBreaker) { | ||
| this(nestedTopDocId, id, nestedIdentity, null, circuitBreaker); | ||
| } | ||
|
|
||
| private SearchHit(int nestedTopDocId, String id, NestedIdentity nestedIdentity, @Nullable RefCounted refCounted) { | ||
| private SearchHit( | ||
| int nestedTopDocId, | ||
| String id, | ||
| NestedIdentity nestedIdentity, | ||
| @Nullable RefCounted refCounted, | ||
| @Nullable CircuitBreaker circuitBreaker | ||
| ) { | ||
| this( | ||
| nestedTopDocId, | ||
| DEFAULT_SCORE, | ||
|
|
@@ -145,7 +155,8 @@ private SearchHit(int nestedTopDocId, String id, NestedIdentity nestedIdentity, | |
| null, | ||
| new HashMap<>(), | ||
| new HashMap<>(), | ||
| refCounted | ||
| refCounted, | ||
| circuitBreaker | ||
| ); | ||
| } | ||
|
|
||
|
|
@@ -169,7 +180,8 @@ public SearchHit( | |
| Map<String, SearchHits> innerHits, | ||
| Map<String, DocumentField> documentFields, | ||
| Map<String, DocumentField> metaFields, | ||
| @Nullable RefCounted refCounted | ||
| @Nullable RefCounted refCounted, | ||
| @Nullable CircuitBreaker circuitBreaker | ||
| ) { | ||
| this.docId = docId; | ||
| this.score = score; | ||
|
|
@@ -191,6 +203,7 @@ public SearchHit( | |
| this.documentFields = documentFields; | ||
| this.metaFields = metaFields; | ||
| this.refCounted = refCounted == null ? LeakTracker.wrap(new SimpleRefCounted()) : refCounted; | ||
| this.circuitBreaker = circuitBreaker; | ||
| } | ||
|
|
||
| public static SearchHit readFrom(StreamInput in, boolean pooled) throws IOException { | ||
|
|
@@ -280,7 +293,8 @@ public static SearchHit readFrom(StreamInput in, boolean pooled) throws IOExcept | |
| innerHits, | ||
| documentFields, | ||
| metaFields, | ||
| isPooled ? null : ALWAYS_REFERENCED | ||
| isPooled ? null : ALWAYS_REFERENCED, | ||
| null | ||
| ); | ||
| } | ||
|
|
||
|
|
@@ -293,7 +307,8 @@ public static SearchHit unpooled(int docId, String id) { | |
| } | ||
|
|
||
| public static SearchHit unpooled(int nestedTopDocId, String id, NestedIdentity nestedIdentity) { | ||
| return new SearchHit(nestedTopDocId, id, nestedIdentity, ALWAYS_REFERENCED); | ||
| // always referenced search hits do NOT call #deallocate | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This might seem as noise but it helped me when making sense of the pool/unpool stuff so thought maybe it's useful to future spelunkers. |
||
| return new SearchHit(nestedTopDocId, id, nestedIdentity, ALWAYS_REFERENCED, null); | ||
| } | ||
|
|
||
| private static final Text SINGLE_MAPPING_TYPE = new Text(MapperService.SINGLE_MAPPING_NAME); | ||
|
|
@@ -447,6 +462,17 @@ public SearchHit sourceRef(BytesReference source) { | |
| return this; | ||
| } | ||
|
|
||
| /** | ||
| * We track the unfiltered, entire, source so we can release the entire size from the | ||
| * circuit breakers when the hit is released. | ||
| * The regular source might be a subset of the unfiltered source due to either | ||
| * source filtering, field collapsing or inner hits. | ||
| */ | ||
| public SearchHit unfilteredSourceRef(BytesReference source) { | ||
| this.unfilteredSourceRef = source; | ||
| return this; | ||
| } | ||
|
|
||
| /** | ||
| * Is the source available or not. A source with no fields will return true. This will return false if {@code fields} doesn't contain | ||
| * {@code _source} or if source is disabled in the mapping. | ||
|
|
@@ -724,6 +750,12 @@ private void deallocate() { | |
| r.decRef(); | ||
| } | ||
| SearchHit.this.source = null; | ||
|
|
||
| if (unfilteredSourceRef != null && circuitBreaker != null) { | ||
| System.out.println(" removing source loaded by inner hit " + unfilteredSourceRef.length()); | ||
| circuitBreaker.addWithoutBreaking(-unfilteredSourceRef.length()); | ||
| } | ||
| this.unfilteredSourceRef = null; | ||
| } | ||
|
|
||
| @Override | ||
|
|
@@ -758,7 +790,8 @@ public SearchHit asUnpooled() { | |
| : innerHits.entrySet().stream().collect(Collectors.toMap(Map.Entry::getKey, e -> e.getValue().asUnpooled())), | ||
| documentFields, | ||
| metaFields, | ||
| ALWAYS_REFERENCED | ||
| ALWAYS_REFERENCED, | ||
| null | ||
| ); | ||
| } | ||
|
|
||
|
|
||
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
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
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is wrong :)