-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Feature: Memory limits #2336
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
Comments
Are you concerned with a single row / field being too large or an entire result set (returned as an array) being too large? If it's the total result set, have you considered using a cursor? That would let you operate on the row in chunks without needing to allocate memory for the entire result set. If you have commands that could return an arbitrary / huge number of rows, it's the way to go to ensure you do not blow up the node process memory usage. |
Both. Yes, I could use a cursor. But in this case I'm not interested in the complexity of streaming the results, or even getting large results set to work. Success for me would simply be large results sets not crashing the process. It's the same rationale as limiting HTTP message size. Yes, you could stream the HTTP body and carry that streaming through the rest of the app. However that is complex. Libraries can be configured to just cancel/error the operation for bodies over a certain size. Likewise, I have a server making PostgreSQL queries. If the response is ever too large, the entire server crashes. Instead, I'd like to cancel/error the specific offending operation. |
Yup, 100% this is the way to handle this! The postgres protocol supports this, and node is obvs build w/ streaming in mind. This library already exists as well. We use pg and regularly stream 500-600 megabytes of results down to clients w/o issue using cursors. As far a single row taking up more memory than your server...you should fix your data in that case. 🤷 One major difference between a db client & a web server & why body parser accepts a limit is a web server needs to protect you from bad actors sending you gigabytes in a post request and DDoSing you. Generally, you control both your web server & database client, so you are just DDoSing yourself in this case. Even if the postgres client threw an error (and forcibly disconnected) the database server has already gone through the work of shoving hundreds of megabytes into the network socket. |
If you find a use case for "I want to close the connection, if the database returned more than N bytes", it might be possible to implement something like this outside the library: node-postgres/packages/pg/lib/connection.js Line 107 in f0fc470
You would need to attach a If you use the pool, you could set this up in the pool's |
That is the ideal way. But now everything for the entire operation has to support streaming, e.g. streaming XML or JSON.
There are a lot of assumptions going on here.
These assumptions do not apply universally, nor is there is an fundamental reason why they should. There's more in the world than a database-backed webserver. It's very common, in fact, best practice, for I/O applications of all kinds to limit incoming payloads to prevent crashes. PostgreSQL itself (despite -- as you say -- usually being controlled by the same party as the client) has limits for incoming data. I see no reason why PostgreSQL client (particularly one with a runtime unable to respond to malloc() errors) can't protect itself in the same way. This is change that is remarkably easy to add, but would be somewhat hairy to implement via a plugin. |
Please feel free to submit a pull request with proper test coverage & I'll be happy to take a look. |
I'd like a large result to produce an Error (which can be handled), rather than crashing the entire process with OutOfMemory.
This is similar in nature to limit I/O for HTTP requests:
I imagine an option to Connection/Pool like
maxResultSize
. When the result exceeds the configured number of bytes, an Error is thrown. (I understand this limit would be subject to the choice of binary and text formats...this isn't super precise, just a way to gracefully handle unexpectedly large results.)This would significantly improve the stability of multi-operation processes using this library.
The text was updated successfully, but these errors were encountered: