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

Fix falcore benchmark #2270

Merged
merged 1 commit into from
Sep 14, 2016
Merged

Fix falcore benchmark #2270

merged 1 commit into from
Sep 14, 2016

Conversation

methane
Copy link
Contributor

@methane methane commented Sep 14, 2016

fixes #2269

@knewmanTE
Copy link
Contributor

@methane thanks for the quick turnaround on the fix! Out of curiosity, how did removing the hardcoded content types fix the issue?

@knewmanTE knewmanTE modified the milestone: Round 13 Sep 14, 2016
@methane
Copy link
Contributor Author

methane commented Sep 14, 2016

http.Header is typedef of map. Some handlers returns same map instance.

Then, following filter adds headers to the http.Header which http handlers returns.

var requiredHeaders = falcore.NewResponseFilter(func(req *falcore.Request, res *http.Response) {
    res.Header.Set("Server", "falcore")
    res.Header.Set("Date", time.Now().Format(time.RFC1123))
  })

@knewmanTE
Copy link
Contributor

Ahh, makes sense. Yeah, I figured there was something that was returning the same instance across threads.

Did some test runs locally and it seems to be running okay for me! the only thing I noticed was that, when I run the plaintext test with the pipeline.lua script like it is in the suite:

 wrk -H 'Host: localhost' -H 'text/plain,text/html;q=0.9,application/xhtml+xml;q=0.9,application/xml;q=0.8,*/*;q=0.7' -H 'Connection: keep-alive' --latency -d 15 -c 256 --timeout 8 -t 4 http://10.0.0.1:8080/plaintext -s ~/pipeline.lua -- 16

I get repeated output that looks like this:

2016/09/14 11:17:57 [EROR] 46156 ERROR writing response: <*net.OpError write tcp 10.0.0.1:8080->10.0.0.3:48382: write: broken pipe>

Plaintext is still returning with responses, but perhaps its performance is less than it should be. In cases where I get that error, wrk is showing very high latency. I think this might be a result of the pipeline.lua script, which, as I understand it, generates ~16x more requests. Generally, max latency increases the more connections that are added, so the pipeline.lua script may just be exacerbating the problem. For comparison, these are my latency results with and without the lua script:

without pipeline.lua

concurrency max latency
8 29.89ms
256 80.40ms
1024 272.20ms
4096 885.84ms

with pipeline.lua

concurrency max latency
8 28.21ms
256 437.88ms
1024 1.44s
4096 6.49s

Anyway, those are just my observations in my local testing. If you're satisfied with the falcore results, I'm happy to merge this in!

@methane
Copy link
Contributor Author

methane commented Sep 14, 2016

The error may happens while wrk is stopping. (wrk may not wait all response of pipelined request.)
And I think the latency is because of overload.

If falcore result is very worse than other Go frameworks, I'll investigate. Go ahead.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Go/falcore breaking on concurrency tests
2 participants