Add more flexibility to MovingFunction window alignment

[Hohol]

Introduce shift field to MovingFunction aggregation.
By default, shift = 0. Behavior, in this case, is the same as before.
Increasing shift by 1 moves starting window position by 1 to the right.

• To simply include current bucket to the window, use shift = 1
• For center alignment (n/2 values before and after the current bucket), use shift = window / 2
• For right alignment (n values after the current bucket), use shift = window.

Closes #42181

[Hohol]

@polyfractal, check this out.
It's only a draft, so a lot of things are still missing.
First of all, could you take a look at tests in MovFnUnitTests and say if you find the expected behaviour in these tests reasonable?
Particularly, I'm not entirely sure how the result should look like when the window moves outside the borders.

[Hohol]

There is a serialization of MovFnPipelineAggregator values. Do we need cross-version compatibility this time?

[Hohol]

We can allow specifying special shift values using descriptive names.
Namely for inclusive, center and right alignments.
Do we need it?

[elasticmachine]

Pinging @elastic/es-analytics-geo

[polyfractal]

Particularly, I'm not entirely sure how the result should look like when the window moves outside the borders.

Checking my understanding: the current behavior is to "clamp" to the available values in the window, correct? So as the window approaches the edge, the actual window of values offered to the function shrinks? E.g. there is no "padding" once the window goes past the values (on either side).

I think that's the correct choice. I dont think we want to introduce padding, or circularizing like is done with FFT. Too complicated, unintuitive. Clamping seems reasonable to me.

There is a serialization of MovFnPipelineAggregator values. Do we need cross-version compatibility this time?

Afraid so. :( We'll need to put version gates on the serialization, and handle the case where we are talking to old nodes that don't serialize shift (setting shift = 0 in that case)

We can allow specifying special shift values using descriptive names.

Hmm, good question. I'm leaning towards "no", and just use the numeric shift parameter. Simpler that way and as much flexibility as a user wants.

[Hohol]

We'll need to put version gates on the serialization, and handle the case where we are talking to old nodes that don't serialize shift (setting shift = 0 in that case)

What if we're writing to an old node and shift != 0? Should we just ignore it, so the node will work as if shift was 0, or should we throw an exception?

[polyfractal]

Yeah, in these cases we assume the old behavior, so here we'd just avoid sending shift and the old node works as if shift == 0.

Heterogeneous clusters are assumed to be short-lived and only during rolling upgrades, so with regards to bwc in mixed clusters we just try not to break things while in that mixed state.

Since pipelines are only executed on the coordinating node, the behavior will depend on if the query lands on a new or old node. Serialization is needed because they go out with the full agg tree, but the execution only happens on the coordinator.

[Hohol]

Ready for review.

[polyfractal]

Left a few comments, but otherwise LGTM! Will kick off a CI build after the last commit. Thanks!

[polyfractal]

@elasticmachine test this please

[polyfractal]

Sorry for the delay, was out on holiday last week. Not sure why CI got stuck like that, going to trigger another build.

@elasticmachine test this please

[Hohol]

Maybe I need to merge with master?

[polyfractal]

++ Let's merge in master, that tends to resolve unrelated failures :)

[polyfractal]

@elasticmachine test this please

[polyfractal]

Hmm, ILM EOFException exception on serialization. I think it's unrelated although we should keep an eye out if we see more serialization issues... might be side-effect of changes here. Re-running bwc tests.

@elasticmachine run elasticsearch-ci/bwc

[Hohol]

Failed again. This time it's a different error.

[polyfractal]

Bleh, that's an artifact from the version bump yesterday. Gonna merge master and start again. Sorry, it's not normally this tough to get a green CI :(

[polyfractal]

@elasticmachine update branch

[polyfractal]

@elasticmachine test this please

[polyfractal]

🎉 merging! Thanks @Hohol :)