Skip to content

Commit 94c60c4

Browse files
committed
Tests: Add logging to FieldSortIT (elastic#131558)
In order to better understand the infrequent failures in elastic#129445 and in the hope to reproduce the issue better, this PR adds logging around which shards and nodes documents end up in FieldSortIT#testSortMixedFieldTypes and increases the log level for this test suite in the o.e.search packages to TRACE and in some action.search packages to DEBUG to better understand where exceptions are thrown and to better trace how resources are released after that. Relates to elastic#129445
1 parent 4b47fd8 commit 94c60c4

File tree

3 files changed

+48
-12
lines changed

3 files changed

+48
-12
lines changed

server/src/internalClusterTest/java/org/elasticsearch/search/sort/FieldSortIT.java

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99

1010
package org.elasticsearch.search.sort;
1111

12+
import org.apache.http.util.EntityUtils;
1213
import org.apache.lucene.tests.util.TestUtil;
1314
import org.apache.lucene.util.BytesRef;
1415
import org.apache.lucene.util.UnicodeUtil;
@@ -20,6 +21,9 @@
2021
import org.elasticsearch.action.search.SearchRequestBuilder;
2122
import org.elasticsearch.action.search.SearchResponse;
2223
import org.elasticsearch.action.search.ShardSearchFailure;
24+
import org.elasticsearch.client.Request;
25+
import org.elasticsearch.client.Response;
26+
import org.elasticsearch.client.RestClient;
2327
import org.elasticsearch.cluster.metadata.IndexMetadata;
2428
import org.elasticsearch.common.settings.Settings;
2529
import org.elasticsearch.core.Strings;
@@ -36,6 +40,7 @@
3640
import org.elasticsearch.search.SearchHits;
3741
import org.elasticsearch.test.ESIntegTestCase;
3842
import org.elasticsearch.test.InternalSettingsPlugin;
43+
import org.elasticsearch.test.junit.annotations.TestIssueLogging;
3944
import org.elasticsearch.xcontent.XContentBuilder;
4045
import org.elasticsearch.xcontent.XContentFactory;
4146
import org.elasticsearch.xcontent.XContentType;
@@ -82,6 +87,12 @@
8287
import static org.hamcrest.Matchers.not;
8388
import static org.hamcrest.Matchers.nullValue;
8489

90+
@TestIssueLogging(
91+
issueUrl = "https://github.com/elastic/elasticsearch/issues/129445",
92+
value = "org.elasticsearch.action.search.SearchQueryThenFetchAsyncAction:DEBUG,"
93+
+ "org.elasticsearch.action.search.SearchPhaseController:DEBUG,"
94+
+ "org.elasticsearch.search:TRACE"
95+
)
8596
public class FieldSortIT extends ESIntegTestCase {
8697
public static class CustomScriptPlugin extends MockScriptPlugin {
8798
@Override
@@ -110,6 +121,10 @@ protected Collection<Class<? extends Plugin>> nodePlugins() {
110121
return Arrays.asList(InternalSettingsPlugin.class, CustomScriptPlugin.class);
111122
}
112123

124+
protected boolean addMockHttpTransport() {
125+
return false;
126+
}
127+
113128
public void testIssue8226() {
114129
int numIndices = between(5, 10);
115130
final boolean useMapping = randomBoolean();
@@ -2242,7 +2257,7 @@ public void testLongSortOptimizationCorrectResults() {
22422257
);
22432258
}
22442259

2245-
public void testSortMixedFieldTypes() {
2260+
public void testSortMixedFieldTypes() throws IOException {
22462261
assertAcked(
22472262
prepareCreate("index_long").setMapping("foo", "type=long"),
22482263
prepareCreate("index_integer").setMapping("foo", "type=integer"),
@@ -2256,6 +2271,16 @@ public void testSortMixedFieldTypes() {
22562271
prepareIndex("index_keyword").setId("1").setSource("foo", "123").get();
22572272
refresh();
22582273

2274+
// for debugging, we try to see where the documents are located
2275+
try (RestClient restClient = createRestClient()) {
2276+
Request checkShardsRequest = new Request(
2277+
"GET",
2278+
"/_cat/shards/index_long,index_double,index_keyword?format=json&h=index,node,shard,prirep,state,docs,index"
2279+
);
2280+
Response response = restClient.performRequest(checkShardsRequest);
2281+
logger.info("FieldSortIT#testSortMixedFieldTypes document distribution: " + EntityUtils.toString(response.getEntity()));
2282+
}
2283+
22592284
{ // mixing long and integer types is ok, as we convert integer sort to long sort
22602285
assertNoFailures(prepareSearch("index_long", "index_integer").addSort(new FieldSortBuilder("foo")).setSize(10));
22612286
}

server/src/main/java/org/elasticsearch/action/search/SearchPhaseController.java

Lines changed: 20 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@
99

1010
package org.elasticsearch.action.search;
1111

12+
import org.apache.logging.log4j.LogManager;
13+
import org.apache.logging.log4j.Logger;
1214
import org.apache.lucene.search.FieldDoc;
1315
import org.apache.lucene.search.ScoreDoc;
1416
import org.apache.lucene.search.Sort;
@@ -72,6 +74,8 @@
7274

7375
public final class SearchPhaseController {
7476

77+
private static final Logger logger = LogManager.getLogger(SearchPhaseController.class);
78+
7579
private final BiFunction<
7680
Supplier<Boolean>,
7781
AggregatorFactories.Builder,
@@ -153,17 +157,22 @@ static TopDocs mergeTopDocs(Collection<TopDocs> results, int topN, int from) {
153157
return topDocs;
154158
}
155159
final TopDocs mergedTopDocs;
156-
if (topDocs instanceof TopFieldGroups firstTopDocs) {
157-
final Sort sort = validateSameSortTypesAndMaybeRewrite(results, firstTopDocs.fields);
158-
TopFieldGroups[] shardTopDocs = topDocsList.toArray(new TopFieldGroups[0]);
159-
mergedTopDocs = TopFieldGroups.merge(sort, from, topN, shardTopDocs, false);
160-
} else if (topDocs instanceof TopFieldDocs firstTopDocs) {
161-
TopFieldDocs[] shardTopDocs = topDocsList.toArray(new TopFieldDocs[0]);
162-
final Sort sort = validateSameSortTypesAndMaybeRewrite(results, firstTopDocs.fields);
163-
mergedTopDocs = TopDocs.merge(sort, from, topN, shardTopDocs);
164-
} else {
165-
final TopDocs[] shardTopDocs = topDocsList.toArray(new TopDocs[0]);
166-
mergedTopDocs = TopDocs.merge(from, topN, shardTopDocs);
160+
try {
161+
if (topDocs instanceof TopFieldGroups firstTopDocs) {
162+
final Sort sort = validateSameSortTypesAndMaybeRewrite(results, firstTopDocs.fields);
163+
TopFieldGroups[] shardTopDocs = topDocsList.toArray(new TopFieldGroups[0]);
164+
mergedTopDocs = TopFieldGroups.merge(sort, from, topN, shardTopDocs, false);
165+
} else if (topDocs instanceof TopFieldDocs firstTopDocs) {
166+
TopFieldDocs[] shardTopDocs = topDocsList.toArray(new TopFieldDocs[0]);
167+
final Sort sort = validateSameSortTypesAndMaybeRewrite(results, firstTopDocs.fields);
168+
mergedTopDocs = TopDocs.merge(sort, from, topN, shardTopDocs);
169+
} else {
170+
final TopDocs[] shardTopDocs = topDocsList.toArray(new TopDocs[0]);
171+
mergedTopDocs = TopDocs.merge(from, topN, shardTopDocs);
172+
}
173+
} catch (IllegalArgumentException e) {
174+
logger.debug("Failed to merge top docs: ", e);
175+
throw e;
167176
}
168177
return mergedTopDocs;
169178
}

server/src/main/java/org/elasticsearch/action/search/SearchQueryThenFetchAsyncAction.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -454,6 +454,7 @@ protected void doRun(Map<SearchShardIterator, Integer> shardIndexMap) {
454454
executeAsSingleRequest(routing, request.shards.get(0));
455455
return;
456456
}
457+
String nodeId = routing.nodeId();
457458
final Transport.Connection connection;
458459
try {
459460
connection = getConnection(routing.clusterAlias(), routing.nodeId());
@@ -503,6 +504,7 @@ public void handleResponse(NodeQueryResponse response) {
503504
@Override
504505
public void handleException(TransportException e) {
505506
Exception cause = (Exception) ExceptionsHelper.unwrapCause(e);
507+
logger.debug("handling node search exception coming from [" + nodeId + "]", cause);
506508
if (e instanceof SendRequestTransportException || cause instanceof TaskCancelledException) {
507509
// two possible special cases here where we do not want to fail the phase:
508510
// failure to send out the request -> handle things the same way a shard would fail with unbatched execution

0 commit comments

Comments
 (0)