Skip to content

Correctly track H2 post vio bytes #6407

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.

Sign up for GitHub

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

Closed
shinrich wants to merge 1 commit into from
+20 −4
Closed

Correctly track H2 post vio bytes #6407

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
8 changes: 5 additions & 3 deletions proxy/http2/Http2ConnectionState.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -180,12 +180,14 @@ rcv_data_frame(Http2ConnectionState &cstate, const Http2Frame &frame)
if (nbytes + read_len > unpadded_length) {
read_len -= nbytes + read_len - unpadded_length;
}
nbytes += writer->write(myreader, read_len);
myreader->consume(nbytes);
int num_written = writer->write(myreader, read_len);
nbytes += num_written;
myreader->consume(num_written);
stream->update_read(num_written);
}
myreader->writer()->dealloc_reader(myreader);

if (frame.header().flags & HTTP2_FLAGS_DATA_END_STREAM) {
Copy link
Contributor

@bryancall bryancall Feb 8, 2020

Choose a reason for hiding this comment

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

Why are we treating read_vio.ntodo() == 0 the same as getting a data end stream flag?

Copy link
Member Author

@shinrich shinrich Feb 10, 2020

Choose a reason for hiding this comment

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

Protecting ourselves from ill-written clients that do not send DATA_END_STREAM. We have seen such clients in our production data stream. In that case they are sending Content-length, so we can still send READ_COMPLETE when all of the data is written.

if (frame.header().flags & HTTP2_FLAGS_DATA_END_STREAM || stream->read_vio_done()) {
// TODO: set total written size to read_vio.nbytes
stream->signal_read_event(VC_EVENT_READ_COMPLETE);
} else {
Expand Down
2 changes: 1 addition & 1 deletion proxy/http2/Http2Stream.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,7 @@ Http2Stream::send_request(Http2ConnectionState &cstate)
return;
}

if (this->recv_end_stream) {
if (this->recv_end_stream || this->read_vio.ntodo() == 0) {
this->read_vio.nbytes = bufindex;
this->signal_read_event(VC_EVENT_READ_COMPLETE);
} else {
Expand Down
14 changes: 14 additions & 0 deletions proxy/http2/Http2Stream.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,8 @@ class Http2Stream : public ProxyTransaction
bool has_trailing_header() const;
void set_request_headers(HTTPHdr &h2_headers);
MIOBuffer *read_vio_writer() const;
bool read_vio_done() const;
void update_read(int num_written);

//////////////////
// Variables
Expand Down Expand Up @@ -326,3 +328,15 @@ Http2Stream::read_vio_writer() const
{
return this->read_vio.get_writer();
}

inline void
Http2Stream::update_read(int num_written)
{
read_vio.ndone += num_written;
}

inline bool
Http2Stream::read_vio_done() const
{
return read_vio.ntodo() == 0;
}