Skip to content

Alternative approach#2

Draft
carlosdelest wants to merge 7 commits into
kkharbas:dense_vector-arithmetic-scalarfrom
carlosdelest:dense_vector-arithmetic-scalar-alternative
Draft

Alternative approach#2
carlosdelest wants to merge 7 commits into
kkharbas:dense_vector-arithmetic-scalarfrom
carlosdelest:dense_vector-arithmetic-scalar-alternative

Conversation

@carlosdelest

@carlosdelest carlosdelest commented Feb 12, 2026

Copy link
Copy Markdown

Hey Kishor:

I've been looking at this - didn't come with a better solution, but wanted to get your opinion on some changes. Feel free to dismiss!

kkharbas and others added 7 commits February 9, 2026 18:08
Adds support for arithmetic operations - '+', '-', '*' and '/' when one operands is a dense_vector and other is scalar. Performs the arithmetic operation
on each element of the dense_vector against the scalar operand.

Closes elastic#140538
return new Add(source(), left(), right(), configuration);
}

private static final DenseVectorBinaryEvaluator ADD_DENSE_VECTOR_EVALUATOR = new DenseVectorBinaryEvaluator() {

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've moved the evaluators to their corresponding operations - kind of made more sense to me

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've removed this one in favor of DenseVectorScalarEvaluator - but it's a very similar solution, only using a single Evaluator and flipping the operation to do

return warnings;
}

static float processAdd(double lhs, double rhs) {

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I moved the specific subclasses to the operations

this.source = source;
this.scalar = lhs;
this.vector = rhs;
// flip the order of arguments for scalar-vector operations, as we assume the scalar is always the rhs in the processing

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here is the trick - we use a single evaluator, and flip the operation args in case the scalar - vector are in different order than expected

public interface DenseVectorBinaryEvaluator {
// when both arguments are dense_vectors
EvalOperator.ExpressionEvaluator.Factory apply(
EvalOperator.ExpressionEvaluator.Factory vectorOperation(

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very similar to what you had - just renaming

@kkharbas
kkharbas force-pushed the dense_vector-arithmetic-scalar branch from 2e234fe to a6ee999 Compare February 23, 2026 19:30
kkharbas pushed a commit that referenced this pull request May 29, 2026
Without --foreground, GNU timeout(1) puts the wrapped bash (and its
gradle child) into a fresh process group. The gradle CLI JVM then
prints BUILD SUCCESSFUL + the build-scan URL but never exits — the
develocity scan plugin's shutdown path stalls until our inner
timeout fires SIGTERM, ~36 minutes wasted per flakiness step.

Confirmed via a 4-variant matrix run as build #2 of
elasticsearch-flakiness-detection-manual on branch
wrapper-hang-diagnostic:

  D bare gradle, no wrapper      1m25s   baseline
  B wrapper + --foreground       1m39s   +14s vs baseline (clean)
  A current wrapper              3m23s   killed by inner timeout
  C wrapper + gradle --no-daemon 3m31s   killed by inner timeout

Only A and C reproduce the production hang; --foreground (B) fixes it
without changing gradle invocation. --no-daemon (C) does not help,
ruling out the gradle daemon as the cause.

Fixes the systematic 58-minute wall every recent flakiness-detection
step has been hitting (builds 150149, 150259, 150411, 150439, 150444,
150445, 150515, ...).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants