broker: assign event seqs in the broker not overlay #7221
Merged
+225
−243
Conversation
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
Problem: flux-framework#7109 moved all event publishing from the broker to the overlay subsystem so that the peer multicast portion could eventually run in separate thread. Unfortunately, moving all event publication machinery to overlay creates an unnecessary requirement that overlay be running on a size=1 instance, and will complicate 'flux module reload overlay', should that be useful. Leave the resource intensive event mcast code in overlay but move the rest of it back into the broker. TL;DR Recall there are two ways that event messages are routed per RFC 3: 1) A bare event message is forwarded upstream on the TBON and published when it arrives on rank 0. 2) An event message is base64 encoded and encapsulated in a request message that is sent to rank 0, where it is published. Publication occurs only on rank 0 and consists of assigning a monotonically increasing sequence number, distributing the message to local broker and module subscribers, and sending the event message to the overlay via the interthread message channel for further distribution. The overlay now routes events as follows: - If received from the local broker on the interthread channel: On rank 0, messages are mcast to all children. On rank > 0, messages are forwarded to the parent for publication. - If received from an overlay child: On rank 0, messages are forwarded to the local broker for publication. On rank > 0, message are forwarded to the parent for publication - If received from the overlay parent (rank > 0), messages are mcast to all children AND sent to the local broker on the interthread channel for distribution to local broker and module subscribers. Update the overlay unit test that were exercising full event publication. Update some event sharness tests that used the other RPC name.
garlick
force-pushed
the
event_publish
branch
from
aace27d to
b504473
Compare
Member
Author
|
Rebased on current master. |
Member
Author
|
Thanks I'll set MWP |
Contributor
Merge Queue Status✅ The pull request has been merged at b504473 This pull request spent 6 seconds in the queue, with no time running CI. Required conditions to merge
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #7221 +/- ##
==========================================
- Coverage 83.66% 83.66% -0.01%
==========================================
Files 554 554
Lines 92625 92646 +21
==========================================
+ Hits 77496 77509 +13
- Misses 15129 15137 +8
🚀 New features to boost your workflow:
|
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.
Problem: #7109 moved all event publishing from the broker to the overlay subsystem so that the peer multicast portion could eventually run in separate thread. Unfortunately, moving all event publication machinery to overlay creates an unnecessary requirement that overlay be running on a size=1 instance, and will complicate
flux module reload overlay, should that be useful.Leave the resource intensive event mcast code in overlay but move the rest of it back into the broker.
TL;DR
Recall there are two ways that event messages are routed per RFC 3:
A bare event message is forwarded upstream on the TBON and published when it arrives on rank 0.
An event message is base64 encoded and encapsulated in a request message that is sent to rank 0, where it is published.
Publication occurs only on rank 0 and consists of assigning a monotonically increasing sequence number, distributing the message to local broker and module subscribers, and sending the event message to the overlay via the interthread message channel for further distribution.
The overlay now routes events as follows:
If received from the local broker on the interthread channel: On rank 0, messages are mcast to all children. On rank > 0, messages are forwarded to the parent for publication.
If received from an overlay child: On rank 0, messages are forwarded to the local broker for publication. On rank > 0, message are forwarded to the parent for publication
If received from the overlay parent (rank > 0), messages are mcast to all children AND sent to the local broker on the interthread channel for distribution to local broker and module subscribers.
Update the overlay unit test that were exercising full event publication. Update some event sharness tests that used the other RPC name.