Add optimistic channels

[aaronmondal]

This variation on make_buf_channel_pair omits the EOF check. This works around the fact that S3 drops connections as soon as all data is sent or received.

Fixes #304 and fixes the same issue for non-oneshot cases where a similar issue occurred as well.

---

This change is 

[MarcusSorealheis]

My question is should these ObjectStore or S3Store . I'm torn because I don't want us to be wed to a vendor in the implementation on the one hand, but on the other hand the other object stores in the market tend to implement the S3 API because they are a leader. My hesitation feels a little nit picky and pedantic and can probably be deferred.

Explicitly keeping S3 in the codebase as a store type is something we probably want to move away from.

Reviewable status: 0 of 3 files reviewed, all discussions resolved (waiting on @allada)

[allada]

Reviewed 2 of 3 files at r1.
Reviewable status: 2 of 3 files reviewed, 3 unresolved discussions (waiting on @aaronmondal)

---

util/buf_channel.rs line 58 at r1 (raw file):

/// the receiver can be trusted to handle failing streams this is safe to do.
#[must_use]
pub fn make_buf_channel_pair_optimistic_writer() -> (DropCloserWriteHalf, DropCloserReadHalf) {

Why not just add .disable_eof_check(&mut self) on the writer?

---

util/buf_channel.rs line 88 at r1 (raw file):

    /// receiver end's drop status (ie: if the receiver dropped unexpectedly).
    close_rx: oneshot::Receiver<Result<(), Error>>,
    disable_eof_check: bool,

It looks like we already have close_after_size on DropCloserReadHalf. I think we can use this flag to ensure the read half can get into this configuration.

As for the Writer, would it make more sense to simply add a similar kind of flag check it insend_eof() and not emit an error if the flag is set (ie: set_ignore_eof())?

---

util/buf_channel.rs line 131 at r1 (raw file):

    {
        loop {
            if let Some(maybe_chunk) = reader.next().await {

nit: I personally prefer the match syntax here, it feels easier to read to me. but just my opinion.

[allada]

Reviewed 2 of 3 files at r1.
Reviewable status: 2 of 3 files reviewed, 3 unresolved discussions (waiting on @aaronmondal)

[MarcusSorealheis]

Reviewable status: 0 of 3 files reviewed, 3 unresolved discussions (waiting on @aaronmondal and @allada)

[aaronmondal]

@MarcusSorealheis I agree that we don't want to name this S3. Something like ObjectStore or BlobStore seems more appropriate and lets us use more vendor-specific libraries to interact with AWS, GCP, Azure and custom solutions like CubeFS, Ceph and Minio.

I'll soon have a new implementation for S3 via the AWS SDK instead of rusoto ready. I still need to look into others. Then we can generalize the current implementation.

Reviewable status: 0 of 3 files reviewed, 3 unresolved discussions (waiting on @allada)

---

util/buf_channel.rs line 58 at r1 (raw file):

Previously, allada (Nathan (Blaise) Bruer) wrote…

Why not just add .disable_eof_check(&mut self) on the writer?

Done. Yes that's much better 👍

---

util/buf_channel.rs line 88 at r1 (raw file):

Previously, allada (Nathan (Blaise) Bruer) wrote…

It looks like we already have close_after_size on DropCloserReadHalf. I think we can use this flag to ensure the read half can get into this configuration.

As for the Writer, would it make more sense to simply add a similar kind of flag check it insend_eof() and not emit an error if the flag is set (ie: set_ignore_eof())?

Not sure I understood you correctly here. Is the new version what you envision?

---

util/buf_channel.rs line 131 at r1 (raw file):

Previously, allada (Nathan (Blaise) Bruer) wrote…

nit: I personally prefer the match syntax here, it feels easier to read to me. but just my opinion.

That was flagged by clippy::single_match_else I'd prefer the new variant since it reduces indentation.

[allada]

Reviewed 3 of 3 files at r2, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @aaronmondal)

---

util/buf_channel.rs line 88 at r1 (raw file):

Previously, aaronmondal (Aaron Siddhartha Mondal) wrote…

Not sure I understood you correctly here. Is the new version what you envision?

Yep, this is exactly what I was referring :-) ... To me this is so much easier to read.