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

http2: send client conn flow control bytes back immediately #144

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 9 additions & 2 deletions http2/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -1772,10 +1772,18 @@ func (sc *serverConn) processData(f *DataFrame) error {
st.bodyBytes += int64(len(data))
}

// Connection level flow control window update must be
// sent immediately after receiving data frame.
// This separates connection's flow control from body reads
// as connection's flow control must not depend on whether
// body has been read by application handler or not.
// This prevents fast streams from starving due to other
// slow streams.
sc.sendWindowUpdate32(nil, int32(f.Length))

// Return any padded flow control now, since we won't
// refund it later on body reads.
if pad := int32(f.Length) - int32(len(data)); pad > 0 {
sc.sendWindowUpdate32(nil, pad)
sc.sendWindowUpdate32(st, pad)
}
}
Expand Down Expand Up @@ -2317,7 +2325,6 @@ func (sc *serverConn) noteBodyReadFromHandler(st *stream, n int, err error) {

func (sc *serverConn) noteBodyRead(st *stream, n int) {
sc.serveG.check()
sc.sendWindowUpdate(nil, n) // conn-level
if st.state != stateHalfClosedRemote && st.state != stateClosed {
// Don't send this WINDOW_UPDATE if the stream is closed
// remotely.
Expand Down
20 changes: 11 additions & 9 deletions http2/server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1251,19 +1251,21 @@ func TestServer_Handler_Sends_WindowUpdate(t *testing.T) {
EndHeaders: true,
})
st.writeData(1, false, []byte("abcdef"))
// Expect to immediately get connection level 6 bytes back.
st.wantWindowUpdate(0, 6)

puppet.do(readBodyHandler(t, "abc"))
st.wantWindowUpdate(0, 3)
st.wantWindowUpdate(1, 3)

puppet.do(readBodyHandler(t, "def"))
st.wantWindowUpdate(0, 3)
st.wantWindowUpdate(1, 3)

st.writeData(1, true, []byte("ghijkl")) // END_STREAM here
st.wantWindowUpdate(0, 6)

// no more stream-level window updates, since END_STREAM
puppet.do(readBodyHandler(t, "ghi"))
puppet.do(readBodyHandler(t, "jkl"))
st.wantWindowUpdate(0, 3)
st.wantWindowUpdate(0, 3) // no more stream-level, since END_STREAM
}

// the version of the TestServer_Handler_Sends_WindowUpdate with padding.
Expand All @@ -1286,17 +1288,17 @@ func TestServer_Handler_Sends_WindowUpdate_Padding(t *testing.T) {
})
st.writeDataPadded(1, false, []byte("abcdef"), []byte{0, 0, 0, 0})

// Expect to immediately get our 5 bytes of padding back for
// both the connection and stream (4 bytes of padding + 1 byte of length)
st.wantWindowUpdate(0, 5)
// Expect to immediately get our 6 bytes of data + 5 bytes of padding
// back at connection level.
// (4 bytes of padding + 1 byte of padding length)
st.wantWindowUpdate(0, 11)
// Expect to immediately get our 5 bytes of padding back at stream level.
st.wantWindowUpdate(1, 5)

puppet.do(readBodyHandler(t, "abc"))
st.wantWindowUpdate(0, 3)
st.wantWindowUpdate(1, 3)

puppet.do(readBodyHandler(t, "def"))
st.wantWindowUpdate(0, 3)
st.wantWindowUpdate(1, 3)
}

Expand Down