Skip to content
This repository has been archived by the owner on Nov 5, 2021. It is now read-only.

Response ETag Header #65

Closed
crookse opened this issue Mar 10, 2019 · 15 comments
Closed

Response ETag Header #65

crookse opened this issue Mar 10, 2019 · 15 comments
Assignees

Comments

@crookse
Copy link
Member

crookse commented Mar 10, 2019

Summary

What: HTTP responses should send the ETag response header for improved caching. See https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/ETag for more info.

Why: Improve caching.

Acceptance Criteria

  • The ETag should be present in response headers.
  • @Lonniebiz points out a potential performance issue. Testing/benchmarking must be included for this feature. See his comment below for more information.

Example Pseudo Code (for implementation)

response.headers.set("ETag", getHeader_eTag());
@Lonniebiz
Copy link

Lonniebiz commented Jul 10, 2019

FYI, the creator of Fastify says you can increase performance in Express by disabling eTags. I'm not saying Drash shouldn't support etags, but I'd like see Drash performance be better than fastify and express; maybe that's possible on top of deno.

@crookse
Copy link
Member Author

crookse commented Jul 10, 2019

@Lonniebiz ,

Thanks for including the link! That's pretty interesting. I haven't done any benchmarking for Drash, but I'll make sure testing performance for this feature is included.

@ebebbington
Copy link
Member

ebebbington commented Apr 16, 2020

Just to clarify on a few things, that you probably know but just showing my findings:

  • ETag header is a hash of a single string, now maybe this could be this.request.route_path + this.request.method (response.ts constructor)

  • There doesn't seem to be a native method or Deno implementation for MD5 algo (which the Etag requires). I found a 3rd party one on Deno.land but the documentation is out of date and steps dont work, plus it's 3rd party (an external dependency)

  • I know virtually nothing about the specifics of hashing algorithms, however, maybe one could be created ourselves, for use within Drash and allow users to import it. Although, i have a block of code that hasn't specifically been mentioned it is an MD5 algorithm, but if it is we could use it:
    (taken from : https://stackoverflow.com/questions/6122571/simple-non-secure-hash-function-for-javascript)

String.prototype.hashCode = function() {
    var hash = 0;
    if (this.length == 0) {
        return hash;
    }
    for (var i = 0; i < this.length; i++) {
        var char = this.charCodeAt(i);
        hash = ((hash<<5)-hash)+char;
        hash = hash & hash; // Convert to 32bit integer
    }
    return hash;
}

@crookse
Copy link
Member Author

crookse commented Apr 16, 2020

In Drash's DigitalOcean setup, the following benchmarks were received compared to Express:

Express

Running 15s test @ http://drash.io/
  8 threads and 64 connections
  Thread Stats   Avg      Stdev     Max   +/- Stdev
    Latency    26.63ms   24.93ms 306.60ms   94.85%
    Req/Sec   346.06     76.06   430.00     88.47%
  40836 requests in 15.06s, 9.28MB read
  Socket errors: connect 0, read 32, write 0, timeout 0
Requests/sec:   2711.33
Transfer/sec:    630.63KB

----

Running 15s test @ http://drash.io/
  8 threads and 40 connections
  Thread Stats   Avg      Stdev     Max   +/- Stdev
    Latency    24.06ms   15.08ms 243.30ms   93.94%
    Req/Sec   221.66     52.10   280.00     87.01%
  26486 requests in 15.10s, 6.02MB read
Requests/sec:   1753.96
Transfer/sec:    407.95KB

Drash

Running 15s test @ http://drash.io/
  8 threads and 64 connections
  Thread Stats   Avg      Stdev     Max   +/- Stdev
    Latency    26.66ms   25.99ms 316.33ms   94.86%
    Req/Sec   349.68     79.24   430.00     88.07%
  41342 requests in 15.08s, 5.72MB read
  Socket errors: connect 0, read 55, write 0, timeout 0
Requests/sec:   2741.49
Transfer/sec:    388.66KB

----

Running 15s test @ http://drash.io/
  8 threads and 40 connections
  Thread Stats   Avg      Stdev     Max   +/- Stdev
    Latency    23.32ms   15.74ms 266.64ms   96.03%
    Req/Sec   229.95     42.15   282.00     90.96%
  27312 requests in 15.10s, 3.78MB read
  Socket errors: connect 0, read 16, write 0, timeout 0
Requests/sec:   1809.21
Transfer/sec:    256.48KB

Both frameworks seem to share the same performance. The benchmark tool tested the "Hello World!" applications of both frameworks.

@ebebbington
Copy link
Member

@crookse The two tests for each framework: is one for using the ETag header and one isn't? If so it is a dramatic decrease in speed :/

Though i believe Deno has the correct hashing algo's now to aid in this (which I know aren't needed)

@crookse
Copy link
Member Author

crookse commented Jun 3, 2020

@ebebbington no that was just using a reverse proxy with varying number of connections. Use of the etag header should be optional via server config

@crookse
Copy link
Member Author

crookse commented Jun 3, 2020

Thinking about it now.... This could probably be solved by middleware. The middleware could check if the header exists and compare the header against the response. If they match then the middleware returns a 304.

Also, yes there was a decrease in speed because I deployed the applications to the smallest digital ocean server instance

@diveDylan
Copy link

Http Cache (ETag) is something that we indeed need in most App. Hope update for support etags

@ebebbington
Copy link
Member

@diveDylan I'm going to try work on this as a middleware at deno-drash-middleware when I have the chance - though I don't know if you have experience with this and wanted to write the implementation yourself inside that repo? Up to you, just thought i'd ask :)

FYI going to move this issue over to the middleware repo (link above) now but will keep this issue open until it's done (unless @crookse thinks otherwise to keep this issue open)

@diveDylan
Copy link

@ebebbington i have no experience with this, but really want to try on this O(∩_∩)O~

@ebebbington
Copy link
Member

@diveDylan Aha me neither to be honest!:P So feel free! The issue is here: https://github.com/drashland/deno-drash-middleware/issues

I working on other stuff atm so I wouldn’t be able to start it straight away anyways, unfortunately

Just pop a comment in the issue and I’ll assign it :)

@ebebbington
Copy link
Member

I think this might work best as middleware?

@crookse
Copy link
Member Author

crookse commented Oct 29, 2020

prolly

@crookse crookse changed the title Response ETag Header feat: Response ETag Header Oct 31, 2020
@crookse crookse changed the title feat: Response ETag Header Response ETag Header Nov 1, 2020
@crookse
Copy link
Member Author

crookse commented Nov 4, 2020

@diveDylan, i'm moving this to the middleware repository. we figure this will be best as middleware so as not to bloat the deno-drash codebase

@crookse crookse transferred this issue from drashland/drash Nov 4, 2020
@crookse
Copy link
Member Author

crookse commented Nov 4, 2020

@diveDylan, closing this since we already have an etag midldeware assigned to you in this repo. see #8

@crookse crookse closed this as completed Nov 4, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

No branches or pull requests

4 participants