Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RFC: Proposed protocol for insuring bitswap is more reliable #201

Closed
wants to merge 1 commit into from

Conversation

hannahhoward
Copy link
Contributor

Goals

We have been dealing with ongoing issues in go-bitswap with remote peers not sending wanted blocks because for any number of reasons, the want list on the remote peer becomes out of sync with the local peer. In addition the remote peer may send the block and think it's no longer wanted, but for whatever reason it's not received. We're currently dealing with all this by periodically re-sending the whole wantlist, but this is an imperfect fix. Fundamentally, the issue is that apart from sending the full wantlist, the bitswap protocol lacks any form of error correction that insures that all messages are received and processed in order.

Implementation

The proposed change adds session ids and frame numbers to the protocol, to make it more reliable. It does so in a way that is backwards compatible when communicating with older peers.

When a new peer connects, the connection is assigned a local unique session id (likely a 16 byte uuid) and a starting frame (should be >0 which is the default value for protobuf ints). Each wantlist change or block send increments the frame counter. This includes a full wantlist send. Frames can be joined together using the following rules:

  • wantlist delta frames are joined by simply applying the deltas successively
  • wantlist full frames are joined by overwriting all previous changes
  • block frames are joined by concatenation
  • if the session id changes everything is overwritten and reset

When sending a message, one or more frames are joined together, and sent with the local session id, and the starting and ending frame.

When a message is received, the local copy of the other peers wantlist is rewound to the start frame as necessary, then deltas are applied. (block sends are less important to rewind cause there's no need re-absorb blocks). The change is not applied if the last received frame is less than the starting frame of the incoming message (it may be kept around temporarily in case the missing in between message is received).

A peer also attaches an acknowledgement of the last received remote session id, and the last received remote frame whenever it sends a message. A peer should never send a frame that is less than the lack acknowledged frame (or there's no point in doing so). The lack of any acknowledgement in a message is an indication a peer does not support error correction and the protocol should be downgraded to BS 1.1. The lack of the acknowledged frame advancing means there has been a lost message and old changes should be aggregated and resent.

@hannahhoward
Copy link
Contributor Author

Tagged a few people cause I wasn't sure who was the appropriate reviewer. Feel free to untag yourself.

@Stebalien
Copy link
Member

I'm a bit concerned with the amount of state we'd need to keep with this protocol. We'd basically be implementing our own TCP on top of messages on top of reliable streams.

There are three things I believe we can take advantage of:

  1. We can actually know if the other side has successfully received a message (I think). We can do this by:
  2. Sending the message update.
  3. Closing the stream.
  4. Waiting for an EOF.
  5. Wantlists are small. We can resend the entire wantlist if something goes wrong.
  6. We can detect connection issues through stream resets. When we detect that our control/want stream is reset, we can just resend the entire wantlist. Note: we can still close the want stream and reopen it without having to do any of this.

So, I believe all we really need are:

  1. Sequence numbers so we can order messages coming in on different streams. We at least need epoch numbers, incremented once per stream, but sequence numbers are probably simpler.
  2. A rule that says that we must send a full update whenever a want stream gets reset (but not when we intentionally close it.
  3. Some logic that waits for an EOF when sending blocks and, if the block send fails (reset and not an EOF) we re-queue the task in the task queue (maybe mark it as failed once and backoff?).

A proposed protocol for insuring bitswap is more reliable
@daviddias daviddias force-pushed the feat/bitswap-error-correction branch from a58caad to 084057a Compare October 17, 2019 07:19
}
```

#### Error Correction

Bitswap 1.2 introduces session ids and frames to make the protocol more reliable. When a new peer connects, the connection is assigned a local unique session id (likely a 16 byte uuid) and a starting frame (should be >0 which is the default value for protobuf ints). Each sent want list change or block send increments the frame counter. When sending a message, one or more frames are joined together, and sent with the local session id, and the starting and ending frame. When the remote peer sends its next message (or immediately), it sends last received local session id, and the last received and transmitted frame (along with its own session_id and frame range). Deltas are never applied unless the starting frame is lesser or equal to the last frame received and applied. Skillful buffering on both sides can use this mechanism to insure an accurate wantlist is maintained and blocks are resent when neccesary. Meanwhile, the fact that the rest of the protocol is identical means the presence of default (missing) values is used to detect and downgrade when communicating with older peers.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bitswap 1.2 as in "go package version" or as in "new wire protocol"? Are these spec'ed out somewhere?

@hsanjuan
Copy link
Contributor

I guess this has been outdated given latest changes to bitswap? I'm closing. Reopen if needed (cc. @dirkmc ).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants