weiqingy opened a new pull request, #978: URL: https://github.com/apache/flink-agents/pull/978
Linked issue: #976 ### Purpose of change `checkpointing.interval` is not a Flink configuration key. The real one is `execution.checkpointing.interval` (`CheckpointingOptions#CHECKPOINTING_INTERVAL`). The bare form is not registered, and it is not wired as a deprecated or fallback key either, so Flink ignores it silently. Since the real option has `noDefaultValue()`, no periodic checkpointing was ever scheduled. The Python e2e tests set the bare form in 11 places across 5 files, so they configure a RocksDB state backend that is never snapshotted while reading as though they exercise checkpointing. The first commit renames the key at all 11 sites. The neighbouring `state.backend.type` and `restart-strategy.type` are both genuine registered options and are left unchanged. The second commit adds one test so this cannot recur. Nothing today can catch this class of bug: Flink emits no signal for an unregistered key, `Configuration` logs only for registered fallbacks, PyFlink writes to a file appender rather than the console, and no lint rule can check a plain string argument. The guard scans the e2e sources for the checkpoint interval keys they configure and compares them against the key read from Flink itself, so neither side of the comparison is a hand-written copy of the string. A guard carrying its own copy of the key would pass while every real call site stayed broken. Two things worth setting expectations on. The rename does not make these tests meaningfully exercise checkpointing: measured on Flink 2.2 it yields one completed checkpoint per job, of largely drained state, because the operator drains its in-flight keys before the snapshot lands. Getting a checkpoint to complete with in-flight state did not happen at intervals down to 10ms, with slow agents, or with larger inputs. Doing better needs a different test design rather than a configuration fix. The other is that the fix activates a path that was previously dead. At every site `tolerable-failed-checkpoints` is 0 and `restart-strategy.type` is `disable`, and with no checkpoint directory configured the storage falls back to JobManager memory, which caps state at 5 MiB. So a failed or oversized checkpoint now fails the job outright, where before it could not occur at all. Nothing hit this in practice, but it is the first thing to check if these tests ever go flaky. ### Tests `./tools/ut.sh -p -e -f 2.2` passes. All 14 tests across the 5 renamed files pass, and they pass on the first attempt rather than being retried by `--reruns`. `mcp_test.py` is normally skipped without a local Ollama, so a local `qwen3:1.7b` was stood up to cover the 11th site rather than leaving it to CI. Both parametrizations pass. The new key was checked against the shipped `flink-dist` jar rather than taken from documentation: `CHECKPOINTING_INTERVAL.key()` resolves to `execution.checkpointing.interval`, its fallback keys are empty, reading the option from a `Configuration` holding the bare key returns `null`, and `"1s"` parses to `PT1S` for its `Duration` type, so the value needed no change. The same holds on 1.20.3. The option is present in `flink-core` on 1.20, 2.0, 2.1, 2.2 and 2.3; 2.0, 2.1 and 2.3 were verified by inspection rather than executed locally. The guard was verified to fail, not just to pass. It goes red on the original defect, on a `set_integer` spelling, on `execution.checkpoint.interval`, on a single-quoted key, on all of those combined, on the longer registered key `execution.checkpointing.interval-during-backlog`, and on a scan that matches nothing. Each failure names the offending file. It produces no false positives: exactly 11 matches across the 5 files, resolving to the single expected key. The guard sits in the pytest selection that runs the full Flink version matrix, since what it checks is whether the key resolves against a given Flink version. It costs about 10ms there, where the JVM gateway is already warm. ### API No public API change. Both commits are test-only. ### Documentation - [ ] `doc-needed` - [x] `doc-not-needed` - [ ] `doc-included` ### Was this patch authored or co-authored using generative AI tooling? - [x] Yes - [ ] No Generated-by: Claude Code 2.1.224 -- 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]
