S3C-11127: add the notification delivery replay tool - #2801
Draft
anurag4DSB wants to merge 3 commits into
Draft
Conversation
The delivery topic is keyed by destination, optionally spread over several keys, so that records of one object always land on the same partition. The populator and the cutover replay both build the key, hence the shared helper.
At cutover the old per-destination queue processors leave records behind in the internal topics. DeliveryTopicDrainer replays them as addressed records so the new delivery worker pool starts with nothing lost: it reads each old group's committed offsets, consumes from there up to the head captured at start, rematches every record against the bucket configuration to recover its configurationId, and produces it keyed like the populator would. It never writes a consumer group offset, so a failed run can be rerun and a rollback to the old pipeline still resumes where it stopped. Duplicates are the accepted cost of that: any error aborts the drain rather than stepping over a record.
Thin wrapper around DeliveryTopicDrainer, run once during cutover with the populator and the queue processors stopped. Exits 0 on a full drain, non-zero otherwise, so the cutover can gate on it.
Codecov Report❌ Patch coverage is
Additional details and impacted files
... and 3 files with indirect coverage changes
@@ Coverage Diff @@
## development/9.3 #2801 +/- ##
===================================================
- Coverage 74.70% 74.38% -0.33%
===================================================
Files 199 202 +3
Lines 13629 13921 +292
===================================================
+ Hits 10182 10355 +173
- Misses 3437 3556 +119
Partials 10 10
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
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
1 participant
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.
Intent: why does this change exist?
An existing deployment has records sitting on the old per-destination internal topics. Turning the delivery pool on needs a way to move that backlog across without losing any of it.
System impact: what's affected, including downstream?
A standalone entrypoint plus a drainer, run on demand during cutover with the populator and the queue processors stopped, never as part of the service. The drain consumes under a throwaway group, while a short-lived client configured with each old group id reads that group's committed offsets to find the start point: it only ever calls committed(), so it never joins the group or triggers a rebalance, and both clients run with auto-commit and auto-offset-store disabled. No offset is written anywhere, which is what keeps a rollback to the old pipeline possible.
Note for review:
extensions/notification/utils/deliveryKey.jsis a deliberately identical copy of the file added by the populator PR, so that both halves address records the same way. The blobs are byte-identical, which is what lets the two branches merge without conflict.Note for review: this branch does not add
deliveryPoolorspreadFactorto the notification config schema, the populator PR (#2799) does. Config validation rejects unknown keys, so until that one merges, a config carrying adeliveryPoolblock will not load and this tool has nothing to point at. Merge order matters.Preserved behavior: what explicitly stays the same?
Nothing in the running pipeline changes. The tool is opt-in, and with respect to existing consumer group offsets it is strictly read-only.
Intended change: what's different after this PR?
Adds a replay entrypoint that re-publishes records from the old internal topics onto the delivery topic, addressed exactly as the populator addresses them, so the delivery workers cannot tell a replayed record from a live one. Old records carry no configurationId, so each one is rematched against the bucket's current notification configuration to recover it, which means a configuration removed since the original event drops the record as unmatched. Rerunning is safe and produces duplicates rather than gaps: any error aborts the drain instead of stepping over a record.
Verification: how do we know this worked, or how would we know if it didn't?
Unit tests cover the drainer: offset math at the edges (no committed offset, a committed offset dropped by retention, a partition the old group already finished), the rematch and skip paths, key encoding, and the batch acking that gates the next read. The key helper has its own tests in the populator PR and is exercised here through the drainer's key assertions rather than a second copy of that spec. The two copies of the helper were confirmed byte-identical by hash rather than by eye.