Skip to content

feat(core): Add config to override default database ping interval and default idle connection timeout #15764

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

guillaumejacquart
Copy link
Contributor

Summary

When using postgres or mysql/mariadb as database, typeorm uses a connection pool. When the dabatase password for the current user changes (password rotation for instance), the db-connection ping function that checks the liveness of the db will most likely re-use an existing connection from the pool, that is kept and not refresh when user db password changes. Thus the ping will succeeded, and n8n will still be up even though new connection from the pool will fail.

To fix that, I suggest to offer the possibility to change the database ping interval as well as the idle connection timeout for a connection in the pool.

That way, a user could setup the idle connection timeout to be less than the ping connection timeout, so that a new connection is created for each ping, thus re-validating the credentials.

We should see later if we want this behavior (each ping connection to be new) to be by default.

Related Linear tickets, Github issues, and Community forum posts

https://linear.app/n8n/issue/PAY-2846/community-issue-endpoint-healthzreadiness-returning-statusok-even-when
closes #15480

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)

Copy link

codecov bot commented May 27, 2025

❌ 1 Tests Failed:

Tests completed Failed Passed Skipped
5240 1 5239 0
View the top 1 failed test(s) by shortest run time
GlobalConfig should use values from env variables when defined
Stack Traces | 0.007s run time
Error: expect(received).toEqual(expected) // deep equality

- Expected  - 0
+ Received  + 1

@@ -37,10 +37,11 @@
        "host": "localhost",
        "password": "",
        "port": 3306,
        "user": "root",
      },
+     "pingIntervalSeconds": 2,
      "postgresdb": Object {
        "connectionTimeoutMs": 20000,
        "database": "n8n",
        "host": "some-host",
        "idleTimeoutMs": 30000,
    at Object.<anonymous> (.../n8n/n8n/packages/@.../config/test/config.test.ts:333:35)
    at Promise.then.completed (.../n8n/node_modules/.pnpm/[email protected]..../jest-circus/build/utils.js:300:28)
    at new Promise (<anonymous>)
    at callAsyncCircusFn (.../n8n/node_modules/.pnpm/[email protected]..../jest-circus/build/utils.js:233:10)
    at _callCircusTest (.../n8n/node_modules/.pnpm/[email protected]..../jest-circus/build/run.js:315:40)
    at processTicksAndRejections (node:internal/process/task_queues:95:5)
    at _runTest (.../n8n/node_modules/.pnpm/[email protected]..../jest-circus/build/run.js:251:3)
    at _runTestsForDescribeBlock (.../n8n/node_modules/.pnpm/[email protected]..../jest-circus/build/run.js:125:9)
    at _runTestsForDescribeBlock (.../n8n/node_modules/.pnpm/[email protected]..../jest-circus/build/run.js:120:9)
    at run (.../n8n/node_modules/.pnpm/[email protected]..../jest-circus/build/run.js:70:3)
    at runAndTransformResultsToJestFormat (.../n8n/node_modules/.pnpm/[email protected]..../build/legacy-code-todo-rewrite/jestAdapterInit.js:122:21)
    at jestAdapter (.../n8n/node_modules/.pnpm/[email protected]..../build/legacy-code-todo-rewrite/jestAdapter.js:79:19)
    at runTestInternal (.../n8n/node_modules/.pnpm/[email protected]..../jest-runner/build/runTest.js:367:16)
    at runTest (.../n8n/node_modules/.pnpm/[email protected]..../jest-runner/build/runTest.js:444:34)
    at Object.worker (.../n8n/node_modules/.pnpm/[email protected]..../jest-runner/build/testWorker.js:106:12)

To view more test analytics, go to the Test Analytics Dashboard
📋 Got 3 mins? Take this short survey to help us improve Test Analytics.

@n8n-assistant n8n-assistant bot added core Enhancement outside /nodes-base and /editor-ui n8n team Authored by the n8n team labels May 27, 2025
@guillaumejacquart guillaumejacquart force-pushed the pay-2846-community-issue-endpoint-healthzreadiness-returning-statusok branch from 5f37a5c to f60889a Compare May 28, 2025 07:34
…e connection timeout of postgres database (to release a connection from the pool)
@guillaumejacquart guillaumejacquart force-pushed the pay-2846-community-issue-endpoint-healthzreadiness-returning-statusok branch from f60889a to 2f1db7a Compare May 28, 2025 07:35
@guillaumejacquart guillaumejacquart marked this pull request as ready for review May 28, 2025 10:02
Copy link

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cubic reviewed 6 files and found no issues. Review PR in cubic.dev.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Enhancement outside /nodes-base and /editor-ui n8n team Authored by the n8n team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

endpoint healthz/readiness returning status:ok even when database password is wrong
1 participant