Add flow control observability #7810
Conversation
okhttp/src/jvmMain/kotlin/okhttp3/internal/http2/Http2Connection.kt
Outdated
Show resolved
Hide resolved
| } | ||
| connection.flowControlListener.receivingStreamWindowChanged(id, readBytes, readBuffer.size) |
There was a problem hiding this comment.
Anything where we call a user-owned listener while holding a lock is bad news. Something to keep in mind if ever we wanted to make a public API.
There was a problem hiding this comment.
Yep. I am becoming more convinced for libraries like exoplayer we should make the flow control visible in some form. But happy to agree to remove this before 5.0 unless we find that nice API.
There was a problem hiding this comment.
I think this can move, out of synchronized, but inside the loop.
| val streamId: Int | ||
| ) { | ||
| /** The total number of bytes consumed. */ | ||
| private val _total: AtomicLong = AtomicLong(0) |
There was a problem hiding this comment.
I don't think I'm clear on all the locking that goes on in all cases. I originally had a data class that was updated. Might switch back to that and define locking rules around writes.
There was a problem hiding this comment.
Removed, no need.
Add an internal API for receiving HTTP/2 flow control information, definitely not complete.
This shows
Connection - unacked on connection - this is not true in the first, as connection doesn't bump until consumed via a stream
3 - The unacked on stream 3 (the stalled request)
3-buffer - The data sitting inside the Buffer to be read
5 & 5-buffer - same as 3.
As an example used before and after this fix https://github.com/square/okhttp/pull/7801/files