Consolidate the retry utilities around a single value-returning retry loop
- Dominant language
- HTML
- Stars
- 6
- Forks
- 8
- Avg merge
- 2d 19h
- Merged PRs (30d)
- 1
Description
The codebase currently contains two independent implementations of the same retry loop:
- `io.debezium.util.RetryingRunnable` (void actions, configurable retriable exception types with cause-chain matching, optional auto-heal)
- `io.debezium.embedded.async.RetryingCallable` (value-returning, hard-coded to Kafka Connect `RetriableException`, top-level match only)
The duplication is historical rather than intentional: `RetryingRunnable`'s javadoc itself records the copy ("Inspired by: io.debezium.embedded.async.RetryingCallable"). It exists because of the module dependency direction: `debezium-embedded` depends on `debezium-connector-common`, so common code cannot reuse the embedded class. Each class currently has exactly one production consumer (`AsyncEmbeddedEngine.PollRecords` and `SignalBasedIncrementalSnapshotChangeEventSource` respectively).
The two have already started drifting: dbz#2329 recently fixed `RetryingRunnable` so that the configured `DelayStrategy` is applied when no auto-heal action is set, while `RetryingCallable` still has its own copy of the loop with slightly different behavior. Any future fix has to be discovered and applied twice.
There are also two functional gaps:
1. There is no reusable value-returning retry utility outside of `debezium-embedded`. Modules that cannot depend on the embedded engine (for example `debezium-connector-common`, which `debezium-embedded` itself depends on) have nothing to build on, which is how the `RetryExecutor` class in PR #7362 came to exist. As requested in the review of that PR, this issue tracks consolidating it with the existing utilities.
2. `DelayStrategy` has no jittered variant. Deployments commonly run several connectors or engine tasks against the same database instance, and each task's polling loop already retries with `DelayStrategy.exponential` (`AsyncEmbeddedEngine.PollRecords`): after a shared failure of that instance they all wake up in lockstep, hitting the recovering database with synchronized reconnection waves. The parallel incremental snapshot proposed in #7362 would add per-worker retry loops inside a single connector, making a jittered variant even more relevant.
Proposal:
- Introduce `io.debezium.util.RetryingSupplier`: same builder contract as `RetryingRunnable` (retries, retriable exception types with cause-chain matching, optional auto-heal, pluggable `DelayStrategy`), but value-returning and generic in the checked exception type, so callers keep their `throws` contracts without wrapping. It hosts the single retry loop, with the exact semantics established by dbz#2329 (delay on every failed attempt when no auto-heal is configured; immediate retry after a successful auto-heal; delay when the auto-heal fails).
- Turn `RetryingRunnable` into a thin adapter over `RetryingSupplier`. No public API change and no behavior change.
- Turn `RetryingCallable` into a thin adapter over `RetryingSupplier` as well. This is the one deliberate behavior change of the proposal: `RetriableException` is then also matched through the cause chain, so a retriable exception wrapped by a connector (a common pattern, and the reason `io.debezium.util.ErrorHandler` walks the cause chain) triggers a retry instead of failing the task. If preferred, this part can be split into a separate follow-up.
- Add a jittered exponential factory to `DelayStrategy` (existing factories untouched).
PR #7362 would then drop its `RetryExecutor` class entirely and build on `RetryingSupplier` at its next iteration.
Contributor guide
Assessment
This issue has not been assessed yet.