-
Notifications
You must be signed in to change notification settings - Fork 25.6k
EQL: add support for partial shard results #116388
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
Changes from 9 commits
4aa4a97
95f9a1b
d14c1f6
b72fd37
a86ab6a
18cfa60
8777fc8
4e63b3c
b6501cc
f052782
3e5439e
90fc499
35eb31e
b003c1c
8978a01
4c421c0
2ab3972
3ab9740
8207ea0
f3a1a65
c54a0c5
a8f5fb5
fcfa021
706935c
545e614
32a7aef
1e97b85
1da924c
3ebacb8
ed6b9a7
9f9eba8
7efff36
045d8da
672e512
e1e83a6
625acf4
7f36a69
984fe02
a1c903f
f58fd1c
3aba03a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
pr: 116388 | ||
summary: Add support for partial shard results | ||
area: EQL | ||
type: enhancement | ||
issues: [] |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -478,3 +478,34 @@ setup: | |
query: 'sequence with maxspan=10d [network where user == "ADMIN"] ![network where used == "SYSTEM"]' | ||
- match: { error.root_cause.0.type: "verification_exception" } | ||
- match: { error.root_cause.0.reason: "Found 1 problem\nline 1:75: Unknown column [used], did you mean [user]?" } | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Or maybe in this or a new yml file to add the more varied scenarios for "failed shards" queries that I mentioned above. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I added more cases to toml tests |
||
|
||
--- | ||
"Execute query with allow_partial_search_results": | ||
- do: | ||
eql.search: | ||
index: eql_test | ||
body: | ||
query: 'process where user == "SYSTEM"' | ||
fields: [{"field":"@timestamp","format":"epoch_millis"},"id","valid","day_of_week"] | ||
allow_partial_search_results: true | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Vary the placement of the parameter as a request parameter as well, not only the body of the request. |
||
|
||
- match: {timed_out: false} | ||
- match: {hits.total.value: 3} | ||
- match: {hits.total.relation: "eq"} | ||
- match: {hits.events.0._source.user: "SYSTEM"} | ||
- match: {hits.events.0._id: "1"} | ||
- match: {hits.events.0.fields.@timestamp: ["1580733296000"]} | ||
- match: {hits.events.0.fields.id: [123]} | ||
- match: {hits.events.0.fields.valid: [false]} | ||
- match: {hits.events.0.fields.day_of_week: ["Monday"]} | ||
- match: {hits.events.1._id: "2"} | ||
- match: {hits.events.1.fields.@timestamp: ["1580819696000"]} | ||
- match: {hits.events.1.fields.id: [123]} | ||
- match: {hits.events.1.fields.valid: [true]} | ||
- match: {hits.events.1.fields.day_of_week: ["Tuesday"]} | ||
- match: {hits.events.2._id: "3"} | ||
- match: {hits.events.2.fields.@timestamp: ["1580906096000"]} | ||
- match: {hits.events.2.fields.id: [123]} | ||
- match: {hits.events.2.fields.valid: [true]} | ||
- match: {hits.events.2.fields.day_of_week: ["Wednesday"]} | ||
|
Large diffs are not rendered by default.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -63,6 +63,7 @@ public class EqlSearchRequest extends ActionRequest implements IndicesRequest.Re | |
private List<FieldAndFormat> fetchFields; | ||
private Map<String, Object> runtimeMappings = emptyMap(); | ||
private int maxSamplesPerKey = RequestDefaults.MAX_SAMPLES_PER_KEY; | ||
private Boolean allowPartialSearchResults; | ||
|
||
// Async settings | ||
private TimeValue waitForCompletionTimeout = null; | ||
|
@@ -83,6 +84,7 @@ public class EqlSearchRequest extends ActionRequest implements IndicesRequest.Re | |
static final String KEY_FETCH_FIELDS = "fields"; | ||
static final String KEY_RUNTIME_MAPPINGS = "runtime_mappings"; | ||
static final String KEY_MAX_SAMPLES_PER_KEY = "max_samples_per_key"; | ||
static final String KEY_ALLOW_PARTIAL_SEARCH_RESULTS = "allow_partial_search_results"; | ||
|
||
static final ParseField FILTER = new ParseField(KEY_FILTER); | ||
static final ParseField TIMESTAMP_FIELD = new ParseField(KEY_TIMESTAMP_FIELD); | ||
|
@@ -97,6 +99,7 @@ public class EqlSearchRequest extends ActionRequest implements IndicesRequest.Re | |
static final ParseField RESULT_POSITION = new ParseField(KEY_RESULT_POSITION); | ||
static final ParseField FETCH_FIELDS_FIELD = SearchSourceBuilder.FETCH_FIELDS_FIELD; | ||
static final ParseField MAX_SAMPLES_PER_KEY = new ParseField(KEY_MAX_SAMPLES_PER_KEY); | ||
static final ParseField ALLOW_PARTIAL_SEARCH_RESULTS = new ParseField(KEY_ALLOW_PARTIAL_SEARCH_RESULTS); | ||
|
||
private static final ObjectParser<EqlSearchRequest, Void> PARSER = objectParser(EqlSearchRequest::new); | ||
|
||
|
@@ -135,6 +138,11 @@ public EqlSearchRequest(StreamInput in) throws IOException { | |
if (in.getTransportVersion().onOrAfter(TransportVersions.V_8_7_0)) { | ||
maxSamplesPerKey = in.readInt(); | ||
} | ||
if (in.getTransportVersion().onOrAfter(TransportVersions.EQL_ALLOW_PARTIAL_SEARCH_RESULTS)) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we have any tests for serialization to make sure in a mixed cluster tests everything is ok from this point of view? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. BWC serialization is unit tested in |
||
allowPartialSearchResults = in.readOptionalBoolean(); | ||
} else { | ||
allowPartialSearchResults = false; | ||
} | ||
} | ||
|
||
@Override | ||
|
@@ -245,6 +253,7 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws | |
builder.field(KEY_RUNTIME_MAPPINGS, runtimeMappings); | ||
} | ||
builder.field(KEY_MAX_SAMPLES_PER_KEY, maxSamplesPerKey); | ||
builder.field(KEY_ALLOW_PARTIAL_SEARCH_RESULTS, allowPartialSearchResults); | ||
|
||
return builder; | ||
} | ||
|
@@ -279,6 +288,7 @@ protected static <R extends EqlSearchRequest> ObjectParser<R, Void> objectParser | |
parser.declareField(EqlSearchRequest::fetchFields, EqlSearchRequest::parseFetchFields, FETCH_FIELDS_FIELD, ValueType.VALUE_ARRAY); | ||
parser.declareObject(EqlSearchRequest::runtimeMappings, (p, c) -> p.map(), SearchSourceBuilder.RUNTIME_MAPPINGS_FIELD); | ||
parser.declareInt(EqlSearchRequest::maxSamplesPerKey, MAX_SAMPLES_PER_KEY); | ||
parser.declareBoolean(EqlSearchRequest::allowPartialSearchResults, ALLOW_PARTIAL_SEARCH_RESULTS); | ||
return parser; | ||
} | ||
|
||
|
@@ -427,6 +437,15 @@ public EqlSearchRequest maxSamplesPerKey(int maxSamplesPerKey) { | |
return this; | ||
} | ||
|
||
public Boolean allowPartialSearchResults() { | ||
return allowPartialSearchResults; | ||
} | ||
|
||
public EqlSearchRequest allowPartialSearchResults(Boolean val) { | ||
this.allowPartialSearchResults = val; | ||
return this; | ||
} | ||
|
||
private static List<FieldAndFormat> parseFetchFields(XContentParser parser) throws IOException { | ||
List<FieldAndFormat> result = new ArrayList<>(); | ||
Token token = parser.currentToken(); | ||
|
@@ -470,6 +489,9 @@ public void writeTo(StreamOutput out) throws IOException { | |
if (out.getTransportVersion().onOrAfter(TransportVersions.V_8_7_0)) { | ||
out.writeInt(maxSamplesPerKey); | ||
} | ||
if (out.getTransportVersion().onOrAfter(TransportVersions.EQL_ALLOW_PARTIAL_SEARCH_RESULTS)) { | ||
out.writeOptionalBoolean(allowPartialSearchResults); | ||
} | ||
} | ||
|
||
@Override | ||
|
@@ -496,7 +518,8 @@ public boolean equals(Object o) { | |
&& Objects.equals(resultPosition, that.resultPosition) | ||
&& Objects.equals(fetchFields, that.fetchFields) | ||
&& Objects.equals(runtimeMappings, that.runtimeMappings) | ||
&& Objects.equals(maxSamplesPerKey, that.maxSamplesPerKey); | ||
&& Objects.equals(maxSamplesPerKey, that.maxSamplesPerKey) | ||
&& Objects.equals(allowPartialSearchResults, that.allowPartialSearchResults); | ||
} | ||
|
||
@Override | ||
|
@@ -517,7 +540,8 @@ public int hashCode() { | |
resultPosition, | ||
fetchFields, | ||
runtimeMappings, | ||
maxSamplesPerKey | ||
maxSamplesPerKey, | ||
allowPartialSearchResults | ||
); | ||
} | ||
|
||
|
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.
The ES search API supports partial results for async search as well, if I read the documentation correctly. Is there something stopping us for doing the same with EQL?
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.
I think when _search docs mention partial results at the beginning of the page, they refer to something slightly different, ie. the first part of a response that is still being calculated.
Search results can also be partial because of missing shards, that is the same we have in EQL, regardless of the request being sync or async.
This said, we definitely need some tests for async EQL queries with
allow_partial_search_results=true
. I'm adding them.