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

Accurate byte counters #2558

Closed

Conversation

Projects
None yet
4 participants
@mbgrydeland
Copy link
Contributor

commented Feb 6, 2018

There was a regression from Varnish 4.0 to 4.1, where the response
bytes was counted as the number of bytes fed to the outgoing write
vector, rather than the bytes that was actually handed off to the OS'
socket buffer. This would cause for many cases the complete object
size counted as transmitted bytes, even though the client hung up the
connection early.

This patch changes the counters to show the amount of bytes sent as
reported from the write() system calls rather than the bytes we planned
and prepared to send. The counters will include any protocol overhead (ie
chunked encoding in HTTP/1 and the frame headers in HTTP/2).

ESI subrequests will as before in their log transactions report the number
of bytes it (and any subrequests below it) contributed to the total body
bytes produced.

Some test cases have been adjusted to account for the new counter behaviour.

One new test case added that attempts to catch this regression.

Accurate byte counters
There was a regression from Varnish 4.0 to 4.1, where the response
bytes was counted as the number of bytes fed to the outgoing write
vector, rather than the bytes that was actually handed off to the OS'
socket buffer. This would cause for many cases the complete object
size counted as transmitted bytes, even though the client hung up the
connection early.

This patch changes the counters to show the amount of bytes sent as
reported from the write() system calls rather than the bytes we planned
and prepared to send. The counters will include any protocol overhead (ie
chunked encoding in HTTP/1 and the frame headers in HTTP/2).

ESI subrequests will as before in their log transactions report the number
of bytes it (and any subrequests below it) contributed to the total body
bytes produced.

Some test cases have been adjusted to account for the new counter behaviour.

One new test case added that attempts to catch this regression.
@bsdphk

This comment has been minimized.

Copy link
Contributor

commented Feb 7, 2018

I pressume this is in response to #2536 ?

@mbgrydeland

This comment has been minimized.

Copy link
Contributor Author

commented Feb 7, 2018

Yes, this supersedes #2536 .

I see that my enterprising test case for this failed on travis. It seemed to work fine on my computer, but I was worried that the test case for this would be brittle. We might want to drop the test case, or come up with some better way of testing the interrupted writes.

@bsdphk bsdphk added a=OK'ed and removed a=bugwash labels Feb 19, 2018

@bsdphk

This comment has been minimized.

Copy link
Contributor

commented Feb 19, 2018

Ditch the test-case and commit, testing partial writes to buffers is not realistic.

@mbgrydeland

This comment has been minimized.

Copy link
Contributor Author

commented Feb 19, 2018

Merged without the test-case in 5117664

@mbgrydeland mbgrydeland deleted the mbgrydeland:accurate_byte_counters branch Feb 19, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.