Mux: Send async mux ack and fix stream error responses (#19149)

Streams can return errors if the cancelation is picked up before the response 
stream close is picked up. Under extreme load, this could lead to missing 
responses.

Send server mux ack async so a blocked send cannot block newMuxStream 
call. Stream will not progress until mux has been acked.
This commit is contained in:
Klaus Post 2024-02-28 10:05:18 -08:00 committed by GitHub
parent 51874a5776
commit 40fb3371fa
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 35 additions and 11 deletions

View File

@ -1504,6 +1504,9 @@ func (c *Connection) handleMuxServerMsg(ctx context.Context, m message) {
}) })
} }
if m.Flags&FlagEOF != 0 { if m.Flags&FlagEOF != 0 {
if v.cancelFn != nil && m.Flags&FlagPayloadIsErr == 0 {
v.cancelFn(errStreamEOF)
}
v.close() v.close()
if debugReqs { if debugReqs {
fmt.Println(m.MuxID, c.String(), "handleMuxServerMsg: DELETING MUX") fmt.Println(m.MuxID, c.String(), "handleMuxServerMsg: DELETING MUX")

View File

@ -323,7 +323,10 @@ func (m *muxClient) handleOneWayStream(respHandler chan<- Response, respServer <
if debugPrint { if debugPrint {
fmt.Println("Client sending disconnect to mux", m.MuxID) fmt.Println("Client sending disconnect to mux", m.MuxID)
} }
m.addErrorNonBlockingClose(respHandler, context.Cause(m.ctx)) err := context.Cause(m.ctx)
if !errors.Is(err, errStreamEOF) {
m.addErrorNonBlockingClose(respHandler, err)
}
return return
case resp, ok := <-respServer: case resp, ok := <-respServer:
if !ok { if !ok {
@ -463,6 +466,7 @@ func (m *muxClient) response(seq uint32, r Response) {
fmt.Println(m.MuxID, m.parent.String(), "CHECKSEQ FAIL", m.RecvSeq, seq) fmt.Println(m.MuxID, m.parent.String(), "CHECKSEQ FAIL", m.RecvSeq, seq)
} }
PutByteBuffer(r.Msg) PutByteBuffer(r.Msg)
r.Msg = nil
r.Err = ErrIncorrectSequence r.Err = ErrIncorrectSequence
m.addResponse(r) m.addResponse(r)
return return
@ -474,6 +478,8 @@ func (m *muxClient) response(seq uint32, r Response) {
} }
} }
var errStreamEOF = errors.New("stream EOF")
// error is a message from the server to disconnect. // error is a message from the server to disconnect.
func (m *muxClient) error(err RemoteErr) { func (m *muxClient) error(err RemoteErr) {
if debugPrint { if debugPrint {

View File

@ -103,14 +103,20 @@ func newMuxStream(ctx context.Context, msg message, c *Connection, handler Strea
BaseFlags: c.baseFlags, BaseFlags: c.baseFlags,
} }
// Acknowledge Mux created. // Acknowledge Mux created.
var ack message // Send async.
ack.Op = OpAckMux var wg sync.WaitGroup
ack.Flags = m.BaseFlags wg.Add(1)
ack.MuxID = m.ID go func() {
m.send(ack) defer wg.Done()
if debugPrint { var ack message
fmt.Println("connected stream mux:", ack.MuxID) ack.Op = OpAckMux
} ack.Flags = m.BaseFlags
ack.MuxID = m.ID
m.send(ack)
if debugPrint {
fmt.Println("connected stream mux:", ack.MuxID)
}
}()
// Data inbound to the handler // Data inbound to the handler
var handlerIn chan []byte var handlerIn chan []byte
@ -118,6 +124,7 @@ func newMuxStream(ctx context.Context, msg message, c *Connection, handler Strea
m.inbound = make(chan []byte, inboundCap) m.inbound = make(chan []byte, inboundCap)
handlerIn = make(chan []byte, 1) handlerIn = make(chan []byte, 1)
go func(inbound <-chan []byte) { go func(inbound <-chan []byte) {
wg.Wait()
defer xioutil.SafeClose(handlerIn) defer xioutil.SafeClose(handlerIn)
// Send unblocks when we have delivered the message to the handler. // Send unblocks when we have delivered the message to the handler.
for in := range inbound { for in := range inbound {
@ -133,6 +140,7 @@ func newMuxStream(ctx context.Context, msg message, c *Connection, handler Strea
// Handler goroutine. // Handler goroutine.
var handlerErr *RemoteErr var handlerErr *RemoteErr
go func() { go func() {
wg.Wait()
start := time.Now() start := time.Now()
defer func() { defer func() {
if debugPrint { if debugPrint {
@ -154,6 +162,7 @@ func newMuxStream(ctx context.Context, msg message, c *Connection, handler Strea
}() }()
// Response sender gorutine... // Response sender gorutine...
go func(outBlock <-chan struct{}) { go func(outBlock <-chan struct{}) {
wg.Wait()
defer m.parent.deleteMux(true, m.ID) defer m.parent.deleteMux(true, m.ID)
for { for {
// Process outgoing message. // Process outgoing message.
@ -196,6 +205,7 @@ func newMuxStream(ctx context.Context, msg message, c *Connection, handler Strea
// Remote aliveness check. // Remote aliveness check.
if msg.DeadlineMS == 0 || msg.DeadlineMS > uint32(lastPingThreshold/time.Millisecond) { if msg.DeadlineMS == 0 || msg.DeadlineMS > uint32(lastPingThreshold/time.Millisecond) {
go func() { go func() {
wg.Wait()
t := time.NewTicker(lastPingThreshold / 4) t := time.NewTicker(lastPingThreshold / 4)
defer t.Stop() defer t.Stop()
for { for {

View File

@ -74,10 +74,15 @@ func (s *Stream) Results(next func(b []byte) error) (err error) {
} }
} }
}() }()
doneCh := s.ctx.Done()
for { for {
select { select {
case <-s.ctx.Done(): case <-doneCh:
return context.Cause(s.ctx) if err := context.Cause(s.ctx); !errors.Is(err, errStreamEOF) {
return err
}
// Fall through to be sure we have returned all responses.
doneCh = nil
case resp, ok := <-s.responses: case resp, ok := <-s.responses:
if !ok { if !ok {
done = true done = true