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

Responses including cached Date header #4291

Closed
an-tao opened this issue Dec 24, 2018 · 14 comments
Closed

Responses including cached Date header #4291

an-tao opened this issue Dec 24, 2018 · 14 comments
Assignees

Comments

@an-tao
Copy link
Contributor

an-tao commented Dec 24, 2018

I did some testing in the benchmark project and I found that in the tests 'Json' and 'Plaintext', the response 'Date' header of some frameworks is always the same as the first response. It hints that the response entire text string including headers and the body is cached in memory for every request. According to the benchmark document, this seems to be illegal. But the framework that uses caching occupies a high position in the rankings. Am I wrong with the test rules? Does the test client check the Date header to ensure that each response is generated separately?

@NateBrady23
Copy link
Member

It would be against the test rules to cache the response. Would you mind pointing out the frameworks you believe to be doing that so we can investigate further? Thanks!

@an-tao
Copy link
Contributor Author

an-tao commented Dec 25, 2018

I think the ulib framework is caching the response, maybe I was wrong, but when I followed it's dockerfile to run the test, I got responses by the curl command as follows:
2018-12-25 9 40 41
As you can see, all the Date headers are the same. Do you think that what I suspect is reasonable? If I am wrong, please correct me. Thank you!

@dividedbynil
Copy link

@stefanocasazza

@stefanocasazza
Copy link
Contributor

I simple follow as other frameworks to not update the Date header for performance reason. So far it is not a requirement to have a Date header timed aligned

@fafhrd91
Copy link
Contributor

Rust top performing frameworks do update date header.

@NateBrady23
Copy link
Member

Per the test rules, the response must be composed on the spot. This should include the proper date, though we currently do not check that the date header is different from request to request, just that it's present. I'll look to add this as part of the verification step before the next round, marking tests that don't fix this in the future as failed.

@bhauer @msmith-techempower @michaelhixson Anything to add here?

@NateBrady23 NateBrady23 changed the title Can the response be cached? Responses including cached Date header Dec 28, 2018
@NateBrady23 NateBrady23 pinned this issue Dec 28, 2018
@NateBrady23 NateBrady23 self-assigned this Dec 28, 2018
@zloster
Copy link
Contributor

zloster commented Jan 4, 2019

@nbrady-techempower Some test implementation are using time-expiration caching for the date header. They refresh the date header once a second or so. Out my head I can recall:

The HTTP date header is with an 1 second granularity - https://www.w3.org/Protocols/rfc2616/rfc2616-sec3.html#sec3.3.1 So caching for a second should not have any big practical implications. Maybe the rules should be amended if this behaviour is allowed.

@NateBrady23
Copy link
Member

@zloster I think we're probably ok with that but I will double check. Whatever verification we put into the toolset likely wouldn't flag that anyway.

@zloster
Copy link
Contributor

zloster commented Jan 6, 2019

@nbrady-techempower I agree.
Just wanted to make sure this fine detail don't get lost.

@boazsegev
Copy link
Contributor

F.Y.I,

The facil.io framework also caches the date string using a single second granularity (not the whole header, but still).

I was assuming that's allowed, since the result doesn't effect the response (it should always show the correct date/time)...

@benaadams
Copy link
Contributor

I assume the General requirements have been updated for this:

  1. ... For Date, we expect that the rendered date be accurate. However, it does not need to be rendered from the system clock to a byte buffer for each request. Re-rendering once per second is an acceptable tactical optimization (and is an optimization baked into many frameworks).

@NateBrady23
Copy link
Member

Yeah just to confirm, we're ok with re-rendering/caching just the date header every one second. Date headers that remain unchanged will cause tests to fail in upcoming rounds.

@NateBrady23
Copy link
Member

@helyho Java/voovan needs to be updated, as it will fail before the next round because of this issue.

@smthing Same with Java/smart-socket

@NateBrady23
Copy link
Member

This should be resolved with #4452

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

No branches or pull requests

8 participants