Skip to content
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

Rule §2.12 prevents some optimizations? #290

Closed
akarnokd opened this issue Aug 27, 2015 · 20 comments
Closed

Rule §2.12 prevents some optimizations? #290

akarnokd opened this issue Aug 27, 2015 · 20 comments

Comments

@akarnokd
Copy link
Contributor

I'd like to implement a publisher which resubscribes to a source publisher a number of times (unless an error happens in one of those runs). The optimization is to only have a single instance of the mediator Subscriber which is always resubscribed (trampolined to prevent SO) to the source when the previous subscription fires onComplete. However, §2.12 forbids this reuse and if the source enforces this in some way (i.e., storing the current set of subscribers in a thread-safe set and removing the completed one after the call to onComplete) the repeat can't work.

Here is the code of the publisher I'm talking about.

public final class PublisherRepeat<T> implements Publisher<T> {
    final Publisher<? extends T> source;
    final long count;
    public PublisherRepeat(Publisher<? extends T> source, long count) {
        this.source = source;
        this.count = count;
    }

    @Override
    public void subscribe(Subscriber<? super T> s) {
        SubscriptionArbiter sa = new SubscriptionArbiter();
        s.onSubscribe(sa);

        RepeatSubscriber<T> rs = new RepeatSubscriber<>(s, count, sa, source);
        rs.subscribeNext();
    }

    static final class RepeatSubscriber<T> extends AtomicInteger implements Subscriber<T> {
        /** */
        private static final long serialVersionUID = -7098360935104053232L;

        final Subscriber<? super T> actual;
        final SubscriptionArbiter sa;
        final Publisher<? extends T> source;
        long remaining;
        public RepeatSubscriber(Subscriber<? super T> actual, long count, 
                SubscriptionArbiter sa, Publisher<? extends T> source) {
            this.actual = actual;
            this.sa = sa;
            this.source = source;
            this.remaining = count;
        }

        @Override
        public void onSubscribe(Subscription s) {
            sa.setSubscription(s);
        }

        @Override
        public void onNext(T t) {
            actual.onNext(t);
        }
        @Override
        public void onError(Throwable t) {
            actual.onError(t);
        }

        @Override
        public void onComplete() {
            long r = remaining;
            if (r != Long.MAX_VALUE) {
                remaining = r - 1;
            }
            if (r != 0L) {
                subscribeNext();
            } else {
                actual.onComplete();
            }
        }

        /**
         * Subscribes to the source again via trampolining.
         */
        void subscribeNext() {
            if (getAndIncrement() == 0) {
                int missed = 1;
                for (;;) {
                    source.subscribe(this);

                    missed = addAndGet(-missed);
                    if (missed == 0) {
                        break;
                    }
                }
            }
        }
    }
}

(SubscriptionArbiter makes sure any unfulfilled request is re-requested from the Publisher in the next turn.)

Therefore, this object equality requirement gives me trouble. Do I interpret §2.12 correctly or rule §2.4 and/or §1.6 actually allows this, i.e., reuse is allowed the moment the publisher is about to call onError or onComplete?

(This also affects operators such as concat and retry where the terminal event triggers a new subscription with the same mediator Subscriber).

@viktorklang
Copy link
Contributor

Hi @akarnokd,

I have no idea how I managed to miss this Issue. I am deeply sorry.

I am not 100% sure I understand what you want to achieve, but it is important to remember that the spec applies to generic Publishers, Subscribers, Subscriptions and Processors.

If you control the Publisher AND Subscriber implementation, you can in theory do what you want, as long as an arbitrary Subscriber which is given to your Publisher gets the RS spec treatment and vice versa.

@reactive-streams/contributors Would be interesting to get your takes on this too.

@akarnokd
Copy link
Contributor Author

The problem arises because I don't control the sources where I want to subscribe the same, thread-safe, Subscriber of "mine".

If it were only RxJava 2.x+ and Project Reactor 2.5+, I am certain this reuse doesn't cause problems because neither of them cares. That leaves us with Akka-Streams as the major player in the field.

@viktorklang
Copy link
Contributor

Hmmm, perhaps I was unclear: you are free to treat implementation that you are sure will be fine with it. The spec applies to generic/arbitrary implementations of the RS interfaces which conform to the spec and TCK. Doesn't that answer your question?
So if you do a typecheck inside publish(s) and see that the implementation is fine with other behavior then that is fine to "exploit". The RS spec and TCK governs the valid interactions of arbitrary instances of the RS interfaces. Does that make sense?

@smaldini
Copy link
Contributor

Type checking as in instanceof seems too specific for at least 2 major implementors. But I'm fine with not solving this immediately, there are other issues that might limit our protocol in general which we need to focus on especially the onErroring on request.

@smaldini
Copy link
Contributor

We will want to loosen some TCK rules around reference checking at least to avoid us constantly commenting out this rule in the reactor tests.

@viktorklang
Copy link
Contributor

@smaldini If your Subscriber instance allows 2:12 to be violated then that is fine. The rule is there to make sure that a Publisher does not assume that an arbitrary Subscriber supports multisubscription. Also, the rule does not require a Publisher to keep track of all Subscribers it has ever Subscribed. :)

@viktorklang
Copy link
Contributor

@smaldini I think we should definitely sit down and write down the intents behind the rules, I've been too busy with other stuff but it seems that a lot of progress could be made if we had those in place. What do you think?

@smaldini
Copy link
Contributor

@viktorklang Don't worry mate I am to blame too for the same reasons as you expect :) But it's not a bad timing, we have all endured and matured for a year now we can do these tweaks. The positive outcome is still here, the contract interfaces while simple are a game changer, focused enough. Then it's all about smoothing corner cases and updating the fantastic TCK job @ktoso pushed. It's our duty to all !
I'm away for the next 2 weeks but we should fix an ETA for 1.1.M1 by end of April to pressure us for triage and discussion end of March early April, wdyt ? /cc @reactive-streams/contributors

@viktorklang
Copy link
Contributor

@smaldini Let's pick it up when you're back! Ping us here :)

@viktorklang
Copy link
Contributor

@smaldini Ping

@DougLea
Copy link
Contributor

DougLea commented Apr 7, 2016

In early versions of spec, I suggested making re-subscribe a no-op, but there was an objection (I think by @rkuhn) that I don't remember and can't easily find. Even if spec is unchanged, it would be good to find or reconstruct this argument.

@viktorklang
Copy link
Contributor

From @akarnokd:

However, §2.12 forbids this reuse and if the source enforces this in some way (i.e., storing the current set of subscribers in a thread-safe set and removing the completed one after the call to onComplete) the repeat can't work.

The RS rules are there for generic Subscribers to be valid incarnations of the RS spec.
In your case, if you know that your Subscriber can handle resubscriptions then you are free to resubscribe, what you cannot assume is a generic Subscriber being able to deal with resubscriptions.
Does that clarify your question?

@DougLea @rkuhn If someone can find and reconstruct it, it would be most helpful indeed.

@djodjoni
Copy link

@viktorklang are Processors considered as "a generic Subscriber" as if 2.12 combines with 4.2:

A Processor MAY choose to recover an onError signal. If it chooses to do so, it MUST consider the Subscription cancelled, otherwise it MUST propagate the onError signal to its Subscribers immediately.

makes Processor useless.

@viktorklang
Copy link
Contributor

Hi Kalin,

I'm not sure I follow, could you elaborate what you mean?

Cheers,

On Sep 18, 2016 11:18, "Kalin Maldzhanski" [email protected] wrote:

@viktorklang https://github.com/viktorklang are Processors considered
as "a generic Subscriber" as if 2.12 combines with 4.2:

A Processor MAY choose to recover an onError signal. If it chooses to do
so, it MUST consider the Subscription cancelled, otherwise it MUST
propagate the onError signal to its Subscribers immediately.

makes Processor useless.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#290 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAAqdx574-JVpkMQriXeztNYaGNQt-Zoks5qrQHngaJpZM4FzRap
.

@djodjoni
Copy link

So as I understand 2.12 re-subscribe is not possible.
Subscriber normally subscribes then it may decide to go away and cancels it subscription. So everything is OK as it is aware of all that.

Processor however routes events from the upstream to its subscribers. In some scenarios upstream Publisher needs to be changed or resubscribed then the processor needs to re-initialize and also all its subscribers.

In the case of 4.2 if a Processor chooses to recover from onError signal a natural continuation would be to resubscribe and continue to provide events to its subscribers otherwise it would simply pass on the OnError.

As you mentioned anybody can do anything with their subscribers however:

... what you cannot assume is a generic Subscriber being able to deal with resubscriptions ...

So is the default implementation of Processors considered as generic Subscriber which shall not allow re-subscription? What shall we expect from those? Currently RxJava 2.x Processors and part of Reactor3 ones allow this.

Thx!

@rkuhn
Copy link
Member

rkuhn commented Sep 18, 2016

Recovery does not necessarily mean to subscribe to another Publisher, it can also mean to just inject some elements of its own and then complete. This is still recovery because it stops the failure from tearing down the (assumed) whole chain of Processors.

Another thought is that once a Publisher has failed, why would a Processor subscribe to that same Publisher again? It has failed, after all. It would be more plausible to procure a new Publisher to subscribe to, which is not forbidden AFAICT.

@djodjoni
Copy link

true. so it is not forbidden to subscribe to another publisher after the current subscription is considered cancelled? This was not that obvious to me.

@rkuhn
Copy link
Member

rkuhn commented Sep 19, 2016 via email

@viktorklang
Copy link
Contributor

Sorry for my slow response rate here, I'm in the middle of a cold; a
cross-country relocation; and Email Hell™.

I think/suspect most of the questions like these could be solved by having
that "Comment"/"Intent" section per rule, I just have to find some time to
actually get started on that. But in case someone else has more time
currently than I do–please start that work :)

On Mon, Sep 19, 2016 at 9:06 AM, Roland Kuhn [email protected]
wrote:

I spoke too soon: 2.12 forbids calling onSubscribe again, which expresses
that even though different Publishers would be unable to prevent such reuse
Subscribers cannot be assumed to be reusable. If you were to write such a
Processor that resubscribes itself then that feature would be special to
your implementation and not usable by external code that is governed by the
spec.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#290 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAAqd8bJlcsKMguXs0a2tgVXWbhRRsNLks5qrjR3gaJpZM4FzRap
.

Cheers,

@viktorklang
Copy link
Contributor

Closing. Generic Subscribers cannot be assumed to be reusable: i.e. if all you know is that you have a Subsriber, you cannot, by the spec, be allowed to resubscribe it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants