Rebatch after filters when using pipeline.ordered

[colinsurprenant]

Fixes #11550
Relates to #11175

WIP TODOs

• major cleanup
• refactor & simplify the filtersExecution + lastFilter logic
• re-enable and fix relevant java tests
• benchmark

Description

When using pipeline.ordered the batch events are sent 1 by 1 down the filters and output. This PR rebatches after the filters so that full batches can be sent to the outputs. It does that by separating the filters and output execution with the introduction of a separator vertex in the pipeline graph between the filters and outputs and compiling both sections separately and in the worker_loop calling both compute separately which provides the possibility to rebatch after the filters.

[colinsurprenant]

I performed the same benchmarks as in #11524 (comment) (but on different hardware, but it's not really important since relative numbers are interesting here). Also, I noticed variations of ~5-10% between runs with the same parameters which I attribute to CPU throttling.

Config1

Java Execution

pipeline.ordered	TPS master	TPS PR
false	158k/s	156k/s
true	80k/s	155k/s

Ruby Execution

pipeline.ordered	TPS master	TPS PR
false/true	152k/s	157k/s

Config2

Java Execution

pipeline.ordered	TPS master	TPS PR
false	70k/s	75k/s
true	38k/s	56k/s

Ruby Execution

pipeline.ordered	TPS master	TPS PR
false/true	60k/s	63k/s

[yaauie]

First pass review: looks like we are headed in the right direction. The intermediate vertex is a clever solution that allows us to keep the graph validation logic, which wouldn't be possible if we ended up with two graphs.

I've left a number of comments and nitpicks, more discussion points than hard requests. The refactors you've brought in give us some opportunity to go a small step further in a few places to provide additional clarity and clearer abstractions.

[andsel]

Nice job @colinsurprenant I've added some nits (probably almost all on personal taste) and a note on a possible recursion (IDEA doesn't show me as it, and I've not tried locally, but I suspect it)

Do you plan to unit ad tests, or the existing give us good confidence?

[colinsurprenant]

Thanks a lot for the reviews @yaauie @andsel for the reviews! I pushed some suggested modifications, will be working on others today. I'd also like your thoughts about the SeparatorVertex comments (keep as-is with an explicit ID or refactor into one more abstract class). LMKWYT.

[colinsurprenant]

@andsel @yaauie I think we are ready for a another round of review - as per my comments

• I suggest we leave the SeparatorVertex implementation as-is here and followup with a broader Vertex refactor which will include the QueueVertex.
• Also defer a AckedReadBatch refactor. There is some cleanups and simplifications to be done there but is out of scope here.

[yaauie]

I think this is looking good, and concentrates the complexity in appropriate places.

I've added a couple nitpicks, praises, and one suggestion to squash a build warning that fails the build.

[andsel]

Only a really minor comment, LGTM

[yaauie]

LGTM with the deferrals noted above.

[colinsurprenant]

Thanks a lot @yaauie @andsel for the excellent reviews!

[colinsurprenant]

Merged in master (8.0), 7.x #11726 and 7.7 #11730