search

envoyproxy / java-control-plane

Java implementation of an Envoy gRPC control plane
Apache License 2.0
293 stars 136 forks source link

cache: respond to watches in parallel #75

Closed snowp closed 5 years ago

snowp commented 6 years ago

This responds to watches in parallel by executing the watch callbacks on a worker thread scoped to each group. By using one thread per group, we maintain a strict response order while while avoiding holding the lock while issuing gRPC responses.

Fixes #59

Signed-off-by: Snow Pettersen snowp@squareup.com

snowp commented 6 years ago

This causes flaky tests at the moment, I'll try to fix the tests

sschepens commented 5 years ago

@snowp how about reviving this? Do we really need an executor per group? isn't it only necessary to mantain order on a given connection?

snowp commented 5 years ago

@sschepens I haven't had time to work on this, but I definitely still want something like this. I agree that one executor per group is probably overkill, but I haven't figured out exactly what kind of concurrency we want here. Maybe just a single executor is sufficient here?

Yes, for ADS we only need to maintain order on the given connection, so another approach would be to maintain an executor per stream.

If you're interested in this feel free to pick it up, otherwise I'll try to get to it when I have free cycles, but I don't think that will be anytime soon.

sschepens commented 5 years ago

@snowp we could probably use something like netty does, binding channels to executors, we could have a pool of executors, dependent on the amount of cpus and then just round robin streams to those executors. This could probably work, the only concern i have is that you could have streams that are more demanding than others so you could have unbalanced executors, but i don't know if it would be that concerning.

Let me know if you're ok with this, I could probably have a go at it.

joeyb commented 5 years ago

Closing this since #85 was merged.