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

fix(Postgres Chat Memory Node): Do not terminate the connection pool #12674

Merged
merged 1 commit into from
Jan 17, 2025

Conversation

netroy
Copy link
Member

@netroy netroy commented Jan 17, 2025

Summary

Postgres connection pools aren't managed in individual nodes now, and are instead managed in the ConnectionPoolManager.
When the Postgres Chat Memory node calls pool.end(), it terminates the pool while the ConnectionPoolManager still holds a reference to it. This leads to the Postgres Chat Memory node being completely broken until the ConnectionPoolManager cleans up the reference to this pool. And if there are a lot of postgres calls on the instance, it's possible that the pool never gets cleaned up, and all postgres nodes using that specific credential are then broken indefinitely.

Related Linear tickets, Github issues, and Community forum posts

Fixes #12517
https://linear.app/n8n/issue/NODE-2240
https://community.n8n.io/t/error-using-postgres-chat-memory-and-supabase-for-ai-tools-agent/70792

Review / Merge checklist

  • PR title and summary are descriptive. (conventions)
  • Docs updated or follow-up ticket created.
  • Tests included.
  • PR Labeled with release/backport (if the PR is an urgent fix that needs to be backported)

@netroy netroy changed the title fix(Postgres Chat Memory): Do not terminate the connection pool fix(Postgres Chat Memory Node): Do not terminate the connection pool Jan 17, 2025
@netroy netroy added the release/backport Changes that need to be backported to older releases. label Jan 17, 2025
Copy link

codecov bot commented Jan 17, 2025

Codecov Report

Attention: Patch coverage is 0% with 1 line in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
...kages/nodes-base/nodes/Postgres/transport/index.ts 0.00% 1 Missing ⚠️

📢 Thoughts on this report? Let us know!

Copy link

cypress bot commented Jan 17, 2025

n8n    Run #8806

Run Properties:  status check passed Passed #8806  •  git commit 8edbbff20e: 🌳 🖥️ browsers:node18.12.0-chrome107 🤖 netroy 🗃️ e2e/*
Project n8n
Branch Review fix-MemoryPostgresChat
Run status status check passed Passed #8806
Run duration 04m 52s
Commit git commit 8edbbff20e: 🌳 🖥️ browsers:node18.12.0-chrome107 🤖 netroy 🗃️ e2e/*
Committer कारतोफ्फेलस्क्रिप्ट™
View all properties for this run ↗︎

Test results
Tests that failed  Failures 0
Tests that were flaky  Flaky 3
Tests that did not run due to a developer annotating a test with .skip  Pending 0
Tests that did not run due to a failure in a mocha hook  Skipped 0
Tests that passed  Passing 489
View all changes introduced in this branch ↗︎

Copy link
Contributor

✅ All Cypress E2E specs passed

@netroy netroy merged commit e7f00bc into master Jan 17, 2025
37 of 38 checks passed
@netroy netroy deleted the fix-MemoryPostgresChat branch January 17, 2025 13:58
This was referenced Jan 17, 2025
@janober
Copy link
Member

janober commented Jan 17, 2025

Got released with [email protected]

@g-roliveira
Copy link

Validated using 1.74.3 - It works!
Validated using 1.75.2 - It works!

Great job, guys!

@n8n-io n8n-io locked as off-topic and limited conversation to collaborators Jan 17, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
release/backport Changes that need to be backported to older releases. Released
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1.74.0 - PostgreSQL - Cannot use a pool after calling end on the pool
4 participants