From 19c85812bf59581a5c5be4a2956dfd4b1db41b9e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iv=C3=A1n=20Cea=20Fontenla?= Date: Thu, 20 Nov 2025 16:08:48 +0100 Subject: [PATCH 1/3] Aggs: Unmute and fix CCSDuelIT flaky tests --- muted-tests.yml | 6 -- .../org/elasticsearch/search/CCSDuelIT.java | 62 ++++++++++++++++--- .../elasticsearch/test/XContentTestUtils.java | 44 +++++++++++-- .../test/XContentTestUtilsTests.java | 54 ++++++++++++++++ 4 files changed, 144 insertions(+), 22 deletions(-) diff --git a/muted-tests.yml b/muted-tests.yml index 56ea32132bac7..54ea3de6e57e7 100644 --- a/muted-tests.yml +++ b/muted-tests.yml @@ -276,12 +276,6 @@ tests: - class: org.elasticsearch.xpack.eql.planner.QueryTranslatorTests method: testMatchOptimization issue: https://github.com/elastic/elasticsearch/issues/132894 -- class: org.elasticsearch.search.CCSDuelIT - method: testTermsAggs - issue: https://github.com/elastic/elasticsearch/issues/132879 -- class: org.elasticsearch.search.CCSDuelIT - method: testTermsAggsWithProfile - issue: https://github.com/elastic/elasticsearch/issues/132880 - class: org.elasticsearch.xpack.security.authc.AuthenticationServiceTests method: testInvalidToken issue: https://github.com/elastic/elasticsearch/issues/133328 diff --git a/qa/multi-cluster-search/src/test/java/org/elasticsearch/search/CCSDuelIT.java b/qa/multi-cluster-search/src/test/java/org/elasticsearch/search/CCSDuelIT.java index 5b4ff08d8dfc7..61fd64dca84b4 100644 --- a/qa/multi-cluster-search/src/test/java/org/elasticsearch/search/CCSDuelIT.java +++ b/qa/multi-cluster-search/src/test/java/org/elasticsearch/search/CCSDuelIT.java @@ -96,6 +96,7 @@ import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicReference; +import java.util.function.Predicate; import static org.hamcrest.CoreMatchers.anyOf; import static org.hamcrest.CoreMatchers.equalTo; @@ -875,7 +876,7 @@ public void testShardFailures() throws Exception { assertNull(response.evaluate("suggest")); assertThat(response.evaluateArraySize("hits.hits"), greaterThan(0)); assertThat(response.evaluate("_shards.failed"), greaterThanOrEqualTo(2)); - }, compareAsyncAndSyncResponses); + }, compareAsyncAndSyncResponses, path -> true); } public void testTermSuggester() throws Exception { @@ -984,18 +985,19 @@ private static SearchRequest initRemoteOnlySearchRequest() { } private void duelRequest(SearchRequest searchRequest, CheckedConsumer responseChecker) throws Exception { - duelRequest(searchRequest, responseChecker, true); + duelRequest(searchRequest, responseChecker, true, p -> true); } private void duelRequest( SearchRequest searchRequest, CheckedConsumer responseChecker, - boolean compareAsyncToSyncResponses + boolean compareAsyncToSyncResponses, + Predicate pathFilter ) throws Exception { - Map syncResponseMap = duelSearchSync(searchRequest, responseChecker); - Map asyncResponseMap = duelSearchAsync(searchRequest, responseChecker); + Map syncResponseMap = duelSearchSync(searchRequest, responseChecker, pathFilter); + Map asyncResponseMap = duelSearchAsync(searchRequest, responseChecker, pathFilter); if (compareAsyncToSyncResponses) { - compareResponseMaps(syncResponseMap, asyncResponseMap, "Comparing sync_search CCS vs. async_search CCS"); + compareResponseMaps(syncResponseMap, asyncResponseMap, "Comparing sync_search CCS vs. async_search CCS", pathFilter); } } @@ -1004,11 +1006,24 @@ private void duelRequest( */ private static Map duelSearchSync(SearchRequest searchRequest, CheckedConsumer responseChecker) throws Exception { + return duelSearchSync(searchRequest, responseChecker, p -> true); + } + + /** + * @return responseMap from one of the Synchronous Search Requests + */ + private static Map duelSearchSync( + SearchRequest searchRequest, + CheckedConsumer responseChecker, + Predicate pathFilter + ) throws Exception { CountDownLatch latch = new CountDownLatch(2); + AtomicReference exception1 = new AtomicReference<>(); AtomicReference minimizeRoundtripsResponse = new AtomicReference<>(); searchRequest.setCcsMinimizeRoundtrips(true); submitSyncSearch(searchRequest, minimizeRoundtripsResponse, exception1, latch); + AtomicReference exception2 = new AtomicReference<>(); AtomicReference fanOutResponse = new AtomicReference<>(); searchRequest.setCcsMinimizeRoundtrips(false); @@ -1075,7 +1090,13 @@ private static Map duelSearchSync(SearchRequest searchRequest, C Map minimizeRoundtripsResponseMap = responseToMap(minimizeRoundtripsSearchResponse); if (minimizeRoundtripsSearchResponse.evaluate("_clusters") != null && fanOutSearchResponse.evaluate("_clusters") != null) { Map fanOutResponseMap = responseToMap(fanOutSearchResponse); - compareResponseMaps(minimizeRoundtripsResponseMap, fanOutResponseMap, "Comparing sync_search minimizeRoundTrip vs. fanOut"); + // TODO: Don't compare doc_count_error_upper_bound, as it's an approximate + compareResponseMaps( + minimizeRoundtripsResponseMap, + fanOutResponseMap, + "Comparing sync_search minimizeRoundTrip vs. fanOut", + pathFilter + ); assertThat( minimizeRoundtripsSearchResponse.evaluate("_shards.skipped"), lessThanOrEqualTo((Integer) fanOutSearchResponse.evaluate("_shards.skipped")) @@ -1123,6 +1144,17 @@ public void onFailure(Exception exception) { private static Map duelSearchAsync( SearchRequest searchRequest, CheckedConsumer responseChecker + ) throws Exception { + return duelSearchAsync(searchRequest, responseChecker, p -> true); + } + + /** + * @return responseMap from one of the async searches + */ + private static Map duelSearchAsync( + SearchRequest searchRequest, + CheckedConsumer responseChecker, + Predicate pathFilter ) throws Exception { searchRequest.setCcsMinimizeRoundtrips(true); ObjectPath minimizeRoundtripsResponse = submitAsyncSearch(searchRequest, TimeValue.timeValueSeconds(1)); @@ -1183,7 +1215,12 @@ private static Map duelSearchAsync( Map minimizeRoundtripsResponseMap = responseToMap(minimizeRoundtripsResponse); if (minimizeRoundtripsResponse.evaluate("_clusters") != null && fanOutResponse.evaluate("_clusters") != null) { Map fanOutResponseMap = responseToMap(fanOutResponse); - compareResponseMaps(minimizeRoundtripsResponseMap, fanOutResponseMap, "Comparing async_search minimizeRoundTrip vs. fanOut"); + compareResponseMaps( + minimizeRoundtripsResponseMap, + fanOutResponseMap, + "Comparing async_search minimizeRoundTrip vs. fanOut", + pathFilter + ); assertThat( minimizeRoundtripsResponse.evaluate("_shards.skipped"), lessThanOrEqualTo((Integer) fanOutResponse.evaluate("_shards.skipped")) @@ -1192,8 +1229,13 @@ private static Map duelSearchAsync( return minimizeRoundtripsResponseMap; } - private static void compareResponseMaps(Map responseMap1, Map responseMap2, String info) { - String diff = XContentTestUtils.differenceBetweenMapsIgnoringArrayOrder(responseMap1, responseMap2); + private static void compareResponseMaps( + Map responseMap1, + Map responseMap2, + String info, + Predicate pathFilter + ) { + String diff = XContentTestUtils.differenceBetweenMapsIgnoringArrayOrder(responseMap1, responseMap2, pathFilter); if (diff != null) { NotEqualMessageBuilder builder = new NotEqualMessageBuilder(); builder.compareMaps(responseMap1, responseMap2); diff --git a/test/framework/src/main/java/org/elasticsearch/test/XContentTestUtils.java b/test/framework/src/main/java/org/elasticsearch/test/XContentTestUtils.java index 61001482753dd..a71ff734c1182 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/XContentTestUtils.java +++ b/test/framework/src/main/java/org/elasticsearch/test/XContentTestUtils.java @@ -82,16 +82,39 @@ public static BytesReference convertToXContent(Map map, XContentType * @return null if maps are equal or path to the element where the difference was found */ public static String differenceBetweenMapsIgnoringArrayOrder(Map first, Map second) { - return differenceBetweenMapsIgnoringArrayOrder("", first, second); + return differenceBetweenMapsIgnoringArrayOrder("", first, second, p -> true); } - private static String differenceBetweenMapsIgnoringArrayOrder(String path, Map first, Map second) { + /** + * Compares two maps generated from XContentObjects. The order of elements in arrays is ignored. + * + * @param pathFilter Predicate to filter a path and its children. True if the path should be checked, false to exclude it. + * @return null if maps are equal or path to the element where the difference was found + */ + public static String differenceBetweenMapsIgnoringArrayOrder( + Map first, + Map second, + Predicate pathFilter + ) { + return differenceBetweenMapsIgnoringArrayOrder("", first, second, pathFilter); + } + + private static String differenceBetweenMapsIgnoringArrayOrder( + String path, + Map first, + Map second, + Predicate pathFilter + ) { + if (pathFilter.test(path) == false) { + return null; + } + if (first.size() != second.size()) { return path + ": sizes of the maps don't match: " + first.size() + " != " + second.size(); } for (String key : first.keySet()) { - String reason = differenceBetweenObjectsIgnoringArrayOrder(path + "/" + key, first.get(key), second.get(key)); + String reason = differenceBetweenObjectsIgnoringArrayOrder(path + "/" + key, first.get(key), second.get(key), pathFilter); if (reason != null) { return reason; } @@ -100,7 +123,16 @@ private static String differenceBetweenMapsIgnoringArrayOrder(String path, Map pathFilter + ) { + if (pathFilter.test(path) == false) { + return null; + } + if (first == null) { if (second == null) { return null; @@ -116,7 +148,7 @@ private static String differenceBetweenObjectsIgnoringArrayOrder(String path, Ob for (Object firstObj : firstList) { boolean found = false; for (Object secondObj : secondList) { - reason = differenceBetweenObjectsIgnoringArrayOrder(path + "/*", firstObj, secondObj); + reason = differenceBetweenObjectsIgnoringArrayOrder(path + "/*", firstObj, secondObj, pathFilter); if (reason == null) { secondList.remove(secondObj); found = true; @@ -140,7 +172,7 @@ private static String differenceBetweenObjectsIgnoringArrayOrder(String path, Ob } } else if (first instanceof Map) { if (second instanceof Map) { - return differenceBetweenMapsIgnoringArrayOrder(path, (Map) first, (Map) second); + return differenceBetweenMapsIgnoringArrayOrder(path, (Map) first, (Map) second, pathFilter); } else { return path + ": the second element is not a map (got " + second + ")"; } diff --git a/test/framework/src/test/java/org/elasticsearch/test/XContentTestUtilsTests.java b/test/framework/src/test/java/org/elasticsearch/test/XContentTestUtilsTests.java index f46fe5ccd17b4..fc18e38856b02 100644 --- a/test/framework/src/test/java/org/elasticsearch/test/XContentTestUtilsTests.java +++ b/test/framework/src/test/java/org/elasticsearch/test/XContentTestUtilsTests.java @@ -27,10 +27,12 @@ import java.util.Map; import java.util.function.Predicate; +import static org.elasticsearch.test.XContentTestUtils.differenceBetweenMapsIgnoringArrayOrder; import static org.elasticsearch.test.XContentTestUtils.insertRandomFields; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.hasItem; import static org.hamcrest.Matchers.instanceOf; +import static org.hamcrest.Matchers.nullValue; public class XContentTestUtilsTests extends ESTestCase { @@ -222,4 +224,56 @@ public void testInsertRandomXContent() throws IOException { assertEquals(1, foo4List.size()); assertEquals(2, ((Map) foo4List.get(0)).keySet().size()); } + + public void testDifferenceBetweenMapsIgnoringArrayOrder() { + var map1 = Map.of("foo", List.of(1, 2, 3), "bar", Map.of("a", 2, "b", List.of(3, 2, 1)), "baz", List.of(3, 2, 1, "test")); + var map2 = Map.of("foo", List.of(3, 2, 1), "bar", Map.of("b", List.of(3, 1, 2), "a", 2), "baz", List.of(1, "test", 2, 3)); + + assertThat(differenceBetweenMapsIgnoringArrayOrder(map1, map2), nullValue()); + } + + public void testDifferenceBetweenMapsIgnoringArrayOrder_WithFilter_Object() { + var map1 = Map.of("foo", List.of(1, 2, 3), "bar", Map.of("a", 2, "b", List.of(3, 2, 1)), "different", Map.of("a", 1, "x", 8)); + var map2 = Map.of( + "foo", + List.of(3, 2, 1), + "bar", + Map.of("b", List.of(3, 1, 2), "a", 2), + "different", + Map.of("a", 1, "x", "different value") + ); + + assertThat( + differenceBetweenMapsIgnoringArrayOrder(map1, map2), + equalTo("/different/x: the elements don't match: [8] != [different value]") + ); + assertThat(differenceBetweenMapsIgnoringArrayOrder(map1, map2, path -> path.equals("/different/x") == false), nullValue()); + assertThat(differenceBetweenMapsIgnoringArrayOrder(map1, map2, path -> path.equals("/different") == false), nullValue()); + assertThat(differenceBetweenMapsIgnoringArrayOrder(map1, map2, path -> path.isEmpty() == false), nullValue()); + } + + public void testDifferenceBetweenMapsIgnoringArrayOrder_WithFilter_Array() { + var map1 = Map.of( + "foo", + List.of(1, 2, 3), + "bar", + Map.of("a", 2, "b", List.of(3, 2, 1)), + "different", + List.of(3, Map.of("x", 10), 1) + ); + var map2 = Map.of( + "foo", + List.of(3, 2, 1), + "bar", + Map.of("b", List.of(3, 1, 2), "a", 2), + "different", + List.of(3, Map.of("x", 5), 1) + ); + + assertThat(differenceBetweenMapsIgnoringArrayOrder(map1, map2), equalTo("/different/*: the second element is not a map (got 1)")); + assertThat(differenceBetweenMapsIgnoringArrayOrder(map1, map2, path -> path.equals("/different/*/x") == false), nullValue()); + assertThat(differenceBetweenMapsIgnoringArrayOrder(map1, map2, path -> path.equals("/different/*") == false), nullValue()); + assertThat(differenceBetweenMapsIgnoringArrayOrder(map1, map2, path -> path.equals("/different") == false), nullValue()); + assertThat(differenceBetweenMapsIgnoringArrayOrder(map1, map2, path -> path.isEmpty() == false), nullValue()); + } } From 527acc29c235f0cb7acf0f91a256bf84e8dfe66a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iv=C3=A1n=20Cea=20Fontenla?= Date: Thu, 20 Nov 2025 16:29:15 +0100 Subject: [PATCH 2/3] Ignore error upper bound in terms aggs response --- .../src/test/java/org/elasticsearch/search/CCSDuelIT.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/qa/multi-cluster-search/src/test/java/org/elasticsearch/search/CCSDuelIT.java b/qa/multi-cluster-search/src/test/java/org/elasticsearch/search/CCSDuelIT.java index 61fd64dca84b4..938640666dfb1 100644 --- a/qa/multi-cluster-search/src/test/java/org/elasticsearch/search/CCSDuelIT.java +++ b/qa/multi-cluster-search/src/test/java/org/elasticsearch/search/CCSDuelIT.java @@ -676,12 +676,12 @@ public void testTermsAggs() throws Exception { { SearchRequest searchRequest = initLocalAndRemoteSearchRequest(); searchRequest.source(buildTermsAggsSource()); - duelRequest(searchRequest, CCSDuelIT::assertAggs); + duelRequest(searchRequest, CCSDuelIT::assertAggs, true, path -> path.endsWith("/doc_count_error_upper_bound") == false); } { SearchRequest searchRequest = initRemoteOnlySearchRequest(); searchRequest.source(buildTermsAggsSource()); - duelRequest(searchRequest, CCSDuelIT::assertAggs); + duelRequest(searchRequest, CCSDuelIT::assertAggs, true, path -> path.endsWith("/doc_count_error_upper_bound") == false); } } @@ -690,12 +690,12 @@ public void testTermsAggsWithProfile() throws Exception { { SearchRequest searchRequest = initLocalAndRemoteSearchRequest(); searchRequest.source(buildTermsAggsSource().profile(true)); - duelRequest(searchRequest, CCSDuelIT::assertAggs); + duelRequest(searchRequest, CCSDuelIT::assertAggs, true, path -> path.endsWith("/doc_count_error_upper_bound") == false); } { SearchRequest searchRequest = initRemoteOnlySearchRequest(); searchRequest.source(buildTermsAggsSource().profile(true)); - duelRequest(searchRequest, CCSDuelIT::assertAggs); + duelRequest(searchRequest, CCSDuelIT::assertAggs, true, path -> path.endsWith("/doc_count_error_upper_bound") == false); } } From 9f4b091047d3f77c8d9f9326f2e28806f7b307b8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iv=C3=A1n=20Cea=20Fontenla?= Date: Fri, 21 Nov 2025 14:01:25 +0100 Subject: [PATCH 3/3] Remove TODO --- .../src/test/java/org/elasticsearch/search/CCSDuelIT.java | 1 - 1 file changed, 1 deletion(-) diff --git a/qa/multi-cluster-search/src/test/java/org/elasticsearch/search/CCSDuelIT.java b/qa/multi-cluster-search/src/test/java/org/elasticsearch/search/CCSDuelIT.java index 938640666dfb1..9ab932143ed86 100644 --- a/qa/multi-cluster-search/src/test/java/org/elasticsearch/search/CCSDuelIT.java +++ b/qa/multi-cluster-search/src/test/java/org/elasticsearch/search/CCSDuelIT.java @@ -1090,7 +1090,6 @@ private static Map duelSearchSync( Map minimizeRoundtripsResponseMap = responseToMap(minimizeRoundtripsSearchResponse); if (minimizeRoundtripsSearchResponse.evaluate("_clusters") != null && fanOutSearchResponse.evaluate("_clusters") != null) { Map fanOutResponseMap = responseToMap(fanOutSearchResponse); - // TODO: Don't compare doc_count_error_upper_bound, as it's an approximate compareResponseMaps( minimizeRoundtripsResponseMap, fanOutResponseMap,