Skip backing indices with a disjoint range on @timestamp field. #85162
Conversation
Implicitly skip backing indices with a time series range that doesn't match with a required filter on @timestamp field. Relates to elastic#74660
|
(this needs more tests) |
| } | ||
|
|
||
| boolean hasTimestampData() { | ||
| return indexLongFieldRange.isComplete() && indexLongFieldRange != IndexLongFieldRange.EMPTY; | ||
| return timeSeriesRange != null || (indexLongFieldRange.isComplete() && indexLongFieldRange != IndexLongFieldRange.EMPTY); |
There was a problem hiding this comment.
If it can be null maybe we need to check that in getMaxTimestamp?
There was a problem hiding this comment.
Ah! It can only be null if the range is complete. Safe enough.
There was a problem hiding this comment.
Err - why don't we just pass one of the two things in? Like, have indexLongFieldRange make a timeSeriesRange if it is complete?
There was a problem hiding this comment.
👍 Makes sense. I will do that and make this PR production ready.
martijnvg
added
>non-issue
:Data Management/Data streams
elasticmachine
added
Team:Data Management
|
Pinging @elastic/es-analytics-geo (Team:Analytics) |
|
Pinging @elastic/es-data-management (Team:Data Management) |
| */ | ||
| @Nullable | ||
| public TimeSeriesRange getTimeSeriesTimestampRange() { | ||
| if (IndexSettings.MODE.get(settings) == IndexMode.TIME_SERIES) { |
There was a problem hiding this comment.
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.
martijnvg
added
the
:Search/Search
|
Pinging @elastic/es-search (Team:Search) |
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.
|
run elasticsearch-ci/part-1 |
| 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)); |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Good point. I think I can reuse getTimestampBound(...) and then IndexMetadata#getTimeSeriesTimestampRange(...) use that to build a IndexLongFieldRange instance.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Ah! Would you be ok to merge this as is and then do some kind of refactoring to make combine the two paths somehow?
|
@elasticmachine update branch |
martijnvg
added
the
auto-merge-without-approval
…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
Implicitly skip backing indices with a time series range that doesn't match with
a required filter on @timestamp field.
Relates to #74660