-
Notifications
You must be signed in to change notification settings - Fork 24.9k
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
Fix TextFieldMapper Retaining a Reference to its Builder #77251
Fix TextFieldMapper Retaining a Reference to its Builder #77251
Conversation
Fixes the text field mapper and the analyzers class that also retained parameter references that go really heavy. Makes `TextFieldMapper` take hundreds of bytes compared to multiple kb per instance. closes elastic#73845
@@ -1003,23 +1030,24 @@ protected void doXContentBody(XContentBuilder builder, Params params) throws IOE | |||
// this is a pain, but we have to do this to maintain BWC | |||
boolean includeDefaults = params.paramAsBoolean("include_defaults", false); | |||
builder.field("type", contentType()); | |||
this.builder.index.toXContent(builder, includeDefaults); | |||
this.builder.store.toXContent(builder, includeDefaults); | |||
final Builder b = (Builder) getMergeBuilder(); |
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.
Not the nicest approach to this I guess, but it should create the correct serialization as far as I understand?
Pinging @elastic/es-search (Team:Search) |
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.
LGTM
Thanks Alan! |
Fixes the text field mapper and the analyzers class that also retained parameter references that go really heavy. Makes `TextFieldMapper` take hundreds of bytes compared to multiple kb per instance. closes elastic#73845
* master: (128 commits) Mute DieWithDignityIT (elastic#77283) Fix randomization in MlNodeShutdownIT (elastic#77281) Add target_node_name for REPLACE shutdown type (elastic#77151) [DOCS] Adds information about version compatibility headers (elastic#77096) Fix template equals when mappings are wrapped (elastic#77008) Fix TextFieldMapper Retaining a Reference to its Builder (elastic#77251) Move die with dignity to be a test module (elastic#77136) Update task names for rest compatiblity (elastic#75267) [ML] adjusting bwc serialization for elastic#77256 (elastic#77257) Move `index.hidden` from Static to Dynamic settings (elastic#77218) Handle cgroups v2 in `OsProbe` (elastic#77128) Choose postings format from FieldMapper instead of MappedFieldType (elastic#77234) Add segment sorter for data streams (elastic#75195) Update skip after backport (elastic#77212) [ML] adding new defer_definition_decompression parameter to put trained model API (elastic#77189) [ML] Fix bug in inference stats persister for when feature reset is called Only check replicas in cancelling existing recoveries. (elastic#60564) Format `AbstractFilteringTestCase` (elastic#77217) [DOCS] Fixes line breaks. (elastic#77248) Convert 'routing' values in REST API tests to strings ... # Conflicts: # server/src/main/java/org/elasticsearch/cluster/metadata/DataStream.java
Fixes the text field mapper and the analyzers class that also retained parameter references that go really heavy.
Makes
TextFieldMapper
take ~200b for the text fields in the beats mapping, compared to ~5kb per instance before.closes #73845