Skip to content

Commit b58343d

Browse files
craigtavernercraigtaverner
authored
Re-enable LOOKUP JOIN tests in 8.18 (#118280)
When we back-ported the LOOKUP JOIN PRs to 8.x (see #117967), we found it necessary to disable all csv-spec tests since they create indices with mode:lookup, which is illegal in the cluster state of mixed clusters where other nodes do not understand the new index mode. We need to re-enable the tests, and make sure the tests are only disabled in mixed clusters with node versions too old to handle the new mode.
1 parent d21c32f commit b58343d

File tree

7 files changed

+65
-33
lines changed

7 files changed

+65
-33
lines changed

‎x-pack/plugin/esql/qa/server/mixed-cluster/build.gradle‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ restResources {
2222
dependencies {
2323
javaRestTestImplementation project(xpackModule('esql:qa:testFixtures'))
2424
javaRestTestImplementation project(xpackModule('esql:qa:server'))
25+
javaRestTestImplementation project(xpackModule('esql'))
2526
}
2627

2728
GradleUtils.extendSourceSet(project, "javaRestTest", "yamlRestTest")

‎x-pack/plugin/esql/qa/server/mixed-cluster/src/javaRestTest/java/org/elasticsearch/xpack/esql/qa/mixed/MixedClusterEsqlSpecIT.java‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,10 @@
1818
import org.junit.ClassRule;
1919

2020
import java.io.IOException;
21+
import java.util.List;
2122

2223
import static org.elasticsearch.xpack.esql.CsvTestUtils.isEnabled;
24+
import static org.elasticsearch.xpack.esql.action.EsqlCapabilities.Cap.JOIN_LOOKUP_V4;
2325
import static org.elasticsearch.xpack.esql.qa.rest.EsqlSpecTestCase.Mode.ASYNC;
2426

2527
public class MixedClusterEsqlSpecIT extends EsqlSpecTestCase {
@@ -92,6 +94,11 @@ protected boolean supportsInferenceTestService() {
9294
return false;
9395
}
9496

97+
@Override
98+
protected boolean supportsIndexModeLookup() throws IOException {
99+
return hasCapabilities(List.of(JOIN_LOOKUP_V4.capabilityName()));
100+
}
101+
95102
@Override
96103
protected boolean deduplicateExactWarnings() {
97104
/*

‎x-pack/plugin/esql/qa/server/multi-clusters/src/javaRestTest/java/org/elasticsearch/xpack/esql/ccq/MultiClusterSpecIT.java‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -280,4 +280,11 @@ protected boolean enableRoundingDoubleValuesOnAsserting() {
280280
protected boolean supportsInferenceTestService() {
281281
return false;
282282
}
283+
284+
@Override
285+
protected boolean supportsIndexModeLookup() throws IOException {
286+
// CCS does not yet support JOIN_LOOKUP_V4 and clusters falsely report they have this capability
287+
// return hasCapabilities(List.of(JOIN_LOOKUP_V4.capabilityName()));
288+
return false;
289+
}
283290
}

‎x-pack/plugin/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/EsqlSpecTestCase.java‎

Lines changed: 31 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -136,8 +136,8 @@ public void setup() throws IOException {
136136
createInferenceEndpoint(client());
137137
}
138138

139-
if (indexExists(availableDatasetsForEs(client()).iterator().next().indexName()) == false) {
140-
loadDataSetIntoEs(client());
139+
if (indexExists(availableDatasetsForEs(client(), supportsIndexModeLookup()).iterator().next().indexName()) == false) {
140+
loadDataSetIntoEs(client(), supportsIndexModeLookup());
141141
}
142142
}
143143

@@ -182,12 +182,33 @@ protected void shouldSkipTest(String testName) throws IOException {
182182

183183
protected static void checkCapabilities(RestClient client, TestFeatureService testFeatureService, String testName, CsvTestCase testCase)
184184
throws IOException {
185-
if (testCase.requiredCapabilities.isEmpty()) {
185+
if (hasCapabilities(client, testCase.requiredCapabilities)) {
186186
return;
187187
}
188+
189+
var features = Stream.concat(
190+
new EsqlFeatures().getFeatures().stream(),
191+
new EsqlFeatures().getHistoricalFeatures().keySet().stream()
192+
).map(NodeFeature::id).collect(Collectors.toSet());
193+
194+
for (String feature : testCase.requiredCapabilities) {
195+
var esqlFeature = "esql." + feature;
196+
assumeTrue("Requested capability " + feature + " is an ESQL cluster feature", features.contains(esqlFeature));
197+
assumeTrue("Test " + testName + " requires " + feature, testFeatureService.clusterHasFeature(esqlFeature));
198+
}
199+
}
200+
201+
protected static boolean hasCapabilities(List<String> requiredCapabilities) throws IOException {
202+
return hasCapabilities(adminClient(), requiredCapabilities);
203+
}
204+
205+
protected static boolean hasCapabilities(RestClient client, List<String> requiredCapabilities) throws IOException {
206+
if (requiredCapabilities.isEmpty()) {
207+
return true;
208+
}
188209
try {
189-
if (clusterHasCapability(client, "POST", "/_query", List.of(), testCase.requiredCapabilities).orElse(false)) {
190-
return;
210+
if (clusterHasCapability(client, "POST", "/_query", List.of(), requiredCapabilities).orElse(false)) {
211+
return true;
191212
}
192213
LOGGER.info("capabilities API returned false, we might be in a mixed version cluster so falling back to cluster features");
193214
} catch (ResponseException e) {
@@ -206,23 +227,17 @@ protected static void checkCapabilities(RestClient client, TestFeatureService te
206227
throw e;
207228
}
208229
}
209-
210-
var features = Stream.concat(
211-
new EsqlFeatures().getFeatures().stream(),
212-
new EsqlFeatures().getHistoricalFeatures().keySet().stream()
213-
).map(NodeFeature::id).collect(Collectors.toSet());
214-
215-
for (String feature : testCase.requiredCapabilities) {
216-
var esqlFeature = "esql." + feature;
217-
assumeTrue("Requested capability " + feature + " is an ESQL cluster feature", features.contains(esqlFeature));
218-
assumeTrue("Test " + testName + " requires " + feature, testFeatureService.clusterHasFeature(esqlFeature));
219-
}
230+
return false;
220231
}
221232

222233
protected boolean supportsInferenceTestService() {
223234
return true;
224235
}
225236

237+
protected boolean supportsIndexModeLookup() throws IOException {
238+
return true;
239+
}
240+
226241
protected final void doTest() throws Throwable {
227242
RequestObjectBuilder builder = new RequestObjectBuilder(randomFrom(XContentType.values()));
228243

‎x-pack/plugin/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/generative/GenerativeRestTest.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ public abstract class GenerativeRestTest extends ESRestTestCase {
4646
@Before
4747
public void setup() throws IOException {
4848
if (indexExists(CSV_DATASET_MAP.keySet().iterator().next()) == false) {
49-
loadDataSetIntoEs(client());
49+
loadDataSetIntoEs(client(), true);
5050
}
5151
}
5252

‎x-pack/plugin/esql/qa/testFixtures/src/main/java/org/elasticsearch/xpack/esql/CsvTestsDataLoader.java‎

Lines changed: 17 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -61,8 +61,8 @@ public class CsvTestsDataLoader {
6161
private static final TestsDataset APPS = new TestsDataset("apps");
6262
private static final TestsDataset APPS_SHORT = APPS.withIndex("apps_short").withTypeMapping(Map.of("id", "short"));
6363
private static final TestsDataset LANGUAGES = new TestsDataset("languages");
64-
// private static final TestsDataset LANGUAGES_LOOKUP = LANGUAGES.withIndex("languages_lookup")
65-
// .withSetting("languages_lookup-settings.json");
64+
private static final TestsDataset LANGUAGES_LOOKUP = LANGUAGES.withIndex("languages_lookup")
65+
.withSetting("languages_lookup-settings.json");
6666
private static final TestsDataset ALERTS = new TestsDataset("alerts");
6767
private static final TestsDataset UL_LOGS = new TestsDataset("ul_logs");
6868
private static final TestsDataset SAMPLE_DATA = new TestsDataset("sample_data");
@@ -77,11 +77,11 @@ public class CsvTestsDataLoader {
7777
.withTypeMapping(Map.of("@timestamp", "date_nanos"));
7878
private static final TestsDataset MISSING_IP_SAMPLE_DATA = new TestsDataset("missing_ip_sample_data");
7979
private static final TestsDataset CLIENT_IPS = new TestsDataset("clientips");
80-
// private static final TestsDataset CLIENT_IPS_LOOKUP = CLIENT_IPS.withIndex("clientips_lookup")
81-
// .withSetting("clientips_lookup-settings.json");
80+
private static final TestsDataset CLIENT_IPS_LOOKUP = CLIENT_IPS.withIndex("clientips_lookup")
81+
.withSetting("clientips_lookup-settings.json");
8282
private static final TestsDataset MESSAGE_TYPES = new TestsDataset("message_types");
83-
// private static final TestsDataset MESSAGE_TYPES_LOOKUP = MESSAGE_TYPES.withIndex("message_types_lookup")
84-
// .withSetting("message_types_lookup-settings.json");
83+
private static final TestsDataset MESSAGE_TYPES_LOOKUP = MESSAGE_TYPES.withIndex("message_types_lookup")
84+
.withSetting("message_types_lookup-settings.json");
8585
private static final TestsDataset CLIENT_CIDR = new TestsDataset("client_cidr");
8686
private static final TestsDataset AGES = new TestsDataset("ages");
8787
private static final TestsDataset HEIGHTS = new TestsDataset("heights");
@@ -113,7 +113,7 @@ public class CsvTestsDataLoader {
113113
Map.entry(APPS.indexName, APPS),
114114
Map.entry(APPS_SHORT.indexName, APPS_SHORT),
115115
Map.entry(LANGUAGES.indexName, LANGUAGES),
116-
// Map.entry(LANGUAGES_LOOKUP.indexName, LANGUAGES_LOOKUP),
116+
Map.entry(LANGUAGES_LOOKUP.indexName, LANGUAGES_LOOKUP),
117117
Map.entry(UL_LOGS.indexName, UL_LOGS),
118118
Map.entry(SAMPLE_DATA.indexName, SAMPLE_DATA),
119119
Map.entry(MV_SAMPLE_DATA.indexName, MV_SAMPLE_DATA),
@@ -123,9 +123,9 @@ public class CsvTestsDataLoader {
123123
Map.entry(SAMPLE_DATA_TS_NANOS.indexName, SAMPLE_DATA_TS_NANOS),
124124
Map.entry(MISSING_IP_SAMPLE_DATA.indexName, MISSING_IP_SAMPLE_DATA),
125125
Map.entry(CLIENT_IPS.indexName, CLIENT_IPS),
126-
// Map.entry(CLIENT_IPS_LOOKUP.indexName, CLIENT_IPS_LOOKUP),
126+
Map.entry(CLIENT_IPS_LOOKUP.indexName, CLIENT_IPS_LOOKUP),
127127
Map.entry(MESSAGE_TYPES.indexName, MESSAGE_TYPES),
128-
// Map.entry(MESSAGE_TYPES_LOOKUP.indexName, MESSAGE_TYPES_LOOKUP),
128+
Map.entry(MESSAGE_TYPES_LOOKUP.indexName, MESSAGE_TYPES_LOOKUP),
129129
Map.entry(CLIENT_CIDR.indexName, CLIENT_CIDR),
130130
Map.entry(AGES.indexName, AGES),
131131
Map.entry(HEIGHTS.indexName, HEIGHTS),
@@ -236,7 +236,7 @@ public static void main(String[] args) throws IOException {
236236
}
237237

238238
try (RestClient client = builder.build()) {
239-
loadDataSetIntoEs(client, (restClient, indexName, indexMapping, indexSettings) -> {
239+
loadDataSetIntoEs(client, true, (restClient, indexName, indexMapping, indexSettings) -> {
240240
// don't use ESRestTestCase methods here or, if you do, test running the main method before making the change
241241
StringBuilder jsonBody = new StringBuilder("{");
242242
if (indexSettings != null && indexSettings.isEmpty() == false) {
@@ -255,26 +255,28 @@ public static void main(String[] args) throws IOException {
255255
}
256256
}
257257

258-
public static Set<TestsDataset> availableDatasetsForEs(RestClient client) throws IOException {
258+
public static Set<TestsDataset> availableDatasetsForEs(RestClient client, boolean supportsIndexModeLookup) throws IOException {
259259
boolean inferenceEnabled = clusterHasInferenceEndpoint(client);
260260

261261
return CSV_DATASET_MAP.values()
262262
.stream()
263263
.filter(d -> d.requiresInferenceEndpoint == false || inferenceEnabled)
264+
.filter(d -> supportsIndexModeLookup || d.indexName.endsWith("_lookup") == false) // TODO: use actual index settings
264265
.collect(Collectors.toCollection(HashSet::new));
265266
}
266267

267-
public static void loadDataSetIntoEs(RestClient client) throws IOException {
268-
loadDataSetIntoEs(client, (restClient, indexName, indexMapping, indexSettings) -> {
268+
public static void loadDataSetIntoEs(RestClient client, boolean supportsIndexModeLookup) throws IOException {
269+
loadDataSetIntoEs(client, supportsIndexModeLookup, (restClient, indexName, indexMapping, indexSettings) -> {
269270
ESRestTestCase.createIndex(restClient, indexName, indexSettings, indexMapping, null);
270271
});
271272
}
272273

273-
private static void loadDataSetIntoEs(RestClient client, IndexCreator indexCreator) throws IOException {
274+
private static void loadDataSetIntoEs(RestClient client, boolean supportsIndexModeLookup, IndexCreator indexCreator)
275+
throws IOException {
274276
Logger logger = LogManager.getLogger(CsvTestsDataLoader.class);
275277

276278
Set<String> loadedDatasets = new HashSet<>();
277-
for (var dataset : availableDatasetsForEs(client)) {
279+
for (var dataset : availableDatasetsForEs(client, supportsIndexModeLookup)) {
278280
load(client, dataset, logger, indexCreator);
279281
loadedDatasets.add(dataset.indexName);
280282
}

‎x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlCapabilities.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -523,7 +523,7 @@ public enum Cap {
523523
/**
524524
* LOOKUP JOIN
525525
*/
526-
JOIN_LOOKUP_V4(false && Build.current().isSnapshot()),
526+
JOIN_LOOKUP_V4(Build.current().isSnapshot()),
527527

528528
/**
529529
* Fix for https://github.com/elastic/elasticsearch/issues/117054

0 commit comments

Comments
 (0)