vbhanuchander-lang commented on PR #309:
URL:
https://github.com/apache/flink-connector-kafka/pull/309#issuecomment-5627951004
@MartijnVisser you were right to retract that nit — taken in this PR, and
thanks for working the chain through. Pushed.
**Metadata change now clears the set**, in `onHandleSubscribedStreamsFetch`
immediately after `closeAllEnumeratorsAndContexts()`, which is the point where
the previous generation stops existing: every sub enumerator is about to be
recreated and will signal for its new assignments, and the reader has closed
and recreated its sub readers back to `noMoreSplitsAssignment == false`. I went
with clearing at the recreation point rather than keying the set on a
generation counter, since the clear sits exactly where the generation changes
and needs no extra state; happy to switch to an explicit generation key if you
would rather have it be impossible to forget at a future recreation site. The
commit message now describes both clears as the two points that define a
generation.
**I could not write a test that proves it, and would rather say so than fake
it.** I wrote one — change metadata mid-run, then drive any path back into
`handleNoMoreSplits` and assert the readers are signalled a second time. It
fails at `expected: 2, but was: 1`, because after the recreation the new sub
enumerators have not signalled, so `allEnumeratorsHaveSignalledNoMoreSplits` is
false and `handleNoMoreSplits` takes the "not ready" branch — the FLINK-31006
gap you described, from the other side. The only ways I found to make it green
were asserting the private set through `Whitebox` or contriving the
precondition, both of which test the harness rather than the behaviour, so I
dropped the test. If you want coverage now I am happy to add whichever form you
prefer, or to add the real one to the 31006 change so it lands with the
behaviour it asserts.
**On the CI failure — that was my mistake twice over and worth naming, since
it explains why a "fixed" formatting failure came back.**
`flink-connector-parent` skips spotless on Java 21 ("Current google format does
not run on Java 21"), and I was running JDK 21, so both `spotless:apply` and
`spotless:check` were no-ops and reported success while changing nothing.
Re-run under JDK 17 it reformatted four wrappings immediately. I now check that
the goal actually executed rather than trusting the exit code: `spotless:check`
and `checkstyle:check` exit 0 under JDK 17 with no "Spotless check skipped"
lines, and the class is 31/31 locally.
Still happy to sequence this behind #291 — say the word if you would rather
I rebase on it once it lands.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
