EventStore / EventStore

EventStoreDB, the event-native database. Designed for Event Sourcing, Event-Driven, and Microservices architectures
https://eventstore.com
Other
5.32k stars 643 forks source link

Settings For GRPC Client #2119

Closed thefringeninja closed 4 years ago

thefringeninja commented 4 years ago

What kind of configurable options do we want for the GRPC Client? I can think of some so far:

ahjohannessen commented 4 years ago

Retry when you subscribe to a stream that does not exist. Exponential back off? Or should we push the responsibility to the caller?

I think it would be nice from a client perspective to subscribe to a stream that yet has no events, e.g. having the server allow that. We have reactors / projectors that subscribe to category / singleton streams that might produce events at some time in future. Using backoff here, e.g. start at 100ms and back off exponentially up to 10 min then retry, would be a bit unfortunate in some cases wrt. view updating lag, this of course depends on backoff strategy. Hammering the server each 100 ms solves that, but not very smart when the server could be instructed to notify even if the stream is not found.

ahjohannessen commented 4 years ago

With respect to cluster settings, I think the following is worth a read:

At the very minimum I would expect that one configures a balancer in the grpc library used, in my case grpc-java. I am not sure how well things are supported in dotnet: https://github.com/grpc/grpc-dotnet/issues/521

However, it seems that some sort of *.proto definition for gossip or whatever that clients could use for balancing might be a step in the right direction.

jen20 commented 4 years ago

@ahjohannessen We do need to do something here for cluster discovery, however we explicitly never want per-call load balancing.

ahjohannessen commented 4 years ago

@jen20 I suppose that depends on balancer / policy? I think in grpc-java there are at least round-robin with stickiness and pick-first built-in.

I am thinking of doing a simple one that uses existing http gossip and updates the address group for sub-channels with master first in list, using a pick-first policy. I wish to piggyback as much on grpc-java load balancer machinery as possible.

however we explicitly never want per-call load balancing

The docs state the following in the intro:

It is worth noting that load-balancing within gRPC happens on a per-call basis, not a per-connection basis. In other words, even if all requests come from a single client, we still want them to be load-balanced across all servers.

However, I suppose that is all up to the strategy/balancer approach one uses on the client, e.g. round-robin, random, pick-first, sticky, or some semi smart one that uses a combination of pick-first and random, i.e. pick-first when master is needed (writes / deletes) and otherwise random for subscription / reading.

Anyway, I think it would be nice if servers could expose cluster status via grpc streams instead of relying on polling on http for gossip.

jen20 commented 4 years ago

It doesn't depend on either the load balancer or the policy. The optimal place to make write requests is always the primary node, and this may also be required for some types of reads and subscriptions. We never want round-robin load balancing on a per-call basis.

I think ultimately we will need to expose the cluster information over the gRPC interface, but that's probably out of scope for the second preview.

ahjohannessen commented 4 years ago

It doesn't depend on either the load balancer or the policy. The optimal place to make write requests is always the primary node, and this may also be required for some types of reads and subscriptions. We never want round-robin load balancing on a per-call basis.

I suppose I should pursue a pick-first balancer with best candidate first in the list of addresses to pick from.

this may also be required for some types of reads and subscriptions

What are the types of reads and subscriptions that require master and which do not?

I think ultimately we will need to expose the cluster information over the gRPC interface, but that's probably out of scope for the second preview.

I think it would be great to have some concrete guidelines soon such that I can start on that part :)

ahjohannessen commented 4 years ago

@jageall Why is this closed?

pgermishuys commented 4 years ago

I suspect this might have been automated @ahjohannessen. There's still work required for this issue to be closed. Re-opening it.

mat-mcloughlin commented 4 years ago

It seems if you link a pull request it will close the issue if there are no other open PR's against it