Skip to content

fix(kafka-sink): return error instead of silent Ok on missing commit restore state#1099

Open
kotwal-itpro wants to merge 1 commit into
ArroyoSystems:masterfrom
kotwal-itpro:fix/kafka-sink-return-error-on-missing-commit-state
Open

fix(kafka-sink): return error instead of silent Ok on missing commit restore state#1099
kotwal-itpro wants to merge 1 commit into
ArroyoSystems:masterfrom
kotwal-itpro:fix/kafka-sink-return-error-on-missing-commit-state

Conversation

@kotwal-itpro

@kotwal-itpro kotwal-itpro commented Jul 8, 2026

Copy link
Copy Markdown

Problem

The exactly-once Kafka sink's handle_commit path silently returned Ok(()) when producer_to_complete was None — a state that indicates the operator restarted between handle_checkpoint and handle_commit and lost the in-memory reference to the transactional producer. In that state the pending Kafka transaction may have already committed on the broker, aborted, or still be open; the sink has no way to tell without querying transaction state, and checkpoint restore during the two-phase-commit window is not yet implemented (as the pre-existing error!("... Restoring from commit phase not yet implemented") on that branch already noted).

Silently returning Ok(()) told the checkpoint coordinator the commit succeeded when its outcome was in fact unknown, breaking the exactly-once contract.

Fix

Return a DataflowError::ConnectorError { domain: Internal, retry: NoRetry, ... } on that branch so the pipeline fails loudly rather than proceeding with unknown consistency. Uses the existing connector_err! macro.

Test

Adds test_handle_commit_returns_error_when_producer_state_missing in kafka/sink/test.rs that constructs a KafkaSinkFunc in ExactlyOnce { producer_to_complete: None } mode and asserts handle_commit returns Err. The test does not require a live Kafka broker because the None branch returns before any producer call.

Existing tests unchanged.


Open in Devin Review

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no bugs or issues to report.

Open in Devin Review

…restore state

The exactly-once Kafka sink's handle_commit path silently returned Ok(()) when
producer_to_complete was None — a state that indicates the operator restarted
between handle_checkpoint and handle_commit and lost the in-memory reference
to the transactional producer. In that state the pending Kafka transaction may
have already committed on the broker, aborted, or still be open; the sink has
no way to tell without querying transaction state, and checkpoint restore
during the two-phase-commit window is not yet implemented (as the pre-existing
error message on that branch noted).

Silently returning Ok(()) told the checkpoint coordinator the commit succeeded
when its outcome was in fact unknown, breaking the exactly-once contract.
Return a DataflowError instead so the pipeline fails loudly rather than
proceeding with unknown consistency.

Adds a unit test covering the None-producer path that does not require a live
Kafka broker.
@kotwal-itpro kotwal-itpro force-pushed the fix/kafka-sink-return-error-on-missing-commit-state branch from 9562721 to f7ffbc7 Compare July 8, 2026 07:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant