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

HTTP response validation #239

Closed
greenlaw110 opened this issue Jun 8, 2018 · 15 comments
Closed

HTTP response validation #239

greenlaw110 opened this issue Jun 8, 2018 · 15 comments

Comments

@greenlaw110
Copy link
Contributor

greenlaw110 commented Jun 8, 2018

An HTTP response should have at least the following headers:

Content-Type
Content-Length (unless chunked)
Date
Server
@waghanza
Copy link
Collaborator

waghanza commented Jun 8, 2018

@OvermindDL1 what do you think ? evhtp has a weak throughput on https://github.com/waghanza/which_is_the_fastest/tree/wrk#all-frameworks

@OvermindDL1
Copy link
Collaborator

@waghanza That is only because it is not returning optional headers. I did not bother setting additional headers in it like the others that I saw, thus it is only returning HTTP Mandatory Headers. It's throughput is lower because it's overall requests/sec is significantly higher. I absolutely agree that the headers should be standardized as it should make the throughput match the requests/s at that point for all frameworks.

I'm curious how go is showing higher requests/s than C++, I was not able to replicate that on my servers (I did not test Rust's actix). I'm wondering if it is due to the lower core count on the machine being tested, 4 cores is not very high (my testing server has 8 physical cores, 16 hyperthreads, AMD's new chip has 32 cores on a single chip, my big server, which is far too loaded down to do actual tests on sadly, has 16 physical xeon cores).

@waghanza
Copy link
Collaborator

@OvermindDL1 the question is more, which headers are required ?

after answering this, we need to fix all suites / tests

@OvermindDL1
Copy link
Collaborator

OvermindDL1 commented Jun 11, 2018

Well the HTTP spec itself doesn't mandate very many headers at all. Adding headers to most of them probably won't affect results any as they will likely still fit in a single packet anyway (so it would increase throughput without messing with reqs/sec much if at all). The list above is a good set to add anyway. Content-Length should be required regardless, chunking should not be used as they are empty/short bodies.

However, adding a Date header should probably not be added. It is not required by the spec and many production web servers don't add it because it adds a syscall overhead to the requests, thus incurring serialization to the kernel and can SIGNIFICANTLY reduce overall performance.

@waghanza
Copy link
Collaborator

@greenlaw110 COULD date header been disabled on act ?

@greenlaw110
Copy link
Contributor Author

Checkout this:

7.1.1.2. Date

An origin server MUST NOT send a Date header field if it does not have a clock capable of providing a reasonable approximation of the current instance in Coordinated Universal Time. An origin server MAY send a Date header field if the response is in the 1xx (Informational) or 5xx (Server Error) class of status codes. An origin server MUST send a Date header field in all other cases.

@OvermindDL1
Copy link
Collaborator

Indeed, if you don't put a Date then you must follow:

Some origin server implementations might not have a clock available. An origin server without a clock MUST NOT assign Expires or Last- Modified values to a response, unless these values were associated with the resource by a system or user with a reliable clock. It MAY assign an Expires value that is known, at or before server configuration time, to be in the past (this allows "pre-expiration" of responses without storing separate Expires values for each resource).

It's generally inferred that Date is required if you need that functionality, otherwise it is optional pursuant to the clock not available section.

A quick test shows that the frameworks are sending a date already so it may just be a moot point, plus on modern linux's it doesn't require a syscall any longer anyway (it's in the kernel shared memory in modern versions), it was a bigger issue back in the day.

@waghanza
Copy link
Collaborator

@greenlaw110 @OvermindDL1 I think the throughput is a specific feature of each framework.

For example, in the last version of this benchmark :

  • Rails has 4.48 MB but roda has 15.90 MB
  • Polka has 40.83 MB but express has 37.87 MB

So, even if the language is the same, throughput COULD differs, it's a part of practices use by frameworks.

For me, we SHOULD keep the default version for each framework, without tweaking / optimizing throughput, it's a framework choice

@greenlaw110
Copy link
Contributor Author

Rails has 4.48 MB but roda has 15.90 MB

That means Rails is much slower than roda, nothing else.

What I want with this issue is to define the response specification for each test, and the specification should include BOTH header and body. Makes sense?

@waghanza
Copy link
Collaborator

Sure, but this was not the question here.

The question IS :

Is our responsability to ensure a standards responses (in headers ...) ?

I thin, NO.

For me it's a framework feature. Like SO_REUSEPORT in crystal is a language feature, so I'm not for implementing it here.

@OvermindDL1 what do you think ?

@greenlaw110
Copy link
Contributor Author

greenlaw110 commented Jun 13, 2018

As we are talking about standard. Then I would say if framework's default behavior doesn't follow the standard, then the benchmark should regular the implementation to overwrite the default behavior.

BTW, SO_REUSEPORT is NOT a standard response header.

@waghanza
Copy link
Collaborator

I understand your point of view. However, I think having a non-default implementations of each frameworks COULD lead to mis-understanding of results / false comparison.

I mean, many developers use framework for what it is (default), and not tweaking to optimize throughput

@OvermindDL1
Copy link
Collaborator

Many frameworks do default to different headers, but they are generally expected to be replaced by the application using them, they are just simple defaults to get running. Honestly it is probably fine to just leave them at defaults, focus on req/s that way (otherwise throughput is testing mismatched things, plus req/s and latency is what the client browser really cares about).

As for SO_REUSEPORT, that is not a crystal thing, that's an OS thing, everything from ruby to python to crystal are all expected to use it as they are inherently single-core. Even the Crystal dev's themselves state that it should be used with the web frameworks. And no SO_REUSEPORT is not a response header, it's a way to mark a listen port for multiple process use.

@waghanza
Copy link
Collaborator

OK for headers

@waghanza waghanza mentioned this issue Jun 13, 2018
@waghanza
Copy link
Collaborator

waghanza commented Jul 2, 2018

@OvermindDL1 I find an interested links https://github.com/vibora-io/benchmarks#infamous-hello-world

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

No branches or pull requests

3 participants