debezium / debezium/dbz

Consolidate the retry utilities around a single value-returning retry loop

Open
#2,345 0 comments 0 reactions 0 assignees View on GitHub
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

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.