Offload reading the replication stream to IO threads

[uriyage]

Support Primary client IO offload.

Related issue: #761

[codecov]

Codecov Report

Attention: Patch coverage is 68.75000% with 5 lines in your changes missing coverage. Please review.

Project coverage is 70.78%. Comparing base (980a801) to head (39f0a48).
Report is 29 commits behind head on unstable.

Files with missing lines	Patch %	Lines
src/io_threads.c	0.00%	3 Missing ⚠️
src/networking.c	81.81%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff            @@
##           unstable    #1449   +/-   ##
=========================================
  Coverage     70.78%   70.78%           
=========================================
  Files           119      119           
  Lines         64691    64740   +49     
=========================================
+ Hits          45790    45829   +39     
- Misses        18901    18911   +10     

Files with missing lines	Coverage Δ
src/replication.c	87.43% <100.00%> (-0.08%)	⬇️
src/server.h	100.00% <ø> (ø)
src/networking.c	88.35% <81.81%> (+0.06%)	⬆️
src/io_threads.c	6.94% <0.00%> (-0.58%)	⬇️

... and 20 files with indirect coverage changes

[zuiderkwast]

That was a small diff! Nice.

[zuiderkwast]

The PR title isn't very clear for someone who doesn't know the internals.

I believe a user could better understand a title like "Offload reading the replication stream to IO threads".

[uriyage]

The PR title isn't very clear for someone who doesn't know the internals.

I believe a user could better understand a title like "Offload reading the replication stream to IO threads".

Thanks, changed.

[zuiderkwast]

LGTM

I will not merge it yet in case others want to review it a bit more.

[jdheyburn]

To the layman such as myself, what benefit does this introduce WRT replication?

[zuiderkwast]

To the layman such as myself, what benefit does this introduce WRT replication?

Replicas can serve more readonly traffic when reading the replication stream can be offloaded to an IO thread instead of being handled by the bottleneck main thread. It's useful in setups where replicas are used for readonly traffic.

It seems more beneficial to let primaries offload the writes to replicas to IO threads. Hopefully, that will come in another PR. @uriyage WDYT?

[uriyage]

To the layman such as myself, what benefit does this introduce WRT replication?

Replicas can serve more readonly traffic when reading the replication stream can be offloaded to an IO thread instead of being handled by the bottleneck main thread. It's useful in setups where replicas are used for readonly traffic.

It seems more beneficial to let primaries offload the writes to replicas to IO threads. Hopefully, that will come in another PR. @uriyage WDYT?

@zuiderkwast Yes, I will open a new PR in the coming days for offloading the writes to replicas. It is a bit more complicated, which is why I started with the primary writes offload.

[ranshid]

@uriyage I think we need to check the failed test in TLS which I can see:

Error writing to client: error:0A00010F:SSL routines::bad length

Maybe it is related (is it the use of writev?)

[ranshid]

I rerun the test to see if this is a stable fail

[uriyage]

I rerun the test to see if this is a stable fail

The test runs without IO threads, in this case, the PR changes have no effect, meaning the failure is unrelated to this PR changes

[zuiderkwast]

I don't want to block PRs that are ready to merge. We should really focus on the failing tests instead of starting too many new features though.