shotover/shotover-proxy

Port KafkaSinkSingle to use try_recv instead of recv

Opened this issue · 0 comments

prereq: #1609

This is part of #1507.
Specifically the checkbox swap single sinks to use SinkConnection::try_recv instead of SinkConnection::recv

Kafka has two sink transforms: KafkaSinkSingle and KafkaSinkCluster
KafkaSinkCluster has already been ported to use SinkConnection::try_recv instead of SinkConnection::recv.
But we still need to port KafkaSinkSingle which is the target of this issue.

  • SinkConnection::recv is an async method that asynchronously waits until at least 1 new response has arrived before returning.
  • SinkConnection::try_recv is a non-async method that immediately returns 0 or more responses.

recv introduces huge delays because it does not return until the server can respond to the request which will be in the milliseconds, which is an eternity for a proxy.
During this time shotover cannot process any new requests coming in from the client on this connection.

Previously we had to use the async wait in recv to avoid spin waiting.
But due to recent improvements to shotover internals we can now return without waiting for all responses, so it is much more efficient to use try_recv instead.

Since we expect performance improvements from this PR, appropriate windsock benchmarks should be run to demonstrate the change actually improves performance.
Take a screenshot and post it in the PR.