Search coordinator uses event.ingested in cluster state to do rewrites#110352
Merged
quux00 merged 13 commits intoelastic:mainfrom Jul 11, 2024
Merged
Conversation
quux00
added
>enhancement
:Search/Search
Collaborator
|
Pinging @elastic/es-search (Team:Search) |
Collaborator
|
Hi @quux00, I've created a changelog YAML for you. |
quux00
commented
Jul 1, 2024
javanna
reviewed
Jul 8, 2024
Contributor
javanna
left a comment
javanna
left a comment
There was a problem hiding this comment.
I left a couple of comments on internal API design etc.
…ses-event.ingested-in-cluster-state
It now looks up time range from timeseries if the @timestamp range from cluster state is not useful (UKNOWN) or not ready (not all shards accounted for). It no longer also depends on eventIngested being in a non-ready/non-usable state.
…ses-event.ingested-in-cluster-state
…xt and replaced Map<String, DataFieldType> with it
…ses-event.ingested-in-cluster-state
javanna
reviewed
Jul 10, 2024
Contributor
javanna
left a comment
javanna
left a comment
There was a problem hiding this comment.
thanks, almost there, a couple more questions.
…and made it immutable
…ses-event.ingested-in-cluster-state
javanna
approved these changes
Jul 10, 2024
Contributor
javanna
left a comment
javanna
left a comment
There was a problem hiding this comment.
LGTM (left a small question, no need for another review round)
| * @return min timestamp for the field from IndexMetadata in cluster state. | ||
| */ | ||
| long getMinTimestamp(String fieldName) { | ||
| if (fieldName.equals(DataStream.TIMESTAMP_FIELD_NAME)) { |
Contributor
There was a problem hiding this comment.
super minor nitpick: fieldName can never be null right? I see that getMaxTimestamp does the equal the other way around, that's not going to throw NPE, while this one is different. Perhaps it does not matter, but I wondered why.
Contributor
Author
There was a problem hiding this comment.
Thanks, good catch. I'll make them consistent.
Contributor
Author
|
buildkite test this |
…ses-event.ingested-in-cluster-state
quux00
added a commit
to quux00/elasticsearch
that referenced
this pull request
Jul 11, 2024
elastic#110352) Min/max range for the event.ingested timestamp field (part of Elastic Common Schema) was added to IndexMetadata in cluster state for searchable snapshots in elastic#106252. This commit modifies the search coordinator to rewrite searches to MatchNone if the query searches a range of event.ingested that, from the min/max range in cluster state, is known to not overlap. This is the same behavior we currently have for the @timestamp field.
quux00
added a commit
that referenced
this pull request
Jul 11, 2024
#110352) (#110782) Min/max range for the event.ingested timestamp field (part of Elastic Common Schema) was added to IndexMetadata in cluster state for searchable snapshots in #106252. This commit modifies the search coordinator to rewrite searches to MatchNone if the query searches a range of event.ingested that, from the min/max range in cluster state, is known to not overlap. This is the same behavior we currently have for the @timestamp field.
martijnvg
reviewed
Jul 12, 2024
Member
martijnvg
left a comment
martijnvg
left a comment
There was a problem hiding this comment.
LGTM2 (I thought I clicked submit review)
| /** | ||
| * Data holder of timestamp fields held in cluster state IndexMetadata. | ||
| */ | ||
| public final class DateFieldRangeInfo { |
Member
There was a problem hiding this comment.
I think this can be a record?
quux00
added a commit
to quux00/elasticsearch
that referenced
this pull request
Jul 15, 2024
… rewrites (elastic#110352)" This reverts commit d45d164.
quux00
added a commit
to quux00/elasticsearch
that referenced
this pull request
Jul 15, 2024
… rewrites (elastic#110352) (elastic#110782)" This reverts commit 9092394.
quux00
added a commit
that referenced
this pull request
Jul 15, 2024
tvernum
pushed a commit
that referenced
this pull request
Feb 25, 2025
#110352) Min/max range for the event.ingested timestamp field (part of Elastic Common Schema) was added to IndexMetadata in cluster state for searchable snapshots in #106252. This commit modifies the search coordinator to rewrite searches to MatchNone if the query searches a range of event.ingested that, from the min/max range in cluster state, is known to not overlap. This is the same behavior we currently have for the @timestamp field.
tvernum
pushed a commit
that referenced
this pull request
Feb 25, 2025
#110352) Min/max range for the event.ingested timestamp field (part of Elastic Common Schema) was added to IndexMetadata in cluster state for searchable snapshots in #106252. This commit modifies the search coordinator to rewrite searches to MatchNone if the query searches a range of event.ingested that, from the min/max range in cluster state, is known to not overlap. This is the same behavior we currently have for the @timestamp field.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Min/max range for the event.ingested timestamp field (part of Elastic Common
Schema) was added to IndexMetadata in cluster state for searchable snapshots
in #106252.
This commit modifies the search coordinator to rewrite searches to MatchNone
if the query searches a range of event.ingested that, from the min/max range
in cluster state, is known to not overlap. This is the same behavior we currently
have for the
@timestampfield.