-
Notifications
You must be signed in to change notification settings - Fork 128
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Multi split operator #1309
Multi split operator #1309
Conversation
Fixes #1278
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #1309 +/- ##
============================================
- Coverage 89.84% 89.83% -0.02%
- Complexity 3340 3357 +17
============================================
Files 458 459 +1
Lines 13215 13327 +112
Branches 1639 1656 +17
============================================
+ Hits 11873 11972 +99
- Misses 695 698 +3
- Partials 647 657 +10
|
@ozangunalp we need to discuss how reactive messaging can leverage this new feature. |
Note that the back-pressure management is simple (1-by-1 requests as long as demand + quorum are met). We could do something more clever by snapshotting the minimal demand among current subscribers, but that'd lead to more complex code, not counting what happens when a split subscriber leaves, etc. |
I am thinking that it'd be useful to expose the key type from |
On a different note, during my tests I find that request propagation is trailing by one. But I need to write a proper test to describe the issue. |
Yes we can do that |
@ozangunalp I've added an interface, it's similar to a grouped Multi |
@jponge That's also useful, but I meant to be able to get the key type (so the enum class of the key) from the |
Ok I can add a method for that as well |
@ozangunalp Note that you can use https://docs.oracle.com/en/java/javase/11/docs/api/java.base/java/lang/Enum.html#getDeclaringClass() from a key |
Sure but that requires having a key to begin with :) To give a more visual example, I am willing to relax the single outgoing target requirement in Reactive Messsaging in order to enable the following usage : @ApplicationScoped
public static class SplitterMulti {
enum Caps {
ALL_CAPS,
ALL_LOW,
MIXED
}
@Incoming("in")
@Outgoing("sink1")
@Outgoing("sink2")
@Outgoing("sink3")
public MultiSplitter<String, Caps> reshape(Multi<String> in) {
return in.split(Caps.class, s -> {
if (Objects.equals(s, s.toLowerCase())) {
return Caps.ALL_LOW;
} else if (Objects.equals(s, s.toUpperCase())) {
return Caps.ALL_CAPS;
} else {
return Caps.MIXED;
}
});
}
} At wiring time the framework verifies the number of splits and the number of outgoing targets and wires them in the order of declaration, ALL_CAPS -> sink1, ALL_LOW -> sink2, MIXED -> sink3 @cescoffier wdyt ? |
Ah ok, so you need that on In that case is having access to the key in the split any useful? |
Yes!
Not really sure, I only see that would be useful if you pass the |
That's more or less what I had in mind. I'm a bit worried about the outgoing order (enum ordering can be funny). |
This reverts commit f307084.
There you go, I have:
|
@ozangunalp all good to merge or do you require more time for investigations on the RM side? |
@jponge Thanks, all good for me for Mutiny! I'd need more time to finish the multiple outgoings support in RM. |
Thanks! |
Fixes #1278