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

LoadBalancer API #77

Open
elandau opened this issue Feb 24, 2015 · 2 comments
Open

LoadBalancer API #77

elandau opened this issue Feb 24, 2015 · 2 comments

Comments

@elandau
Copy link
Contributor

elandau commented Feb 24, 2015

We've gone back and forth on various public API options for the load balancer. This issue shall serve as a placeholder for the discussion on which API is the most robust and semantically correct.

Iterator

Getting a client from the load balancer fits naturally with the Iterator interface since it follows the pull model. However, since the load balancer list of hosts may be updated asynchronously the Iterator API can easily be violated. hasNext() may return true but the list may be empty by the time next() is called.

public Observable<Response> submit(final Request request)  {
    return Observable.from(lb).flatMap(new Func1<Client, Observable<Response>>() {
        Observable<Response> call(Client c) {
            return c.submit(request);
        }
    }).single();
}
Observable

Implementing the LoadBalancer as an Observable give great flexibility for managing various states and error conditions.

public Observable<Response> submit(final Request request) {
    return lb.flatMap(new Func1<Client, Observable<Response>>() {
        Observable<Response> call(Client c) {
            return c.submit(request);
        }
    });
}
Func0

Using Func0 reduces the API to a simple method call that returns the next client or null if no client is available. Since an Observable must be created to tie the client selection with request execution triggered by a subscription to the Observable we may as well just implement the LoadBalancer as an Observable in the first place.

public Observable<Response> submit(final Request request) {
    return Observable.create(new OnSubscribe<Client>() {
        public void call(Subscriber<Client> s) {
            ObClient c = lb.call();
            if (c == null) {
                s.onError(new NoSuchElementException());
            }
            else {
                s.onNext(c);
                s.onCompleted();
            }
        }
    }).flatMap(new Func1<Client, Observable<Response>>() {
        Observable<Response> call(Client c) {
            return c.submit(request);
        }
    });
}
@NiteshKant
Copy link
Contributor

Thanks @elandau for creating this issue, we need to nail down the contract :)

hasNext() may return true but the list may be empty by the time next() is called.

Is it true that the Load balancer works on immutable list of instances and for every change the list is swapped with a new list?
If so, then this should not be an issue, correct w.r.t hasNext() not being consistent with next()? As a worst case, one would work off an older list in the scope of a single request. If it is over two requests, then one should anyways do a fresh call to the LB.

Implementing the LoadBalancer as an Observable give great flexibility for managing various states and error conditions.

Can you explain this more as compared to an Iterable?

Func0

This is much like an Iterable of 1 semantics. In this case, would a backup request, do multiple calls to the Func0 till it receives a distinct item?

Since an Observable must be created to tie the client selection with request execution triggered by a subscription to the Observable we may as well just implement the LoadBalancer as an Observable in the first place.

I think its true that we will use it with an Observable at the end, but not sure whether that requires this API to be Observable based for the primary reason that the interaction is always pull-based.

@elandau
Copy link
Contributor Author

elandau commented Feb 24, 2015

The worst case for the iterator is the scenario where hasNext() returns true but the list is set to empty() before next() is called. next() will end up throwing a NoSuchElementException. To me this sounds like a violation of the contract for iterator. For this to work correctly hasNext() must always return true and we will need to expect a NoSuchElementException from next().

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

2 participants