Netflix / ocelli

Apache License 2.0
53 stars 28 forks source link

LoadBalancer API #77

Open elandau opened 9 years ago

elandau commented 9 years ago

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 commented 9 years ago

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 commented 9 years ago

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().