Affinitize server call notification #6612
Conversation
…s makes writing certain test cases (like hybrid_end2end tests) easier
listening completion queue (i.e frequently polled)
flag to be set to true on the pollset in case of 'poll' strategy. To fix this I am calling grpc_pollset_work with a 0 timeout right after adding the fds)
|
Ok... polling problem fixed. The final thing is that we need to keep track of non-listening cqs for real, so that they can work-steal even if they're not listening. |
| ResetStub(); | ||
| std::thread generic_handler_thread([this, &generic_service] { | ||
| gpr_log(GPR_DEBUG, "t0 start"); |
There was a problem hiding this comment.
delete the debug lines (513, 515, 519, 521, 525, 527)
|
Flakes seem unrelated (sanity looks like misconfiguration, tsan is known) |
|
Seems ready pending a last check through |
| @@ -207,6 +207,9 @@ class HybridEnd2endTest : public ::testing::Test { | |||
| ServerBuilder builder; | |||
| builder.AddListeningPort(server_address_.str(), | |||
| grpc::InsecureServerCredentials()); | |||
| // Always add a sync unimplemented service: we rely on having at least one | |||
| // synchronous method to get a listening cq | |||
| builder.RegisterService(&unimplemented_service_); | |||
There was a problem hiding this comment.
Do we really need this to get a listening cq ? - the hybrid server already has a "sync part" and that should give us a listening cq anyway.
On a related note: I thought that these tests might have been previously failing because non-listening completion queues were not being added to server->cqs and hence weren't getting picked up for queuing completed events. I see that you fixed that (i.e add non-listening cqs to server-cqs and don't start a listener in grpc_server_start function on those pollsets)
So shouldn't that fix take care of the hybrid_end2end test failures ?
There was a problem hiding this comment.
We've at least one test in the suite that overrides all methods as async or generic, meaning there's no sync part - meaning there's no sync listener (and that test needs one).
We could either split out some code for that specific test (which one is escaping me at the minute), or live with this to ensure that there's a sync listener around.
|
LGTM. I just had one comment/question in the hybrid_end2end tests but that is not blocking my LGTM |
Server was continuing to make requests for new calls forever, which were starving out the shutdown sequence. Change order and win.
Takes #6149 and extends it to also keep call request queue affinity with the underlying channel.
Improves throughput benchmarks somewhere between 20-90% (finally we see the benefit from the initial work!)
Some refactoring left to get tests building, and no doubt some bugs left.
Also built on #6603, #6585