Introduce dynamic runtime setting #65489

javanna · 2020-11-25T10:51:21Z

The dynamic:runtime setting is similar to dynamic:true in that it dynamically defines fields based on values parsed from incoming documents. Though instead of defining leaf fields under properties, it defines them as runtime fields under the runtime section. This is useful in scenarios where search speed can be traded for storage costs, given that runtime fields are loaded at runtime rather than indexed.

server/src/main/java/org/elasticsearch/index/mapper/DynamicFieldsBuilder.java

server/src/main/java/org/elasticsearch/index/mapper/DynamicRuntimeFieldsBuilder.java

server/src/main/java/org/elasticsearch/index/mapper/DocumentParser.java

nik9000 · 2020-11-30T18:07:08Z

server/src/main/java/org/elasticsearch/index/mapper/DocumentParser.java

-                builder = new BinaryFieldMapper.Builder(currentFieldName);
-            }
-            return builder;
+            return dynamicFieldsBuilder.newDynamicBinaryField(context, currentFieldName);


server/src/main/java/org/elasticsearch/index/mapper/DocumentParser.java

javanna · 2020-12-04T17:59:27Z

server/src/main/java/org/elasticsearch/index/mapper/DocumentParser.java

-        throw new IllegalStateException("Can't handle serializing a dynamic type with content token [" + token + "] and field name ["
-            + currentFieldName + "]");
-    }
-


this is all moved to DynamicFieldsBuilder now. it simplifies things as we no longer need to return either a runtime field or a mapper builder. We can directly do what's necessary, meaning adding the field where appropriate to the context, and potentially going further with parsing for concrete fields.

javanna · 2020-12-07T16:55:28Z

server/src/main/java/org/elasticsearch/index/mapper/DocumentParser.java

+        if (dynamicMappers.isEmpty() == false) {
+            root = createDynamicUpdate(mapping.root, docMapper, dynamicMappers);
+        } else {
+            root = (RootObjectMapper)mapping.root.mappingUpdate(null);


this is pretty horrible. I am not sure how to make it better yet. We are introducing one reason to update the root without having to add a mapper to it.

Passing null to make a copy is a bit sad, yeah. Maybe we could have a copy method?

I followed this approach and things do look a bit better, thanks!

javanna · 2020-12-07T16:58:34Z

server/src/main/java/org/elasticsearch/index/mapper/ParseContext.java

@@ -407,6 +432,9 @@ public void seqID(SeqNoFieldMapper.SequenceIDFields seqID) {

        @Override
        public void addDynamicMapper(Mapper mapper) {
+            if (mapper instanceof ObjectMapper) {
+                dynamicObjectMappers.put(mapper.name(), (ObjectMapper)mapper);
+            }


this is not particularly elegant, but it does the job. Ideas on how to make it nicer?

Not really.... We do a fair bit of `instanceof to dig up ObjectMapper and its friends like MetaDataFieldMapper. It ain't great but its what we do a fair bit.....

javanna · 2020-12-08T08:04:02Z

I am wondering whether I should update the qa tests as part of this PR or as a follow-up.

I figured that we are not ready yet to re-enable the qa tests for runtime fields defined in index mappings, as we also need to be able to define runtime fields as part of dynamic templates, which I will work on as a follow-up.

romseygeek

LGTM overall. I think we need to some work on DocumentParser as a whole and how it builds dynamic mappings but we can get this in first.

romseygeek · 2020-12-08T09:33:18Z

server/src/main/java/org/elasticsearch/index/mapper/DocumentParser.java

+        }
+        root.addRuntimeFields(dynamicRuntimeFields);
+        return mapping.mappingUpdate(root);
+    }


I think we should rework this to use ObjectMapper.Builder so that we can make the actual mappings immutable, but let's do that in a followup.

agreed, this improvement is already listed here: #64663

romseygeek · 2020-12-08T10:06:05Z

server/src/test/java/org/elasticsearch/index/mapper/DocumentParserTests.java

+        assertNull(doc.rootDoc().getField("foo.bar.baz"));
+        assertEquals("{\"_doc\":{\"dynamic\":\"false\"," +
+            "\"runtime\":{\"foo.bar.baz\":{\"type\":\"string\"},\"foo.baz\":{\"type\":\"string\"}}," +
+            "\"properties\":{\"foo\":{\"dynamic\":\"runtime\",\"properties\":{\"bar\":{\"type\":\"object\"}}}}}}",


Are we happy that this intermediate object with no concrete leaf fields gets added? It feels a bit weird to me, but I can see how it ends up being necessary because of all the book-keeping we do while we create dynamic mappings during parsing.

good question: I have wondered the same, and wondered also: what do we do when a document is sent which contains no leaf fields, only objects? Effectively no lucene fields (besides metadata fields) are created, yet we map the objects. I followed the same pattern here, which kind of fits in the runtime section design, as it is applied on top of mappings, hence it is natural that it does not hold objects, and that only the leaf fields get mapped in there. As a consequence, unseen objects get dynamically mapped under properties in dynamic runtime mode. I can see how this may feel weird at first glance, though I wonder what the alternatives are, and what the downsides could be of this approach.

replying to myself 6 months later: this would be the main downside of dynamically mapping objects under dynamic:runtime: #70268 . I am thinking now that it makes more sense to skip mapping objects in dynamic runtime mode.

romseygeek · 2020-12-08T10:15:36Z

.../src/main/java/org/elasticsearch/xpack/runtimefields/mapper/DynamicRuntimeFieldsBuilder.java

+
+    @Override
+    public RuntimeFieldType newDynamicDateField(String name, DateFormatter dateFormatter) {
+        return new DateScriptFieldType(name);


Shouldn't this take the date formatter into account?

good point, maybe. That brings up that we may need more testing with the real implementations.

javanna · 2020-12-08T13:50:22Z

run elasticsearch-ci/packaging-sample-unix

The dynamic:runtime setting is similar to dynamic:true in that it dynamically defines fields based on values parsed from incoming documents. Though instead of defining leaf fields under properties, it defines them as runtime fields under the runtime section. This is useful in scenarios where search speed can be traded for storage costs, given that runtime fields are loaded at runtime rather than indexed.

jtibshirani · 2020-12-09T23:19:28Z

I was catching up on this exciting change and noticed -- should we update the docs for the dynamic param?

javanna · 2020-12-10T08:02:44Z

@jtibshirani yes I was waiting for #62653 to be merged so we can go ahead and document dynamic:runtime too.

lockewritesdocs · 2020-12-10T22:54:26Z

@jtibshirani, I opened #66194 to include changes for dynamic runtime settings, which affects the dynamic parameter page (and a few others).

When we introduced dynamic:runtime (elastic#65489) we decided to have it create objects dynamically under properties, as the runtime section did not (and still does not) support object fields. That proved to be a poor choice, because the runtime section is flat, supports dots in field names, and does not really need objects. Also, these end up causing unnecessary mapping conflicts. With this commit we adapt dynamic:runtime to not dynamically create objects.

When we introduced dynamic:runtime (#65489) we decided to have it create objects dynamically under properties, as the runtime section did not (and still does not) support object fields. That proved to be a poor choice, because the runtime section is flat, supports dots in field names, and does not really need objects. Also, these end up causing unnecessary mapping conflicts. With this commit we adapt dynamic:runtime to not dynamically create objects. Closes #70268

When we introduced dynamic:runtime (elastic#65489) we decided to have it create objects dynamically under properties, as the runtime section did not (and still does not) support object fields. That proved to be a poor choice, because the runtime section is flat, supports dots in field names, and does not really need objects. Also, these end up causing unnecessary mapping conflicts. With this commit we adapt dynamic:runtime to not dynamically create objects. Closes elastic#70268

When we introduced dynamic:runtime (#65489) we decided to have it create objects dynamically under properties, as the runtime section did not (and still does not) support object fields. That proved to be a poor choice, because the runtime section is flat, supports dots in field names, and does not really need objects. Also, these end up causing unnecessary mapping conflicts. With this commit we adapt dynamic:runtime to not dynamically create objects. Closes #70268

javanna added 4 commits November 25, 2020 11:21

Merge branch 'master' into enhancement/dynamic_runtime_mode

5f037e4

iter

34ed399

iter

8b2bcaa

javanna added >enhancement :Search Foundations/Mapping Index mappings, including merging and defining field types v8.0.0 v7.11.0 labels Nov 25, 2020

javanna requested review from nik9000 and romseygeek November 25, 2020 10:51

iter

0841dc1

nik9000 reviewed Nov 30, 2020

View reviewed changes

javanna added 7 commits December 4, 2020 12:30

Merge branch 'master' into enhancement/dynamic_runtime_mode

07de311

checkstyle

d5587f7

iter

222321d

iter

e08844e

Merge branch 'master' into enhancement/dynamic_runtime_mode

26a559a

fix date creation

f528d26

iter

776fb4e

javanna commented Dec 4, 2020

View reviewed changes

javanna added 5 commits December 4, 2020 19:09

docs and rename

3a9e625

iter

8952c53

Merge branch 'master' into enhancement/dynamic_runtime_mode

edc6a29

iter and some tests

c2e9797

spotless

114b5f4

javanna commented Dec 7, 2020

View reviewed changes

javanna added 2 commits December 7, 2020 21:51

iter

9e6f13e

iter

613b882

javanna marked this pull request as ready for review December 7, 2020 21:08

romseygeek approved these changes Dec 8, 2020

View reviewed changes

javanna added 6 commits December 8, 2020 11:46

iter

1920acd

Merge branch 'master' into enhancement/dynamic_runtime_mode

7a557db

spotless

d10f0d5

fix date format and add tests

bcd0f50

spotless

7c600c3

one more test

540f335

javanna merged commit e144471 into elastic:master Dec 8, 2020

javanna mentioned this pull request Dec 8, 2020

Introduce dynamic runtime setting (#65489) #66041

Merged

lockewritesdocs mentioned this pull request Dec 10, 2020

[DOCS] Add dynamic runtime fields to docs #66194

Merged

lockewritesdocs mentioned this pull request Dec 14, 2020

[DOCS] Add dynamic runtime fields to docs (#66194) #66304

Merged

felixbarny mentioned this pull request Jan 18, 2021

dynamically index http.request.headers by default elastic/apm-server#4137

Open

javanna mentioned this pull request Jun 17, 2021

Dynamic runtime to not dynamically create objects #74234

Merged

javanna mentioned this pull request Jun 18, 2021

Dynamic runtime to not dynamically create objects (#74234) #74301

Merged

jakelandis added v8.0.0-alpha1 and removed v8.0.0 labels Jul 26, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Introduce dynamic runtime setting #65489

Introduce dynamic runtime setting #65489

javanna commented Nov 25, 2020

nik9000 Nov 30, 2020

javanna Dec 4, 2020

javanna Dec 7, 2020

nik9000 Dec 7, 2020

javanna Dec 8, 2020

javanna Dec 7, 2020

nik9000 Dec 7, 2020

javanna commented Dec 8, 2020

romseygeek left a comment

romseygeek Dec 8, 2020

javanna Dec 8, 2020

romseygeek Dec 8, 2020

javanna Dec 8, 2020

javanna Jun 17, 2021

romseygeek Dec 8, 2020

javanna Dec 8, 2020

javanna commented Dec 8, 2020

jtibshirani commented Dec 9, 2020

javanna commented Dec 10, 2020

lockewritesdocs commented Dec 10, 2020

Introduce dynamic runtime setting #65489

Introduce dynamic runtime setting #65489

Conversation

javanna commented Nov 25, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

javanna commented Dec 8, 2020

romseygeek left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

javanna commented Dec 8, 2020

jtibshirani commented Dec 9, 2020

javanna commented Dec 10, 2020

lockewritesdocs commented Dec 10, 2020