-
-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Create aws-lambda.md #12642
Create aws-lambda.md #12642
Conversation
Codecov Report
@@ Coverage Diff @@
## master #12642 /- ##
==========================================
- Coverage 96.32% 95.85% -0.47%
==========================================
Files 95 92 -3
Lines 9261 9057 -204
Branches 90 90
==========================================
- Hits 8921 8682 -239
- Misses 323 358 35
Partials 17 17
Continue to review full report at Codecov.
|
I have updated the guide with a simpler solution and more in-depth details for people that may be interested in diving deeper (diff). |
I have updated the guide fixing some typos and other minor clarifications (diff). |
Thanks for this document it's very useful. |
Hi @amitaymolko:
Since connection pooling is not used across invocations, the settings are not as important as before. For most cases the default settings should be OK: {
"max": 5,
"min": 0,
"idle": 10000,
"acquire": 60000,
"evict": 1000,
} However you can optimize on a few things:
I'll probably update the pull request with these recommendations. |
@marcogrcr Really appreciate the time & thought put into this writeup! I ran into the issue today when setting up a serverless framework API, and noted that the
I believe it's because calling
I got around this by writing a function to do the same things, but without rewriting const closeConnection = async () => {
await sequelize.connectionManager.pool.drain()
return sequelize.connectionManager.pool.destroyAllNow()
} and calling it at the end of each handler in lieu of My understanding is that this setup closes & then reinstantiates the If anyone reading this sees a flaw with that approach, or something I may have misunderstood with the original writeup, please let me know! edit: this is using sequelize 6.3.5 |
@johnwquarles I can confirm your code works for me, thanks! |
@johnwquarles you're right about that. As a disclaimer, I have not had the opportunity run the TL;DR code on my project yet. When I analyzed that code, I got confused for a moment and thought the The code that you're proposing should do the trick since it's the equivalent of v6.3.5 sequelize/lib/dialects/abstract/connection-manager.js Lines 110 to 117 in 933b3f6
sequelize/lib/dialects/abstract/connection-manager.js Lines 94 to 103 in 933b3f6
v5.22.3 sequelize/lib/dialects/abstract/connection-manager.js Lines 117 to 124 in 3d2df28
sequelize/lib/dialects/abstract/connection-manager.js Lines 101 to 110 in 3d2df28
The only downside I see with this code is that v6.3.5 sequelize/types/lib/connection-manager.d.ts Lines 14 to 34 in 933b3f6
v5.22.3 sequelize/types/lib/connection-manager.d.ts Lines 16 to 36 in 3d2df28
This means that:
As an alternative to accessing internals, you can: const { Sequelize } = require("sequelize");
let sequelize = null;
module.exports.handler = async function (event, callback) {
// re-use the sequelize instance across invocations to improve performance
if (!sequelize) {
sequelize = await loadSequelize();
} else {
// restart connection pool to ensure connections are not re-used across invocations
sequelize.connectionManager.initPools();
// restore `getConnection` if it has been overwritten
if (sequelize.connectionManager.hasOwnProperty("getConnection")) {
delete sequelize.connectionManager.getConnection;
}
}
try {
return await doSomethingWithSequelize(sequelize);
} finally {
// close any opened connections during the invocation
// this will wait for any in-progress queries to finish before closing the connections
await sequelize.connectionManager.close();
}
}; I guess the ideal solution would be to have |
@amitaymolko / @johnwquarles : Thanks to your feedback, I have created a new revision (see diff here). |
Hello everyone!! Thank you so much @marcogrcr for the detailed explanation! I will not merge yet because I want to take a closer look first, and I haven't had much time, please wait a little more and thank you very much!! 🚀 |
Hello @papb Just checking if you've had a chance to take a look at this pull request? It seems like various people are taking notice of it and many more would benefit from this being published in https://sequelize.org/. Would you be able to share an ETA of when do you think someone from the |
Hey @papb just pinging here again to see if can find a chance to look at this PR. I understand that you're probably busy with other priorities, but it's been more than a year since your update 😬 |
Hey there :) First of all: Thanks a bunch for your contribution to Sequelize! Much appreciated! A couple of months ago, we have switched from master to main branch as our primary development branch and hence this PR is now outdated :( If you still think this change is making sense, please consider recreating the PR against ✌️ |
The base branch was changed.
This document aims to guide developers into how to use sequelize in AWS Lambda. Properly configuring sequelize in AWS Lambda is a common source of pain for developers.
Hey @sdepold! I have updated the pull request branch and rebased the commit as requested. |
Hey @sdepold: Can we re-open this pull-request? I rebased the branch on top of |
Haven't tried it but you have my trust :) |
* feat(typescript): create alpha release with ts * ci: add other ts versions to ci (#13686) * fix: wrong interface used within mixin (#13685) * fix(increment): fix key value broken query (#12985) Co-authored-by: Sascha Depold <[email protected]> * fix(upsert): fall back to DO NOTHING if no update key values provided (#13863) * fix(types): add instance member declaration (#13684) * fix(types): add instance member declaration * test(types): add static/instance members test cases Co-authored-by: Sascha Depold <[email protected]> * Create aws-lambda.md (#12642) * fix(docs): add aws-lamda route (#13693) * fix(types): allow override json function with custom return type (#13694) * fix(types): allow override to json function with custom return type * fix(types): remove automatic linter changes Co-authored-by: sander-mol <[email protected]> * ci(docs): add doc generation to checks (#13704) * docs: add logo (#13700) * docs: add logo * fix(docs): logo not show up (#13699) * fix(build): markdownlint * docs(readme): use internal link * docs(index.md): use internal link * docs(index): update logo rendering in docs * Center logo and headline Co-authored-by: Sascha Depold <[email protected]> Co-authored-by: Sascha Depold <[email protected]> * test: fix mocha (#13707) Co-authored-by: Rik Smale <[email protected]> Co-authored-by: Sascha Depold <[email protected]> * test: fix failing stack trace test (#13708) * test: fix failing tests (#13709) * test: fix failing tests due to minification Removes esbuild minification which was causing tests to fail and some changed behavior. * Revert "fix(upsert): fall back to DO NOTHING if no update key values provided (#13863)" This reverts commit 4071378. * fix(upsert): fall back to DO NOTHING if no update key values provided (#13711) * fix(upsert): fall back to DO NOTHING if no update key values provided (#13863) * fix: remove erroneous .length in _.isEmpty * refactor: use includes instead of or operators (#13706) * docs(contributing): add section on adding/updating deps (#13715) * docs(contributing): add Node versions Fixes #13714 * docs(contributing): add section on adding/updating deps * fix(types): add Col to where Ops (#13717) * fix(types): add Col to where Ops * fix(types): tests * fix(data-types): unnecessary warning when getting data with DATE dataTypes (#13712) * fix(data-types): unnecessary error when getting data with DATE dataTypes * fix(data-types): date stringify mariadb * fix(types): add missing schema field to sequelize options Fixes #12606 Co-authored-by: Sascha Depold <[email protected]> * fix(example): fix coordinates format as per GeoJson (#13718) * fix(example): fix coordinates format as per GeoJson * Update data-types.js Co-authored-by: Sascha Depold <[email protected]> * ci(node): use Node 16 instead of 12 (#13703) * ci(node): add Node 14 and 16 to DB tests * ci(node): move linting, test typing and release to Node 16 * ci(docs): use Node 16 for docs * ci(node): run tests on Node 10 and 16 Co-authored-by: Rik Smale <[email protected]> * refactor(postgres): move `clientMinMessages` from general to pg options (#13720) * refactor(postgres): move `clientMinMessages` from general to pg options * refactor(postgres): address review comments * refactor(postgres): address review comments * refactor(postgres): fix pipeline * fix(dialect): try to fix flaky test * fix(dialect): try to fix flaky test Co-authored-by: Jesse Peng <[email protected]> * refactor(build): use rm instead of rmdir for Node 14 and up (#13702) * fix(build): refactor rmdir to rm * refactor(build): only use rm in Node 14 and up * refactor(build): move consts inside function * refactor(build): fix definition Co-authored-by: Rik Smale <[email protected]> Co-authored-by: Sascha Depold <[email protected]> * docs(postgres): warn about deprecated clientMinMessage option (#13727) * docs(postgres): warn about deprecated clientMinMessage option * docs(deprecation-warning): fix typo in deprecation warning * docs(deprecation-warning): fix typo in deprecation warning * meta(deps): upgrade (dev)deps (#13729) Co-authored-by: Rik Smale <[email protected]> * test(integration): mark and fix flaky test (#13735) * feat(dialect): snowflake dialect support (#13406) Co-authored-by: Mohamed El Mahallawy <[email protected]> Co-authored-by: bparan <[email protected]> Co-authored-by: Matthew Blasius <[email protected]> Co-authored-by: WeRDyin <[email protected]> Co-authored-by: Marco Gonzalez <[email protected]> Co-authored-by: Constantin Metz <[email protected]> Co-authored-by: Sander Mol <[email protected]> Co-authored-by: sander-mol <[email protected]> Co-authored-by: Rik Smale <13023439 [email protected]> Co-authored-by: Fauzan <[email protected]> Co-authored-by: Rik Smale <[email protected]> Co-authored-by: AllAwesome497 <47748690 [email protected]> Co-authored-by: manish kakoti <[email protected]> Co-authored-by: Marco Kerwitz <[email protected]> Co-authored-by: Jesse Peng <[email protected]> Co-authored-by: Jesse Peng <[email protected]>
🎉 This PR is included in version 6.12.0-beta.1 🎉 The release is available on: Your semantic-release bot 📦🚀 |
🎉 This PR is included in version 6.12.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Hello @marcogrcr "Since connection pooling is not used across invocations, the settings are not as important as before. For most cases the default settings should be OK" Here (#4938 (comment)) you also wrote: "Keep in mind that AWS has released a product for AWS Lambda called Amazon RDS Proxy where you don't have to worry about connection pooling and you can create a new connection for every request." How do you suggest we build our "lambda.handler"? Followed the guide here (https://sequelize.org/docs/v6/other-topics/aws-lambda/) but we continue to experience "SequelizeConnectionAcquireTimeoutError" from time to time. I even removed the close() and initPool() because seems unnecessary by your comments. And RDS connections to the DB seems to be stacking to the limit of out current allowance. Thank you for the time you put doing this. |
Hi all, thanks for all the work here. I'm still having an issue with sequelize (^6.19.0) and lambda (nodejs12.x) w/ RDS Aurora Postgres. We've implemented the code as described in the lambda doc, and are now seeing new error when a lambda function is invoked after "re-connecting" using the same sequelize instance (e.g. after calling the The symptoms of the error:
For more context, our entire API is implemented using a single lambda function, so we're reusing containers at a pretty high frequency. Also, here's our sequelize config. I have played with many configurations (including using the config in the doc), and have not found any solutions, so I currently have it stripped down. Anything here that could be the problem?
TIA! |
This document aims to guide developers on how to use
sequelize
in AWS Lambda. Properly configuringsequelize
for AWS Lambda is a common source of pain for developers.