allow_partial_sequence_results=false by default

luigidellaquila · Nov 21, 2024 · b003c1c · b003c1c
1 parent 35eb31e
commit b003c1c
Show file tree

Hide file tree

Showing 5 changed files with 37 additions and 41 deletions.
diff --git a/docs/reference/eql/eql-search-api.asciidoc b/docs/reference/eql/eql-search-api.asciidoc
@@ -114,16 +114,19 @@ If both parameters are specified, only the query parameter is used.
 
 
 Used together with `allow_partial_search_results=true`, controls the behavior of sequence queries specifically
-(if `allow_partial_search_results=false` this setting has no effect).
-If `true` and if some shards are unavailable, the sequences are calculated on available shards.
+(if `allow_partial_search_results=false`, this setting has no effect).
+If `true` and if some shards are unavailable, the sequences are calculated on available shards only.
 +
 If `false` and if some shards are unavailable, the query only returns information about the shard failures,
 but no further results.
 +
-Defaults to `true`.
+Defaults to `false`.
++
+Consider that sequences calculated with `allow_partial_search_results=true` can return incorrect results
+(eg. if a <<eql-missing-events, missing event>> clause matches records in unavailable shards)
 +
 To override the default for this field, set the
-`xpack.eql.default_allow_partial_sequence_results` cluster setting to `false`.
+`xpack.eql.default_allow_partial_sequence_results` cluster setting to `true`.
 
 
 [IMPORTANT]

diff --git a/...c/internalClusterTest/java/org/elasticsearch/xpack/eql/action/PartialSearchResultsIT.java b/...c/internalClusterTest/java/org/elasticsearch/xpack/eql/action/PartialSearchResultsIT.java
@@ -248,6 +248,7 @@ public void testPartialResults() throws Exception {
 
         // ------------------------------------------------------------------------
         // same queries, with missing shards and allow_partial_search_results=true
+        // and allow_partial_sequence_result=true
         // ------------------------------------------------------------------------
 
         // event query
@@ -262,7 +263,8 @@ public void testPartialResults() throws Exception {
         // sequence query on both shards
         request = new EqlSearchRequest().indices("test-*")
             .query("sequence [process where value == 1] [process where value == 2]")
-            .allowPartialSearchResults(true);
+            .allowPartialSearchResults(true)
+            .allowPartialSequenceResults(true);
         response = client().execute(EqlSearchAction.INSTANCE, request).get();
         assertThat(response.hits().sequences().size(), equalTo(0));
         assertThat(response.shardFailures().length, is(1));
@@ -272,7 +274,8 @@ public void testPartialResults() throws Exception {
         // sequence query on the available shard only
         request = new EqlSearchRequest().indices("test-*")
             .query("sequence [process where value == 1] [process where value == 3]")
-            .allowPartialSearchResults(true);
+            .allowPartialSearchResults(true)
+            .allowPartialSequenceResults(true);
         response = client().execute(EqlSearchAction.INSTANCE, request).get();
         assertThat(response.hits().sequences().size(), equalTo(1));
         sequence = response.hits().sequences().get(0);
@@ -285,7 +288,8 @@ public void testPartialResults() throws Exception {
         // sequence query on the unavailable shard only
         request = new EqlSearchRequest().indices("test-*")
             .query("sequence [process where value == 0] [process where value == 2]")
-            .allowPartialSearchResults(true);
+            .allowPartialSearchResults(true)
+            .allowPartialSequenceResults(true);
         response = client().execute(EqlSearchAction.INSTANCE, request).get();
         assertThat(response.hits().sequences().size(), equalTo(0));
         assertThat(response.shardFailures().length, is(1));
@@ -295,7 +299,8 @@ public void testPartialResults() throws Exception {
         // sequence query with missing event on unavailable shard. THIS IS A FALSE POSITIVE
         request = new EqlSearchRequest().indices("test-*")
             .query("sequence with maxspan=10s  [process where value == 1] ![process where value == 2] [process where value == 3]")
-            .allowPartialSearchResults(true);
+            .allowPartialSearchResults(true)
+            .allowPartialSequenceResults(true);
         response = client().execute(EqlSearchAction.INSTANCE, request).get();
         assertThat(response.hits().sequences().size(), equalTo(1));
         sequence = response.hits().sequences().get(0);
@@ -308,7 +313,8 @@ public void testPartialResults() throws Exception {
         // sample query on both shards
         request = new EqlSearchRequest().indices("test-*")
             .query("sample by key [process where value == 2] [process where value == 1]")
-            .allowPartialSearchResults(true);
+            .allowPartialSearchResults(true)
+            .allowPartialSequenceResults(true);
         response = client().execute(EqlSearchAction.INSTANCE, request).get();
         assertThat(response.hits().sequences().size(), equalTo(0));
         assertThat(response.shardFailures().length, is(1));
@@ -318,7 +324,8 @@ public void testPartialResults() throws Exception {
         // sample query on the available shard only
         request = new EqlSearchRequest().indices("test-*")
             .query("sample by key [process where value == 3] [process where value == 1]")
-            .allowPartialSearchResults(true);
+            .allowPartialSearchResults(true)
+            .allowPartialSequenceResults(true);
         response = client().execute(EqlSearchAction.INSTANCE, request).get();
         assertThat(response.hits().sequences().size(), equalTo(1));
         sample = response.hits().sequences().get(0);
@@ -331,22 +338,21 @@ public void testPartialResults() throws Exception {
         // sample query on the unavailable shard only
         request = new EqlSearchRequest().indices("test-*")
             .query("sample by key [process where value == 2] [process where value == 0]")
-            .allowPartialSearchResults(true);
+            .allowPartialSearchResults(true)
+            .allowPartialSequenceResults(true);
         response = client().execute(EqlSearchAction.INSTANCE, request).get();
         assertThat(response.hits().sequences().size(), equalTo(0));
         assertThat(response.shardFailures().length, is(1));
         assertThat(response.shardFailures()[0].index(), is("test-1"));
         assertThat(response.shardFailures()[0].reason(), containsString("NoShardAvailableActionException"));
 
         // ------------------------------------------------------------------------
-        // same queries, with missing shards and allow_partial_search_results=true and allow_partial_sequence_results=false
+        // same queries, with missing shards and allow_partial_search_results=true
+        // and default allow_partial_sequence_results (ie. false)
         // ------------------------------------------------------------------------
 
         // event query
-        request = new EqlSearchRequest().indices("test-*")
-            .query("process where true")
-            .allowPartialSearchResults(true)
-            .allowPartialSequenceResults(false);
+        request = new EqlSearchRequest().indices("test-*").query("process where true").allowPartialSearchResults(true);
         response = client().execute(EqlSearchAction.INSTANCE, request).get();
         assertThat(response.hits().events().size(), equalTo(5));
         for (int i = 0; i < 5; i++) {
@@ -357,8 +363,7 @@ public void testPartialResults() throws Exception {
         // sequence query on both shards
         request = new EqlSearchRequest().indices("test-*")
             .query("sequence [process where value == 1] [process where value == 2]")
-            .allowPartialSearchResults(true)
-            .allowPartialSequenceResults(false);
+            .allowPartialSearchResults(true);
         response = client().execute(EqlSearchAction.INSTANCE, request).get();
         assertThat(response.hits().sequences().size(), equalTo(0));
         assertThat(response.shardFailures().length, is(1));
@@ -368,8 +373,7 @@ public void testPartialResults() throws Exception {
         // sequence query on the available shard only
         request = new EqlSearchRequest().indices("test-*")
             .query("sequence [process where value == 1] [process where value == 3]")
-            .allowPartialSearchResults(true)
-            .allowPartialSequenceResults(false);
+            .allowPartialSearchResults(true);
         response = client().execute(EqlSearchAction.INSTANCE, request).get();
         assertThat(response.hits().sequences().size(), equalTo(0));
         assertThat(response.shardFailures().length, is(1));
@@ -379,8 +383,7 @@ public void testPartialResults() throws Exception {
         // sequence query on the unavailable shard only
         request = new EqlSearchRequest().indices("test-*")
             .query("sequence [process where value == 0] [process where value == 2]")
-            .allowPartialSearchResults(true)
-            .allowPartialSequenceResults(false);
+            .allowPartialSearchResults(true);
         response = client().execute(EqlSearchAction.INSTANCE, request).get();
         assertThat(response.hits().sequences().size(), equalTo(0));
         assertThat(response.shardFailures().length, is(1));
@@ -390,8 +393,7 @@ public void testPartialResults() throws Exception {
         // sequence query with missing event on unavailable shard. THIS IS A FALSE POSITIVE
         request = new EqlSearchRequest().indices("test-*")
             .query("sequence with maxspan=10s  [process where value == 1] ![process where value == 2] [process where value == 3]")
-            .allowPartialSearchResults(true)
-            .allowPartialSequenceResults(false);
+            .allowPartialSearchResults(true);
         response = client().execute(EqlSearchAction.INSTANCE, request).get();
         assertThat(response.hits().sequences().size(), equalTo(0));
         assertThat(response.shardFailures().length, is(1));
@@ -401,8 +403,7 @@ public void testPartialResults() throws Exception {
         // sample query on both shards
         request = new EqlSearchRequest().indices("test-*")
             .query("sample by key [process where value == 2] [process where value == 1]")
-            .allowPartialSearchResults(true)
-            .allowPartialSequenceResults(false);
+            .allowPartialSearchResults(true);
         response = client().execute(EqlSearchAction.INSTANCE, request).get();
         assertThat(response.hits().sequences().size(), equalTo(0));
         assertThat(response.shardFailures().length, is(1));
@@ -412,8 +413,7 @@ public void testPartialResults() throws Exception {
         // sample query on the available shard only
         request = new EqlSearchRequest().indices("test-*")
             .query("sample by key [process where value == 3] [process where value == 1]")
-            .allowPartialSearchResults(true)
-            .allowPartialSequenceResults(false);
+            .allowPartialSearchResults(true);
         response = client().execute(EqlSearchAction.INSTANCE, request).get();
         assertThat(response.hits().sequences().size(), equalTo(1));
         sample = response.hits().sequences().get(0);
@@ -426,8 +426,7 @@ public void testPartialResults() throws Exception {
         // sample query on the unavailable shard only
         request = new EqlSearchRequest().indices("test-*")
             .query("sample by key [process where value == 2] [process where value == 0]")
-            .allowPartialSearchResults(true)
-            .allowPartialSequenceResults(false);
+            .allowPartialSearchResults(true);
         response = client().execute(EqlSearchAction.INSTANCE, request).get();
         assertThat(response.hits().sequences().size(), equalTo(0));
         assertThat(response.shardFailures().length, is(1));
@@ -465,10 +464,7 @@ public void testPartialResults() throws Exception {
         // sequence query on the available shard only
         request = new EqlSearchRequest().indices("test-*").query("sequence [process where value == 1] [process where value == 3]");
         response = client().execute(EqlSearchAction.INSTANCE, request).get();
-        assertThat(response.hits().sequences().size(), equalTo(1));
-        sequence = response.hits().sequences().get(0);
-        assertThat(sequence.events().get(0).toString(), containsString("\"value\" : 1"));
-        assertThat(sequence.events().get(1).toString(), containsString("\"value\" : 3"));
+        assertThat(response.hits().sequences().size(), equalTo(0));
         assertThat(response.shardFailures().length, is(1));
         assertThat(response.shardFailures()[0].index(), is("test-1"));
         assertThat(response.shardFailures()[0].reason(), containsString("NoShardAvailableActionException"));
@@ -485,10 +481,7 @@ public void testPartialResults() throws Exception {
         request = new EqlSearchRequest().indices("test-*")
             .query("sequence with maxspan=10s  [process where value == 1] ![process where value == 2] [process where value == 3]");
         response = client().execute(EqlSearchAction.INSTANCE, request).get();
-        assertThat(response.hits().sequences().size(), equalTo(1));
-        sequence = response.hits().sequences().get(0);
-        assertThat(sequence.events().get(0).toString(), containsString("\"value\" : 1"));
-        assertThat(sequence.events().get(2).toString(), containsString("\"value\" : 3"));
+        assertThat(response.hits().sequences().size(), equalTo(0));
         assertThat(response.shardFailures().length, is(1));
         assertThat(response.shardFailures()[0].index(), is("test-1"));
         assertThat(response.shardFailures()[0].reason(), containsString("NoShardAvailableActionException"));

diff --git a/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/action/EqlSearchRequest.java b/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/action/EqlSearchRequest.java
@@ -146,7 +146,7 @@ public EqlSearchRequest(StreamInput in) throws IOException {
             allowPartialSequenceResults = in.readOptionalBoolean();
         } else {
             allowPartialSearchResults = false;
-            allowPartialSequenceResults = true;
+            allowPartialSequenceResults = false;
         }
     }
 

diff --git a/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/plugin/EqlPlugin.java b/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/plugin/EqlPlugin.java
@@ -69,7 +69,7 @@ public class EqlPlugin extends Plugin implements ActionPlugin, CircuitBreakerPlu
 
     public static final Setting<Boolean> DEFAULT_ALLOW_PARTIAL_SEQUENCE_RESULTS = Setting.boolSetting(
         "xpack.eql.default_allow_partial_sequence_results",
-        true,
+        false,
         Setting.Property.NodeScope,
         Setting.Property.Dynamic
     );

diff --git a/...ck/plugin/eql/src/test/java/org/elasticsearch/xpack/eql/action/EqlSearchRequestTests.java b/...ck/plugin/eql/src/test/java/org/elasticsearch/xpack/eql/action/EqlSearchRequestTests.java
@@ -142,7 +142,7 @@ protected EqlSearchRequest mutateInstanceForVersion(EqlSearchRequest instance, T
             version.onOrAfter(TransportVersions.EQL_ALLOW_PARTIAL_SEARCH_RESULTS) ? instance.allowPartialSearchResults() : false
         );
         mutatedInstance.allowPartialSequenceResults(
-            version.onOrAfter(TransportVersions.EQL_ALLOW_PARTIAL_SEARCH_RESULTS) ? instance.allowPartialSequenceResults() : true
+            version.onOrAfter(TransportVersions.EQL_ALLOW_PARTIAL_SEARCH_RESULTS) ? instance.allowPartialSequenceResults() : false
         );
 
         return mutatedInstance;