Skip backing indices with a disjoint range on @timestamp field. by martijnvg · Pull Request #85162 · elastic/elasticsearch

martijnvg · 2022-03-21T13:49:26Z

Implicitly skip backing indices with a time series range that doesn't match with
a required filter on @timestamp field.

Relates to #74660

@timestamp

Implicitly skip backing indices with a time series range that doesn't match with a required filter on @timestamp field. Relates to elastic#74660

martijnvg · 2022-03-23T11:00:36Z

(this needs more tests)

nik9000 · 2022-03-29T15:07:16Z

server/src/main/java/org/elasticsearch/index/query/CoordinatorRewriteContext.java


    boolean hasTimestampData() {
-        return indexLongFieldRange.isComplete() && indexLongFieldRange != IndexLongFieldRange.EMPTY;
+        return timeSeriesRange != null || (indexLongFieldRange.isComplete() && indexLongFieldRange != IndexLongFieldRange.EMPTY);


If it can be null maybe we need to check that in getMaxTimestamp?

And call it @Nullable?

Ah! It can only be null if the range is complete. Safe enough.

Err - why don't we just pass one of the two things in? Like, have indexLongFieldRange make a timeSeriesRange if it is complete?

👍 Makes sense. I will do that and make this PR production ready.

elasticmachine · 2022-05-10T13:03:30Z

Pinging @elastic/es-analytics-geo (Team:Analytics)

elasticmachine · 2022-05-10T13:04:33Z

Pinging @elastic/es-data-management (Team:Data Management)

nik9000 · 2022-05-10T13:15:40Z

server/src/main/java/org/elasticsearch/cluster/metadata/IndexMetadata.java

+     */
+    @Nullable
+    public TimeSeriesRange getTimeSeriesTimestampRange() {
+        if (IndexSettings.MODE.get(settings) == IndexMode.TIME_SERIES) {


Maybe it'd make sense to name this method getConfiguredTimestampRange.

Also, I'd prefer a method on IndexMode for this rather than a hard check. I'm trying to keep the number of == checks on IndexMode to a minimum so you can look at the implementation of IndexMode and see all of the kinds of changes in behavior that it can make to indices.

elasticmachine · 2022-06-27T15:01:19Z

Pinging @elastic/es-search (Team:Search)

…index settings

we have an entry in TimestampFieldMapperService, so that search request with a required range query on `@timestamp` field can skip indices/shards on coordinating node.

martijnvg · 2022-06-29T13:58:27Z

run elasticsearch-ci/part-1

nik9000 · 2022-06-29T20:41:52Z

server/src/main/java/org/elasticsearch/index/IndexMode.java

+        public IndexLongFieldRange getConfiguredTimestampRange(IndexMetadata indexMetadata) {
+            long min = indexMetadata.getTimeSeriesStart().toEpochMilli();
+            long max = indexMetadata.getTimeSeriesEnd().toEpochMilli();
+            return IndexLongFieldRange.NO_SHARDS.extendWithShardRange(0, 1, ShardLongFieldRange.of(min, max));


Hey. So. Like. I think I made getTimestampBound and it does like nearly the same thing, right? I feel like that method should be involved in this PR somehow. Or maybe IndexSettings.timestampBounds should be.

Good point. I think I can reuse getTimestampBound(...) and then IndexMetadata#getTimeSeriesTimestampRange(...) use that to build a IndexLongFieldRange instance.

TimestampBounds needs IndexScopedSettings which I don't have in the context In want to use it.
This technically only needed for dynamically updating endTime field, which is my case isn't needed.
But maybe with a minor change to TimestampBounds, I can use it too.

Ah! Would you be ok to merge this as is and then do some kind of refactoring to make combine the two paths somehow?

Yes, I can do that.

martijnvg · 2022-07-04T08:41:57Z

@elasticmachine update branch

…tTimestampBound(...) `IndexMode#getConfiguredTimestampRange(...)` functionally overlaps a lot with `IndexMode#getTimestampBound(...)`. In order to reuse `IndexMode#getTimestampBound(...)` for the use case `IndexMode#getConfiguredTimestampRange(...)` is reused a change had to be made to TimestampBounds. During query rewrite and in `TimestampFieldMapperService` no `IndexScopedSettings` is available, so made this an optional parameter. Only the usage in `IndexSettings` needs this, so that the instance is updated in the case `index.time_series.end_time` setting is updated. In the case of query rewrite and in `TimestampFieldMapperService` this isn't needed, since the instance is kept around and reused. Followup from elastic#85162

…tTimestampBound(...) (#88258) `IndexMode#getConfiguredTimestampRange(...)` functionally overlaps a lot with `IndexMode#getTimestampBound(...)`. In order to reuse `IndexMode#getTimestampBound(...)` for the use case `IndexMode#getConfiguredTimestampRange(...)` is reused a change had to be made to TimestampBounds. During query rewrite and in `TimestampFieldMapperService` no `IndexScopedSettings` is available, so made this an optional parameter. Only the usage in `IndexSettings` needs this, so that the instance is updated in the case `index.time_series.end_time` setting is updated. In the case of query rewrite and in `TimestampFieldMapperService` this isn't needed, since the instance is kept around and reused. Followup from #85162

Skip backing indices with a disjoint range on @timestamp field.

1a130aa

Implicitly skip backing indices with a time series range that doesn't match with a required filter on @timestamp field. Relates to elastic#74660

elasticsearchmachine added the v8.2.0 label Mar 21, 2022

martijnvg added 4 commits March 22, 2022 13:51

fix mistake

9db31c1

Merge remote-tracking branch 'es/master' into skip_backing_indices

2b0e37c

added first test

ac5cb16

checkstyle

e0fc357

martijnvg requested a review from nik9000 March 23, 2022 11:00

nik9000 reviewed Mar 29, 2022

View reviewed changes

salvatore-campagna added v8.3.0 and removed v8.2.0 labels Mar 30, 2022

martijnvg added 5 commits May 4, 2022 14:45

Merge remote-tracking branch 'es/master' into skip_backing_indices

82c41e2

fix test compile error after merging in master

a90bd47

Merge remote-tracking branch 'es/master' into skip_backing_indices

025e05e

iter

74277ba

spotless

8314534

nik9000 mentioned this pull request May 9, 2022

Min and max time for *any* index? #86579

Closed

martijnvg added 2 commits May 10, 2022 14:53

iter

7e9a877

Merge remote-tracking branch 'es/master' into skip_backing_indices

b5d0d7d

martijnvg marked this pull request as ready for review May 10, 2022 12:59

martijnvg added >non-issue :StorageEngine/Data streams Data streams and their lifecycles :StorageEngine/TSDB You know, for Metrics labels May 10, 2022

martijnvg requested a review from nik9000 May 10, 2022 12:59

elasticmachine added Team:Data Management (obsolete) DO NOT USE. This team no longer exists. Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) labels May 10, 2022

nik9000 reviewed May 10, 2022

View reviewed changes

craigtaverner added v8.4.0 and removed v8.3.0 labels May 25, 2022

martijnvg added 4 commits June 17, 2022 17:32

Merge remote-tracking branch 'es/master' into skip_backing_indices

714f629

fix test compile error after merging in master

de2e3ed

Merge remote-tracking branch 'es/master' into skip_backing_indices

c78845e

Merge remote-tracking branch 'es/master' into skip_backing_indices

a4845e3

martijnvg added the :Search/Search Search-related issues that do not fall into other categories label Jun 27, 2022

elasticmachine added the Team:Search Meta label for search team label Jun 27, 2022

martijnvg added 9 commits June 28, 2022 11:34

use start and end time from IndexMetadata instead of parsing it from …

0034d7a

…index settings

Add unit like test that verifies that for tsdb data stream

b475761

we have an entry in TimestampFieldMapperService, so that search request with a required range query on `@timestamp` field can skip indices/shards on coordinating node.

Merge remote-tracking branch 'es/master' into skip_backing_indices

68539e1

spotless

c5ecf62

fix bug

ffb33ec

Merge remote-tracking branch 'es/master' into skip_backing_indices

710bb28

handle when index mode is provided in upper case

25b3bb3

Merge remote-tracking branch 'es/master' into skip_backing_indices

ca5893b

spotless

679e699

nik9000 reviewed Jun 29, 2022

View reviewed changes

nik9000 approved these changes Jun 30, 2022

View reviewed changes

Merge branch 'master' into skip_backing_indices

f09ffe7

martijnvg added the auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) label Jul 4, 2022

elasticsearchmachine merged commit 577d3e2 into elastic:master Jul 4, 2022

martijnvg deleted the skip_backing_indices branch July 4, 2022 09:39

martijnvg mentioned this pull request Jul 4, 2022

Collapse IndexMode#getConfiguredTimestampRange(...) into IndexMode#getTimestampBound(...) #88258

Merged

Conversation

martijnvg commented Mar 21, 2022

Uh oh!

martijnvg commented Mar 23, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

elasticmachine commented May 10, 2022

Uh oh!

elasticmachine commented May 10, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

elasticmachine commented Jun 27, 2022

Uh oh!

martijnvg commented Jun 29, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

martijnvg commented Jul 4, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants