Skip to content

Conversation

@masaori335
Copy link
Contributor

Another approach to fix #6178.

@masaori335 masaori335 added this to the 10.0.0 milestone Nov 19, 2019
@masaori335 masaori335 self-assigned this Nov 19, 2019
@masaori335
Copy link
Contributor Author

It looks like simple check of write_vio.ntodo() makes a problem with 100 Continue response for the POST request with expect: 100-continue header.

@masaori335
Copy link
Contributor Author

The issue with 100 Continue response looks exists regardless of the dechunk cleanup changes. Filed as #6202.

@masaori335
Copy link
Contributor Author

masaori335 commented Nov 19, 2019

100 continue issue is addressed with new commit. I'll try applying the fix on the 8.0.x branch.

shinrich
shinrich previously approved these changes Nov 19, 2019
Copy link
Member

@shinrich shinrich left a comment

Choose a reason for hiding this comment

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

These changes also correctly return a header only in the case of reponse-less statuses like 304.

@shinrich
Copy link
Member

Will set up a performance build too.

@shinrich
Copy link
Member

shinrich commented Nov 19, 2019

My test build on a prod box is looking good so far with respect to latency. Average latency staying around 120ms which is what I was seeing before the Dechunk changes. I'll build an official build tomorrow so I'm certain that I'm running the right thing. But so far, so good.

@masaori335
Copy link
Contributor Author

Pushed new commits. Logics are almost the same as before.

@masaori335 masaori335 merged commit 52538c4 into apache:master Nov 20, 2019
@zwoop
Copy link
Contributor

zwoop commented Dec 12, 2019

Oops, since there were two commits here, I missed one. I've cherry-picked both of these to the v9.0.x branch.

@zwoop zwoop modified the milestones: 10.0.0, 9.0.0 Dec 12, 2019
@masaori335 masaori335 removed this from the 9.0.0 milestone Apr 6, 2020
@masaori335 masaori335 added this to the 8.1.0 milestone Apr 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ATS9: performance regression

3 participants