S3C-11127: publish addressed notification entries behind a delivery pool flag - #2799
Draft
anurag4DSB wants to merge 3 commits into
Draft
S3C-11127: publish addressed notification entries behind a delivery pool flag#2799anurag4DSB wants to merge 3 commits into
anurag4DSB wants to merge 3 commits into
Conversation
The key maps a destination and an object to a stable record key, so the same object always lands on the same partition of the delivery topic. A destination with a spread factor above 1 is split over that many keys to let several delivery workers share it.
Adds the deliveryPool block, disabled by default, and the per-destination spreadFactor. topic and groupId are only required once the pool is enabled, so existing configurations keep validating unchanged.
Splits the destination fan-out in two: the existing path, moved as is, and a new one that publishes one record per matching destination on the shared delivery topic, carrying destinationId and configurationId in the payload instead of encoding the destination in the topic. With the pool disabled the populator behaves exactly as before.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files
... and 3 files with indirect coverage changes
@@ Coverage Diff @@
## development/9.3 #2799 +/- ##
===================================================
- Coverage 74.70% 74.53% -0.18%
===================================================
Files 199 200 +1
Lines 13629 13658 +29
===================================================
- Hits 10182 10180 -2
- Misses 3437 3468 +31
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
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?
Today every notification destination runs its own consumer group over a shared topic, so N destinations each read every message and we pay N times the reads. This is the populator half of a POC that addresses each record to its destination instead, so a single shared pool can deliver for all of them.
System impact: what's affected, including downstream?
Only the notification populator, and only when the new
extensions.notification.deliveryPool.enabledflag is on. It adds one config block, a per-destinationspreadFactor, and a small key helper that the replay tool will carry an identical copy of. Nothing consumes the new delivery topic yet; the worker arrives in a separate PR.Preserved behavior: what explicitly stays the same?
With the flag off, which is the default, the populator behaves exactly as before. The existing fan-out moved verbatim into
_publishLegacyEntries, including apushedToTopicmap that is declared as a Map but used with bracket access, deliberately left alone so flag-off stays byte-identical.Intended change: what's different after this PR?
With the flag on, the populator publishes one record per matching destination onto a single delivery topic, carrying
destinationIdandconfigurationIdin the payload, keyed so the same destination and object always land on the same partition.Worth knowing before you read
spreadFactor: it buys you AT MOST that many parallel lanes, never exactly that many. Sub-keys collide under the partitioner, so on a 4-partition topic a spreadFactor of 4 yields exactly 2 lanes for every possible destination name. Sizing guidance is in the POC design notes.Verification: how do we know this worked, or how would we know if it didn't?
Unit suite green, 182 in
tests/unit/notification, with the pre-existing assertions untouched. The flag-off claim is checked mechanically rather than by eye: the moved block diffs clean against the original once the indentation shift is normalized. Beyond unit tests, the real publish path ran against a live Kafka broker, confirming the wire keys, per-destination routing and partition stability across processes.