Joystream / joystream

Joystream Monorepo
http://www.joystream.org
GNU General Public License v3.0
1.42k stars 115 forks source link

Disable express response validation in production #4810

Closed mnaamani closed 1 year ago

mnaamani commented 1 year ago

A potential source of delay in processing requests for assets from storage and distributor nodes is the OpenAPI response validation that is happening because the nodes are running in "development" mode.

Most operators are likely not setting their NODE_ENV=production at all. But even they did, Colossus has hard-coded true for response validation, and Argus only disables it if NODE_ENV=prod.

I tested disabling response validation on my distributor node. Clearly we see a significant jump in the number requests/s the node is able to handle once the validation is disabled.

With response validation: without-node_env-prod

Without validation: with-node_env-prod

For Colossus it might also make sense to disable request validation to avoid validating large file uploads?

Distributors can immediately test this out by adding NODE_ENV: prod under the environment: section of their docker-compose.yml.

Storage operators would have to pull down latest version once it is merged, and set env variable NODE_ENV=production

If this truly makes a difference we will not feel it until all distributors and storage nodes apply this.

mnaamani commented 1 year ago

Once distributors update to new verison you should change your NODE_ENV from prod to production so any other packages that maybbe also behaving in development mode, would switch to production and we may see more improvements (possibly).

mnaamani commented 1 year ago

Looks Good.

Please run yarn generate:docs:all in distributor-node directory to update the API documentation

will do.

Just curious how did you figure out that this was sort of bottleneck. Was this latency information present in Opentelemetry traces.

Actually nothing in the traces pointed at response validation being a bottleneck. I was just parsing the code and looking for something small to change that could have an impact, and from past experience I now if the response is large it could be a problem.

But I tested it after disabling validation and saw a large spike in request rate. But I'm starting to wonder if this was pure chance timing..

This is express-openapi-validator's validator function responsible for response validation. I am assuming it involves complex checks like regex matching, type checking, or nested validation for objects and arrays, which increases response times

Looking at the validation code, it looks like even if validation is enabled it seems it only does it if the content-type is JSON and skips otherwise. So It might in-fact not be having much of an affect.

mnaamani commented 1 year ago

@zeeshanakram3 I set distributor-node package version to 1.3.2 as there is another already published v1.3.1 as you know from https://github.com/Joystream/joystream/pull/4886

kdembler commented 1 year ago

oof and I just changed to 1.4.0 in my PR, we will need to merge this all somehow :D

mnaamani commented 1 year ago

oof and I just changed to 1.4.0 in my PR, we will need to merge this all somehow :D

Lets merge yours first, then I'll update this one to also be part of 1.4.0 before publishing docker image. WDYT?