Skip to content

Commit 443be2a

Browse files
authored
Merge branch 'main' into 2025/03/20/onResponseSent-redundant-response
2 parents 1341b38 + a16eaf1 commit 443be2a

File tree

7 files changed

+308
-16
lines changed

7 files changed

+308
-16
lines changed

docs/changelog/122250.yaml

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
pr: 122250
2+
summary: "ESQL: Align `RENAME` behavior with `EVAL` for sequential processing"
3+
area: ES|QL
4+
type: enhancement
5+
issues:
6+
- 121739

muted-tests.yml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -399,6 +399,9 @@ tests:
399399
- class: org.elasticsearch.reservedstate.service.ReservedClusterStateServiceTests
400400
method: testProcessMultipleChunks
401401
issue: https://github.com/elastic/elasticsearch/issues/125305
402+
- class: org.elasticsearch.index.mapper.NativeArrayIntegrationTestCase
403+
method: testSynthesizeArrayRandomIgnoresMalformed
404+
issue: https://github.com/elastic/elasticsearch/issues/125319
402405

403406
# Examples:
404407
#

x-pack/plugin/esql/qa/testFixtures/src/main/resources/rename.csv-spec

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -214,3 +214,85 @@ x:keyword
214214
Facello
215215
Simmel
216216
;
217+
218+
swappingNames
219+
required_capability: rename_sequential_processing
220+
FROM employees
221+
| SORT emp_no ASC
222+
| KEEP first_name, last_name
223+
| RENAME first_name AS last_name, last_name AS first_name, first_name as name
224+
| LIMIT 2
225+
;
226+
227+
name:keyword
228+
Georgi
229+
Bezalel
230+
;
231+
232+
complexSwappingNames
233+
required_capability: rename_sequential_processing
234+
FROM employees
235+
| SORT emp_no ASC
236+
| KEEP first_name, last_name, emp_no
237+
| RENAME first_name AS last_name, last_name AS first_name, first_name as emp_no, emp_no AS first_name
238+
| LIMIT 2
239+
;
240+
241+
first_name:keyword
242+
Georgi
243+
Bezalel
244+
;
245+
246+
247+
reuseRenamedAlias
248+
required_capability: rename_sequential_processing
249+
FROM employees
250+
| SORT emp_no ASC
251+
| KEEP first_name, last_name
252+
| LIMIT 2
253+
| RENAME first_name AS x, x AS y, last_name as x
254+
;
255+
256+
y:keyword | x:keyword
257+
Georgi | Facello
258+
Bezalel | Simmel
259+
;
260+
261+
262+
multipleRenamesToSameAliasLastOnePrevails
263+
required_capability: rename_sequential_processing
264+
FROM employees
265+
| SORT emp_no ASC
266+
| KEEP first_name, last_name
267+
| LIMIT 2
268+
| RENAME first_name AS x, last_name as x
269+
;
270+
271+
x:keyword
272+
Facello
273+
Simmel
274+
;
275+
276+
277+
swapNames
278+
required_capability: rename_sequential_processing
279+
ROW a="keyword", b=5
280+
| RENAME a AS temp, b AS a, temp AS b
281+
;
282+
283+
b:keyword | a:integer
284+
keyword | 5
285+
;
286+
287+
288+
multipleRenames
289+
required_capability: rename_sequential_processing
290+
ROW a="keyword", b=5, c=null
291+
| RENAME a AS c, b AS a
292+
| RENAME c AS b
293+
| RENAME a AS b, b AS a
294+
;
295+
296+
a:integer
297+
5
298+
;

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -407,6 +407,12 @@ public enum Cap {
407407
*/
408408
UNION_TYPES_FIX_RENAME_RESOLUTION,
409409

410+
/**
411+
* Execute `RENAME` operations sequentially from left to right,
412+
* see <a href="https://github.com/elastic/elasticsearch/issues/122250"> ESQL: Align RENAME behavior with EVAL for sequential processing #122250 </a>
413+
*/
414+
RENAME_SEQUENTIAL_PROCESSING,
415+
410416
/**
411417
* Fix for union-types when some indexes are missing the required field. Done in #111932.
412418
*/

x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/analysis/Analyzer.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -986,6 +986,7 @@ public static List<NamedExpression> projectionsForRename(Rename rename, List<Att
986986
if (alias.child() instanceof UnresolvedAttribute ua && alias.name().equals(ua.name()) == false) {
987987
// remove attributes overwritten by a renaming: `| keep a, b, c | rename a as b`
988988
projections.removeIf(x -> x.name().equals(alias.name()));
989+
childrenOutput.removeIf(x -> x.name().equals(alias.name()));
989990

990991
var resolved = maybeResolveAttribute(ua, childrenOutput, logger);
991992
if (resolved instanceof UnsupportedAttribute || resolved.resolved()) {

x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/inference/adaptiveallocations/AdaptiveAllocationsScalerService.java

Lines changed: 32 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
import org.elasticsearch.xpack.core.ml.action.GetDeploymentStatsAction;
3131
import org.elasticsearch.xpack.core.ml.action.UpdateTrainedModelDeploymentAction;
3232
import org.elasticsearch.xpack.core.ml.inference.assignment.AssignmentStats;
33+
import org.elasticsearch.xpack.core.ml.inference.assignment.RoutingState;
3334
import org.elasticsearch.xpack.core.ml.inference.assignment.TrainedModelAssignment;
3435
import org.elasticsearch.xpack.core.ml.inference.assignment.TrainedModelAssignmentMetadata;
3536
import org.elasticsearch.xpack.ml.MachineLearning;
@@ -213,6 +214,7 @@ Collection<DoubleWithAttributes> observeDouble(Function<AdaptiveAllocationsScale
213214
private volatile Scheduler.Cancellable cancellable;
214215
private final AtomicBoolean busy;
215216
private final long scaleToZeroAfterNoRequestsSeconds;
217+
private final long scaleUpCooldownTimeMillis;
216218
private final Set<String> deploymentIdsWithInFlightScaleFromZeroRequests = new ConcurrentSkipListSet<>();
217219
private final Map<String, String> lastWarningMessages = new ConcurrentHashMap<>();
218220

@@ -224,7 +226,17 @@ public AdaptiveAllocationsScalerService(
224226
MeterRegistry meterRegistry,
225227
boolean isNlpEnabled
226228
) {
227-
this(threadPool, clusterService, client, inferenceAuditor, meterRegistry, isNlpEnabled, DEFAULT_TIME_INTERVAL_SECONDS);
229+
this(
230+
threadPool,
231+
clusterService,
232+
client,
233+
inferenceAuditor,
234+
meterRegistry,
235+
isNlpEnabled,
236+
DEFAULT_TIME_INTERVAL_SECONDS,
237+
SCALE_TO_ZERO_AFTER_NO_REQUESTS_TIME_SECONDS,
238+
SCALE_UP_COOLDOWN_TIME_MILLIS
239+
);
228240
}
229241

230242
// visible for testing
@@ -235,7 +247,9 @@ public AdaptiveAllocationsScalerService(
235247
InferenceAuditor inferenceAuditor,
236248
MeterRegistry meterRegistry,
237249
boolean isNlpEnabled,
238-
int timeIntervalSeconds
250+
int timeIntervalSeconds,
251+
long scaleToZeroAfterNoRequestsSeconds,
252+
long scaleUpCooldownTimeMillis
239253
) {
240254
this.threadPool = threadPool;
241255
this.clusterService = clusterService;
@@ -244,14 +258,15 @@ public AdaptiveAllocationsScalerService(
244258
this.meterRegistry = meterRegistry;
245259
this.isNlpEnabled = isNlpEnabled;
246260
this.timeIntervalSeconds = timeIntervalSeconds;
261+
this.scaleToZeroAfterNoRequestsSeconds = scaleToZeroAfterNoRequestsSeconds;
262+
this.scaleUpCooldownTimeMillis = scaleUpCooldownTimeMillis;
247263

248264
lastInferenceStatsByDeploymentAndNode = new HashMap<>();
249265
lastInferenceStatsTimestampMillis = null;
250266
lastScaleUpTimesMillis = new HashMap<>();
251267
scalers = new HashMap<>();
252268
metrics = new Metrics();
253269
busy = new AtomicBoolean(false);
254-
scaleToZeroAfterNoRequestsSeconds = SCALE_TO_ZERO_AFTER_NO_REQUESTS_TIME_SECONDS;
255270
}
256271

257272
public synchronized void start() {
@@ -375,6 +390,9 @@ private void processDeploymentStats(GetDeploymentStatsAction.Response statsRespo
375390

376391
Map<String, Stats> recentStatsByDeployment = new HashMap<>();
377392
Map<String, Integer> numberOfAllocations = new HashMap<>();
393+
// Check for recent scale ups in the deployment stats, because a different node may have
394+
// caused a scale up when an inference request arrives and there were zero allocations.
395+
Set<String> hasRecentObservedScaleUp = new HashSet<>();
378396

379397
for (AssignmentStats assignmentStats : statsResponse.getStats().results()) {
380398
String deploymentId = assignmentStats.getDeploymentId();
@@ -401,6 +419,12 @@ private void processDeploymentStats(GetDeploymentStatsAction.Response statsRespo
401419
(key, value) -> value == null ? recentStats : value.add(recentStats)
402420
);
403421
}
422+
if (nodeStats.getRoutingState() != null && nodeStats.getRoutingState().getState() == RoutingState.STARTING) {
423+
hasRecentObservedScaleUp.add(deploymentId);
424+
}
425+
if (nodeStats.getStartTime() != null && now < nodeStats.getStartTime().toEpochMilli() + scaleUpCooldownTimeMillis) {
426+
hasRecentObservedScaleUp.add(deploymentId);
427+
}
404428
}
405429
}
406430

@@ -416,9 +440,12 @@ private void processDeploymentStats(GetDeploymentStatsAction.Response statsRespo
416440
Integer newNumberOfAllocations = adaptiveAllocationsScaler.scale();
417441
if (newNumberOfAllocations != null) {
418442
Long lastScaleUpTimeMillis = lastScaleUpTimesMillis.get(deploymentId);
443+
// hasRecentScaleUp indicates whether this service has recently scaled up the deployment.
444+
// hasRecentObservedScaleUp indicates whether a deployment recently has started,
445+
// potentially triggered by another node.
446+
boolean hasRecentScaleUp = lastScaleUpTimeMillis != null && now < lastScaleUpTimeMillis + scaleUpCooldownTimeMillis;
419447
if (newNumberOfAllocations < numberOfAllocations.get(deploymentId)
420-
&& lastScaleUpTimeMillis != null
421-
&& now < lastScaleUpTimeMillis + SCALE_UP_COOLDOWN_TIME_MILLIS) {
448+
&& (hasRecentScaleUp || hasRecentObservedScaleUp.contains(deploymentId))) {
422449
logger.debug("adaptive allocations scaler: skipping scaling down [{}] because of recent scaleup.", deploymentId);
423450
continue;
424451
}

0 commit comments

Comments
 (0)