[flink] improve lifecycle handling of GroupAlsoByWindowWrapper

[mxm]

Could someone have a look if these minor changes are good to merge? @davorbonaci @kennknowles

Note, that there are two commits. One for the lifecycle of the Operator, another one for improving code readability.

[kennknowles]

LGTM.

One consideration off the top of my head: one reason we process a whole KeyedWorkItem at once is so we can eagerly merge together session windows. Essentially a keyed work item represents a whole bundle so the ReduceFnRunner can go aggregate work. This is less of an issue than it used to be since we re-use state cells and windows instead of building an unbounded tree of merged windows. @mshields822 might have more to suggest.

Jenkins failure is a known issue with a conflict between local spark endpoint configuration and our GCE workers. We are investigating, but it should not block progress.

[davorbonaci]

LGTM.

[dhalperi]

R: @kennknowles @davorbonaci

is it possible to kick the Jenkins now that it's been fixed? Otherwise, what's blocking merge?

[davorbonaci]

Unfortunately, we don't know how to kick Jenkins to re-run the build without pushing to this branch. If somebody has an idea, I'd love to hear it. (Standard things like Rebuild button in Jenkins UI and adding the specific comment here don't work.)

I think this is merge-ready. I'd leave it to @mxm -- just in case if there's any comment on Kenn's previous one, or if @mshields822 wants to add anything.

[kennknowles]

One way to kick Jenkins, which seems fine at this point, would be to rebase to a different upstream commit (it doesn't really matter which one).

Agree that it should just be merged. We have a very strong signal from Travis, and in fact the Flink runner tests run first in Jenkins so they are known to have passed.

[mxm]

Thanks for the comments. I can kick the Jenkins build (as a Jenkins job admin). Just did that and will merge after tests have passed.