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

Upstream prematurely closed connection while reading response header from upstream #1166

Closed
doender opened this issue May 16, 2018 · 8 comments

Comments

@doender
Copy link

doender commented May 16, 2018

Using the latest version of learning locker, I get the following nginx error when querying the /api/statements/aggregate endpoint.

upstream prematurely closed connection while reading response header from upstream

This only happens when the amount of data that is requested is large.

@doender
Copy link
Author

doender commented May 22, 2018

Is there any extra information I can give that would help in finding out where the problem lies?

@ht2
Copy link
Contributor

ht2 commented May 22, 2018

Hey @doender - this sounds like nginx timing out on the request. Likely problems:

  • Your query is not optimised
  • Your query does not use good indexes
  • Your query times out (scans too much data)

If I was you I would monitor the database queries and see if anything is taking too long (and why).

@ht2 ht2 closed this as completed May 22, 2018
@doender
Copy link
Author

doender commented May 22, 2018

@ht2 It could be that the query can be optimized, but we have set the appropriate indexes and as far as I understand MAX_TIME_MS and MAX_SCAN are set to unlimited by default (http://docs.learninglocker.net/guides-configuring/). Or is there another timeout set?

@ht2
Copy link
Contributor

ht2 commented May 22, 2018

What's the query, and looking at your query logs can you see things taking longer than expected (say 60s)? I would recommend using the following tools to understand exactly where you might be seeing issue:

https://gist.github.com/rantav/3433277

https://docs.mongodb.com/manual/reference/method/db.setProfilingLevel/

@doender
Copy link
Author

doender commented Nov 29, 2018

FYI, we're seeing this again but for another reason. In Node 8.14, URL length is limited to 8192 bytes: https://nodejs.org/en/blog/vulnerability/november-2018-security-releases/ ('Denial of Service with large HTTP headers') and nodejs/node@1860352

For this reason, calls to the aggregation API (where URLs can get quite long...) fail when long URLs are used (the UI service immediately closes the connection). We are now in process of downgrading to Node 8.13 because of this.

@ht2 I think it would make sense to encode the pipeline in the request body. A POST endpoint could exist alongside the current GET endpoint for backwards compatibility. I could make a PR if you want?

@ht2
Copy link
Contributor

ht2 commented Nov 30, 2018

Hey @doender - yea we've toyed with the idea of changing the aggregation to a POST endpoint (or at least adding one). Good spot on the 8.14 restriction. Up to now when we've hit this limitation it's been on the nginx side for us and we've simply upped the client buffers accordingly, but Node having the limitation makes this food for thought.

https://stackoverflow.com/questions/1067334/how-to-set-the-allowed-url-length-for-a-nginx-request-error-code-414-uri-too

@figureone
Copy link

Ok, looks like this was addressed in node 8.15.0 (and 6.16.0, 10.15.0):
nodejs/node#24811

They added a command line arg, --max-http-header-size, that allows customizing the new default of 8192 bytes.

@ht2, you should be able to add this param to xapi/pm2/xapi.json by adding "node_args": "--max-http-header-size=262144" (using the 256k value that's specified in nginx config in large_client_header_buffers) like so:

{
  "apps": [
    {
      "name": "xAPI",
      "script": "./dist/server.js",
      "cwd": ".",
      "output": "logs/xapi_stdout.log",
      "error": "logs/xapi_stderr.log",
      "pid": "pids/xapi.pid",
      "instances": 2,
      "exec_mode": "cluster",
      "wait_ready": true,
      "listen_timeout": 30000,
      "watch": [
        ".env"
      ],
      "node_args": "--max-http-header-size=262144",
      "max_memory_restart": "500M"
    }
  ]
}

https://futurestud.io/tutorials/pm2-how-to-start-your-app-with-node-js-v8-arguments

I haven't gotten a chance to test this yet though!

@figureone
Copy link

FYI I was able to get this to work by passing the argument directly to the pm2 restart command:

pm2 restart all --node-args="--max-http-header-size=262144"

Note that you need to be running Node 6.16.0, 8.15.0, 10.15.0, or above.
Also note that this should persist to the next restart command, even if you don't pass the argument:
Unitech/pm2#13 (comment)

I can't seem to get it working by modifying the pm2 json files, but that may be because the command line argument overrides anything set in the json config. Also, webapp/pm2/all.json may need the change, instead of xapi/pm2/xapi.json as mentioned above.

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