Adding System.IO.Pipelines to replace Channels and Streams (replaces #949)

[stebet]

Proposed Changes

This PR adds System.IO.Pipelines to replace using Channels and Streams to receive and send data to/from the client.

• Removes Channels/Streams and replaces them with PipeReader/PipeWriter, which are now handled by the Connection instead of SocketFrameHandler
  • Simplifies the SocketFrameHandler a lot! Gets rid of extra Read/Write tasks which are now managed by the Pipelines.
  • Does require sync-over-async ugliness to Flush the Pipe when publishes manage to fill the Pipe buffer faster than data gets sent. That is however rare under normal circumstances.
• Prepares the client to be able to support fully asynchronous operations more easily.
  • This does create a little bit of CPU/Allocation overhead for the async operations under mass publish scenarios (benchmarks incoming), but we would always have run into those issues when making the operations Async anyway or when fixing the potential OOM issues like in #1106 due to having to create "fake" tasks to block while waiting for the buffers to drop below the threshold levels. This overhead should however be negligible under normal scenarios and should actually be a lot better off under fully async scenarios since we are less likely to block threads and create ThreadPool starvation scenarios.
• Should solve several problems similar to #1106, which stem from the current client being able publish messages faster than the network connection/RabbitMQ server can handle them for async scenarios.
• Lays groundwork to fix #970, #966 and #843

Types of Changes

What types of changes does your code introduce to this project? Put an x in the boxes that apply

• [X] Bug fix (non-breaking change which fixes issue #1106)
• [X] New feature (non-breaking change which adds functionality)
• [ ] Breaking change (fix or feature that would cause an observable behavior change in existing systems)
• [ ] Documentation improvements (corrections, new content, etc)
• [ ] Cosmetic change (whitespace, formatting, etc)

Checklist

• [X] I have read the CONTRIBUTING.md document
• [X] I have signed the CA (see https://cla.pivotal.io/sign/rabbitmq)
• [X] All tests pass locally with my changes
• [ ] I have added tests that prove my fix is effective or that my feature works
• [ ] I have added necessary documentation (if appropriate)
• [ ] Any dependent changes have been merged and published in related repositories

Further Comments

Reasoning

I see Pipelines being a perfect fit here. They make processing buffers a lot easier, reduce overhead when deserializing data and make it a lot easier to implement fully async operations. That's also why they are used for the same purpose in Kestrel (the ASP.NET Core server) Kestrel and StackExchange.Redis.

I also created as-of-yet unused WriteAsync methods for that purpose, which would make it easy to implement fully Async publish methods for example.

Benchmark results (to be added)

[stebet]

Still ironing out a few things that only seem to manifest in the CI tests (tests all run fine locally), but think they have to do with the fact that synchronous publish now potentially blocks where before it had an unlimited buffer with the UnboundedChannel to write into, so WaitForConfirm tests behave slightly differently with Pipelines.

[stebet]

Yeah I'm working on cleaning it up a bit. Found some test issues and I'm working on a PR to clean those up first to reduce the noise.

[michaelklishin]

Sorry to drag you into resolving some non-trivial conflicts @stebet. We decided to go ahead and merge https://github.com/rabbitmq/rabbitmq-dotnet-client/pull/1202 as it felt like the right thing to do since we require net6.0 now for 7.0. This obviously affects all pull requests in flight.

[michaelklishin]

I reverted #1202 because it makes back and forward-porting impossible unless all maintained branches adopt file-scoped namespaces.

[stebet]

No probs. I'm a bit busy with work and RL these days. Hoping to submit another PR first that cleans up the unit tests a lot and hopefully makes the more reliable and then clean this PR up after that.

[lukebakken]

@stebet I've actually got some time to dedicate to releasing version 7.0 of this library. I'll get the most recent conflicts fixed in this branch, as well as address the most recent review comments. We could discuss the remainder of work here and I could take over. Let me know what you think.

[lukebakken]

Hi @stebet, I'm wondering if you have time to read my comment and respond. @bollhals asked about the status of version 7.0 and one major task is completing this PR.

As I said, I'm happy to take over work as long as your piplines branch is up-to-date in your fork. Thanks!

[stebet]

I need to take a little bit closer look at this since I've been out of the loop a bit. I'll take a look in the next days and give you an update.

[lukebakken]

Superseded by #1264