-
Notifications
You must be signed in to change notification settings - Fork 534
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
Explain the goal of each rule in the spec #177
Comments
I think this is a very good idea. |
Possibly relates to #145 implementor notes, could be put in there? |
@ktoso it definitely relates to implementor notes, but I'd like to keep it adjacent to the actual rules themselves, to make sure that it's kept up to date. |
@viktorklang Happy to contribute in my small way (spelling and whitespace a speciality). So the location of these explanations is in the notes [1],[2], etc at the bottom of each section Publisher, Subscriber, Subscription. Is that so? |
@davidmoten I imagined a new column (to the right) with the explanation of the rule, keeping the bits together, so to say. |
@viktorklang Just read #283 and was reminded of this little task.
I knocked up a gist demoing options using markdown. https://gist.github.com/davidmoten/03ca6332e416e050a4a6 Given the formatting limitiations of markdown I'd suggest adding explanations as an extra row rather than in a column. |
The 3rd option in your gist looks pretty good actually (notes under rule)! |
Thanks @davidmoten! |
This would be immensely useful. I got here through Java 9's corresponding Flow API, but don't have any experience with implementations such as Reactor, RxJava or Akka. So for people like me, this would be a great addition. What's the preferred way to suggest explanations to be added? I have a few more questions concerning the spec/README. Can I add them as a comment here, or is it preferred I file new issues for these (maybe with a specific label, since they're really just questions to get a better understanding of the spec)? |
Ask away! |
Thanks for the invitation @akarnokd. Here goes:
What does it mean for a Subscriber to be "valid to the Publisher"?
I'm not sure I understand this. From the mentions in the example AsyncSubscriber, I'd guess it means that a Subscriber MAY asynchronously process its signals, but MUST process them one at a time, with a happens-before relationship between the "end of processing signal N" and the "start of processing signal N+1". Is this correct?
Is it allowed for an onXxx method to throw an Error (e.g. OutOfMemoryError)? The comment of @smaldini in #292 suggests to me it is, but I was unable to find this mention of "non-fatal" in the spec. Moreover, the examples always catch Throwable, suggesting that it is indeed not allowed. Also, there is a note saying
but it's unclear to me if "exceptions" refers to java.lang.Exception or java.lang.Throwable.
When is something "adequate for the runtime environment"? I assume it's always adequate to simply not handle such exceptions at all, right?
What does "inside of its Subscriber context" mean here? For example, say I have a Subscriber which passes its Subscription to an instance of class X, which does nothing other than invoking the Subscription's cancel() method after a certain time. Would this be illegal, i.e. is X considered to be outside of the Subscription's Subscriber context? In the section "Asynchronous vs Synchronous Processing", the first sentence says:
However, there's no occurrence in the specification of the term "block" and in the rules about responsivity (2.3, 3.4, 3.5), only 3.5 says Subscription.cancel MUST respect the responsivity of its caller, while the others use RECOMMENDED/SHOULD. The section "Asynchronous vs Synchronous Processing" also mentions:
Why does it say push-based here, and not pull-based? Since the Subscriber explicitly requests items, I'd say this is a pull-based mechanism, no? What am I missing here? |
§2.6: That sentence doesn't make any sense to me. §2.11: Not sure, it probably means the need for barriers before going async with the supplied parameter to onXXX. In practice, we had to make sure there was no reordering in the sequence over an async boundary. §2.13: indeed, the spec is vague on this one. In practice, we had to make exceptions to some Exceptions and propagate them even if catched as a Throwable. A great candidate to fix in the next version of the spec. §2.13: That's an escape hatch in the spec. The example prints the stacktrace to the console in case there is an exception that can't be delivered (including fatal ones). In practice, we have something like a catch-all sink for undeliverable exceptions where a global handler can get them anyway. Dropping them is an option but one could miss subtle errors. §3.1: Not sure what it meant. In practice, exposing a Subscription outside its target Subscriber can lead to race condition with the Subscriber's own logic. First, the Subscription itself may arrive late and thus you can't cancel it or request values from it upfront. Second, issuing extra request could overflow the Subscriber's logic or cancelling the upstream may leave the Subscriber hang and leaking resources. "not block": sounds a bit harsh. It is supposed to indicate that one should avoid sleeping, waiting on an object or condition, because those can be written in a notification-like (i.e., push) fashion. "push-based": the fact that the Subscriber request values doesn't mean that those values are immediately available. In a pull-based API, like Iterable, when you call next(), it can't return saying "sorry, no value yet but may come later"; it has to return a valid value or throw NoSuchElementException but still has to wait for the next value to become available in some way (i.e., waiting on a blocking queue). This comes from the extended Observable-Iterable duality. In Iterable terms, you'd have an available() method somewhere that tells how many times you could call onNext() without blocking (see InputStream). |
Thanks @akarnokd!
I'm afraid I don't understand this explanation. What does it mean to "go async" with the parameter? |
How about creating corresponding Issues per Rule paired with questions, then we can refine the explanation in the Issue and create PRs from them? |
§2.6: If the "downstream" completes or errors before the "upstream". Imagine for instance a Processor which only wants the first element from its "upstream". Or a Subscriber which receives a "forbidden" value. etc. The intent of this rule is to make sure that Publishers will be able to do resource cleanup when they are no longer needed. §2.11: This Rule intends to ensure that safe publication of Subscriber signals are the responsibility of the Subscriber. §2.13: The intent of the rule is to make sure nobody uses Exception throwing as signalling of anything but fatal errors, where fatal is to be considered failure conditions who are not recoverable and require program termination. As for exceptions and errors, please see this first sentence of the java.lang.Error javadoc: "An Error is a subclass of Throwable that indicates serious problems that a reasonable application should not try to catch." As for "raise this error condition in a fashion that is adequate for the runtime environment." means that instead of rethrowing the exception it should use whatever mechanisms provided to it, this may include things like logging. §3.1: See §2.7 for more background. Linking to §2.7 from §3.1 seems like a good idea. As for:
"block" may not have been the best word, and it should most definitely be explained, but the intent is that a Subscriber should not impede the progress of the Publisher from an execution PoV. I.e. the Subscriber should not starve the Publisher from CPU cycles. As for:
Good catch, I think this should say "dynamic push-pull stream" |
Notice how the methods on subscriber is referred to as "signals". This means that invoking the onX methods (signalling something) is divorced from the processing of said signal. Hence that signal can either be processed synchronously, or asynchronously. |
I've just created issue #321 for §2.6 I will create analog issues for other things that came up in the above comments. If you'd like me to change the title/labels or something, just let me know. |
@anthonyvdotbe That's perfect, thank you! |
Apologies for the delay here, fixed in #339 |
I think it could provide a lot of value to readers of the spec if every rule in it was annotated with what the rule is there to achieve/prevent.
Thoughts?
The text was updated successfully, but these errors were encountered: