Skip to content

Commit 0f74838

Browse files
authored
ESQL: Fix EvalBenchmark (elastic#124736) (elastic#124922)
Fix the benchmark for `EVAL` which was failing because of a strange logging error. The benchmarks really didn't want to run when we use commons-logging. That's fine - we can use the ES logging facade thing. I also added a test to the benchmarks which should run the self-tests for `EVAL` on `gradle check`.
1 parent 12bcc4f commit 0f74838

File tree

4 files changed

+34
-5
lines changed

4 files changed

+34
-5
lines changed

benchmarks/build.gradle

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@ base {
2525
archivesName = 'elasticsearch-benchmarks'
2626
}
2727

28-
tasks.named("test").configure { enabled = false }
2928
tasks.named("javadoc").configure { enabled = false }
3029

3130
configurations {
@@ -52,8 +51,10 @@ dependencies {
5251
api "org.openjdk.jmh:jmh-core:$versions.jmh"
5352
annotationProcessor "org.openjdk.jmh:jmh-generator-annprocess:$versions.jmh"
5453
// Dependencies of JMH
55-
runtimeOnly 'net.sf.jopt-simple:jopt-simple:5.0.4'
54+
runtimeOnly 'net.sf.jopt-simple:jopt-simple:5.0.2'
5655
runtimeOnly 'org.apache.commons:commons-math3:3.6.1'
56+
57+
testImplementation(project(':test:framework'))
5758
}
5859

5960
// enable the JMH's BenchmarkProcessor to generate the final benchmark classes

benchmarks/src/main/java/org/elasticsearch/benchmark/compute/operator/EvalBenchmark.java

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111

1212
import org.apache.lucene.util.BytesRef;
1313
import org.elasticsearch.common.breaker.NoopCircuitBreaker;
14+
import org.elasticsearch.common.logging.LogConfigurator;
1415
import org.elasticsearch.common.settings.Settings;
1516
import org.elasticsearch.common.util.BigArrays;
1617
import org.elasticsearch.compute.data.Block;
@@ -28,6 +29,8 @@
2829
import org.elasticsearch.compute.operator.EvalOperator;
2930
import org.elasticsearch.compute.operator.Operator;
3031
import org.elasticsearch.core.TimeValue;
32+
import org.elasticsearch.logging.LogManager;
33+
import org.elasticsearch.logging.Logger;
3134
import org.elasticsearch.xpack.esql.core.expression.Expression;
3235
import org.elasticsearch.xpack.esql.core.expression.FieldAttribute;
3336
import org.elasticsearch.xpack.esql.core.expression.FoldContext;
@@ -89,9 +92,16 @@ public class EvalBenchmark {
8992
static final DriverContext driverContext = new DriverContext(BigArrays.NON_RECYCLING_INSTANCE, blockFactory);
9093

9194
static {
95+
LogConfigurator.configureESLogging();
9296
// Smoke test all the expected values and force loading subclasses more like prod
97+
selfTest();
98+
}
99+
100+
static void selfTest() {
101+
Logger log = LogManager.getLogger(EvalBenchmark.class);
93102
try {
94103
for (String operation : EvalBenchmark.class.getField("operation").getAnnotationsByType(Param.class)[0].value()) {
104+
log.info("self testing {}", operation);
95105
run(operation);
96106
}
97107
} catch (NoSuchFieldException e) {
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3+
* or more contributor license agreements. Licensed under the "Elastic License
4+
* 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side
5+
* Public License v 1"; you may not use this file except in compliance with, at
6+
* your election, the "Elastic License 2.0", the "GNU Affero General Public
7+
* License v3.0 only", or the "Server Side Public License, v 1".
8+
*/
9+
10+
package org.elasticsearch.benchmark.compute.operator;
11+
12+
import org.elasticsearch.test.ESTestCase;
13+
14+
public class EvalBenchmarkTests extends ESTestCase {
15+
public void testSelfTest() {
16+
EvalBenchmark.selfTest();
17+
}
18+
}

x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/convert/AbstractConvertFunction.java

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

1010
import joptsimple.internal.Strings;
1111

12-
import org.apache.commons.logging.Log;
13-
import org.apache.commons.logging.LogFactory;
1412
import org.elasticsearch.common.io.stream.StreamInput;
1513
import org.elasticsearch.compute.data.Block;
1614
import org.elasticsearch.compute.data.Page;
@@ -20,6 +18,8 @@
2018
import org.elasticsearch.compute.operator.EvalOperator.ExpressionEvaluator;
2119
import org.elasticsearch.compute.operator.Warnings;
2220
import org.elasticsearch.core.Releasables;
21+
import org.elasticsearch.logging.LogManager;
22+
import org.elasticsearch.logging.Logger;
2323
import org.elasticsearch.xpack.esql.EsqlIllegalArgumentException;
2424
import org.elasticsearch.xpack.esql.core.expression.Expression;
2525
import org.elasticsearch.xpack.esql.core.tree.Source;
@@ -128,7 +128,7 @@ public ExpressionEvaluator.Factory toEvaluator(ToEvaluator toEvaluator) {
128128

129129
public abstract static class AbstractEvaluator implements EvalOperator.ExpressionEvaluator {
130130

131-
private static final Log logger = LogFactory.getLog(AbstractEvaluator.class);
131+
private static final Logger logger = LogManager.getLogger(AbstractEvaluator.class);
132132

133133
protected final DriverContext driverContext;
134134
private final EvalOperator.ExpressionEvaluator fieldEvaluator;

0 commit comments

Comments
 (0)