cockroachdb / cockroachdb/cockroach

drpc: review MsgSend semantics when the stream is canceled after a successful flush

Open
#163,836 1 comment 0 reactions 0 assignees View on GitHub
C-bug T-db-server
Dominant language
Go
Stars
32.5k
Forks
4.1k
PR merge metrics
PR metrics pending

Description

**Describe the problem**
After drpc MsgSend flushes the write buffer, it currently [checks for cancelation](https://github.com/cockroachdb/drpc/blob/ba071c6f9395def71be74b365b3351ee4869cef8/drpcstream/stream.go#L428) and returns an error if the stream was canceled. If the goroutine that calls flush is scheduled out for a long time after the flush but before executing the cancelation check, it could result in unexpected semantics for the calling code.
For example, consider the execution sequence below:

```
3 Goroutines

t1 MsgSend goroutine (client)
t2. MsgRecv goroutine (client)
t3. manageStream goroutine (client)

Local Node Remote Node
------------------------------------------------------------
t1 MsgSend flushes data
successfully and is scheduled MsgRecv
out. Processes msg
MsgSend
t2 MsgRecv starts and finishes
t3 Receives context cancelation
t1 is scheduled - MsgSend calls
checkCancelError, returns error
```

In the above scenario, the client has received a response for a request whose Send failed with errors. This can have unintended consequences.

**To Reproduce**

This issue was seen in a [test failure](https://github.com/cockroachdb/cockroach/issues/157997). The ticket and the slack thread referenced in it have more details about the issue and how to reproduce it.

**Expected behavior**
As a fix for this, we could consider two alternatives:
- On the client side, detect and discard a response that arrives out of turn.
- Review MsgSend semantics to see if it makes sense to process cancel errors in MsgSend after the message has been successfully flushed to the remote side.

Jira issue: CRDB-60489

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.