-
Notifications
You must be signed in to change notification settings - Fork 25k
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
Introduce TimeSeriesRoutingIdFieldMapper and use it to create TSDB ids #106080
Conversation
# Conflicts: # server/src/test/java/org/elasticsearch/index/mapper/RootObjectMapperTests.java
# Conflicts: # modules/data-streams/src/yamlRestTest/resources/rest-api-spec/test/data_stream/150_tsdb.yml
@elasticsearchmachine run elasticsearch-ci/bwc-snapshots |
I think the engine level changes in this PR can be undone and use routing based on index mode in the mapping / document parser: martijnvg@609405c The tests in server and data stream module passed with this change. |
This reverts commit e916599
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 getting close! My main concern is around the change in yml test.
modules/reindex/src/yamlRestTest/resources/rest-api-spec/test/update_by_query/90_tsdb.yml
Outdated
Show resolved
Hide resolved
server/src/main/java/org/elasticsearch/cluster/routing/IndexRouting.java
Outdated
Show resolved
Hide resolved
...les/lang-painless/src/main/java/org/elasticsearch/painless/action/PainlessExecuteAction.java
Outdated
Show resolved
Hide resolved
server/src/main/java/org/elasticsearch/cluster/routing/IndexRouting.java
Outdated
Show resolved
Hide resolved
@elasticsearchmachine run elasticsearch-ci/part-1 |
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 🎉
...les/lang-painless/src/main/java/org/elasticsearch/painless/action/PainlessExecuteAction.java
Outdated
Show resolved
Hide resolved
Thanks for the patience and the thorough review! |
Supporting non-keyword fields requires updating non-keyword fields in the routing path to be included in routing calculations. Routing is performed in coordinating nodes that lack mappings (or mappings haven't been created yet, for dynamically-defined dimensions), so the routing hash they calculate are passed to data nodes and stored in a new fields, namely _ts_routind_hash. This is included in the _id field, in turn, so that it can consistently reach the right shard for get-by-id and delete-by-id operations.
A few interesting points:
routing
field inIndexRequest
; adding another field to the latter requires updating dozens of classes.id
prefix.Related to #103567