Add Kafka offset-commit restart case for delivery verification#9
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughThis PR adds a new test case type ChangesKafka Offset Commit Restart Correctness Verification
Sequence DiagramsequenceDiagram
participant TestRunner
participant Generator
participant Subject
participant Receiver
participant MetricsStore
TestRunner->>Generator: Start
TestRunner->>Subject: Start
TestRunner->>Receiver: Start
Generator->>Receiver: Produce messages
Receiver->>Receiver: Consume and track count
Generator->>TestRunner: Complete (sent count)
Receiver->>TestRunner: Reached full delivery count
TestRunner->>TestRunner: Settle period
TestRunner->>Subject: Stop gracefully
TestRunner->>Subject: Restart
Subject->>Receiver: Resume consumption
Receiver->>TestRunner: Final count (detect re-consumption)
TestRunner->>MetricsStore: Compute loss and over-delivery %
TestRunner->>MetricsStore: Validate against thresholds
TestRunner->>MetricsStore: Persist RunResult
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/config/case.go`:
- Around line 526-533: The Case validation currently does not reject negative
MaxOverDeliveryPct, so add a validation check in the Case.Validate() method to
ensure MaxOverDeliveryPct is >= 0 and return an explicit validation error when
it's negative; reference the MaxOverDeliveryPct field on the Case struct and
update Validate() to produce a clear error message like "max_overdelivery_pct
must be non-negative" (or equivalent) so malformed case files fail fast during
parsing/validation rather than later in the runner.
In `@internal/runner/runner.go`:
- Around line 1767-1770: The check currently reads only tc.Generator.TotalLines
and aborts if it's 0; update the logic in the runner (the block referencing
tc.Generator.TotalLines) to account for plural generators or actual generator
output by computing the effective total lines across tc.Generators when
tc.Generator is not set or has TotalLines==0 (e.g., sum TotalLines from
tc.Generators or derive from provided generator outputs) and only error if the
computed total is <= 0; apply the same fix to the second occurrence around the
block that references the same fields (the block near the other check at lines
~1795-1800) so both paths accept cases using plural generators or non-singular
generator configurations.
- Around line 1802-1893: The runner currently uses LinesReceived
(rm.LinesReceived / recvMetrics.LinesReceived) to detect full delivery and
compute loss/over-delivery, which can false-pass when unique records are lost
but total deliveries match; update the logic to use UniqueLines for delivery and
stability checks (replace rm.LinesReceived checks in the "waiting for full
delivery" loop and the "observing for re-consumption" loop with rm.UniqueLines),
and compute lossPct/overPct using recvMetrics.UniqueLines (and consider
recvMetrics.Duplicates separately for reporting); also update printed messages
that show counts to reflect UniqueLines where appropriate (keep
LinesReceived/Duplicates available in logs but base correctness decisions on
UniqueLines).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e3e3d78f-297b-4266-8684-d8357d624dbc
📒 Files selected for processing (2)
internal/config/case.gointernal/runner/runner.go
Member
|
@namles can you resolve the ai reviews? Thanks in advance! |
Deploying pipebench with
|
| Latest commit: |
30f0a79
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://d6619978.pipebench.pages.dev |
| Branch Preview URL: | https://kafka-offset-commit-restart.pipebench.pages.dev |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Introduce a new test case to verify that delivery-bound source acknowledgments persist correctly during a Kafka offset-commit restart. This ensures that after a graceful restart, the consumer resumes from committed offsets without re-consuming messages, thus enforcing delivery limits and preventing over-delivery.
Summary by CodeRabbit