Skip to content

Use context-aware wait in Kafka sender retry loop#187

Merged
andrewwormald merged 2 commits intomainfrom
fix/kafka-sender-context-aware-retry
Mar 10, 2026
Merged

Use context-aware wait in Kafka sender retry loop#187
andrewwormald merged 2 commits intomainfrom
fix/kafka-sender-context-aware-retry

Conversation

@andrewwormald
Copy link
Collaborator

@andrewwormald andrewwormald commented Mar 10, 2026

Problem

Kafka sender retry uses time.Sleep(100ms) which ignores context cancellation. During shutdown, the goroutine blocks for the full sleep duration.

Why

Shutdown signals are delayed — in a tight retry loop this adds unnecessary latency to graceful shutdown.

Fix

Replace time.Sleep with context-aware wait() that returns immediately on cancellation.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes

    • Enhanced Kafka writer error handling to support context cancellation and properly propagate context errors during retries.
  • Tests

    • Added comprehensive test coverage for Kafka writer timing behaviour and context cancellation scenarios.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@coderabbitai
Copy link

coderabbitai bot commented Mar 10, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: a8be9dc6-b9d4-43a1-a36a-48a19c32c900

📥 Commits

Reviewing files that changed from the base of the PR and between f4d926f and d948e3e.

📒 Files selected for processing (2)
  • adapters/kafkastreamer/kafka.go
  • adapters/kafkastreamer/kafka_internal_test.go

📝 Walkthrough

Walkthrough

This pull request modifies Kafka writer error handling by replacing a hardcoded 100ms sleep with a context-aware wait helper function. The change enables the retry logic to respond to context cancellation events by propagating context errors, whilst maintaining the existing retry-once behaviour. Corresponding test coverage is added with four new test cases that exercise the wait helper with zero duration, context cancellation, normal duration completion, and Send() method cancellation scenarios.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐰 A context-aware hop through the Kafka streams so keen,
No more sleeping blindly at a hundred milliseconds seen,
With cancellation whispers caught mid-air,
And tests that prove our rabbits care,
We retry wisely, context-aware! 🏔️✨

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main change: replacing fixed sleep with a context-aware wait in the Kafka sender retry loop.
Description check ✅ Passed The description is directly related to the changeset, clearly explaining the problem, rationale, and solution for context-aware waiting in the Kafka sender.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/kafka-sender-context-aware-retry

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@sonarqubecloud
Copy link

@andrewwormald andrewwormald merged commit 2c1bcf6 into main Mar 10, 2026
9 checks passed
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.

2 participants