sequelize / sequelize/sequelize

[BUG] Unable to reacquire connection on socket drop when using pg-native

Open
#14,086 1 comment 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
TypeScript
Stars
30.4k
Forks
4.3k
Avg merge
1d 6h
Merged PRs (30d)
68

Description

Hello,

we have been experiencing issues in cloud every single time when ALB rebooted and all connections in the cluster were terminated.

I have analyzed the code and found that the issue occurs for everyone who is using pg-native and it is due to this line in the code
https://github.com/sequelize/sequelize/blob/main/src/dialects/postgres/query.js#L88

However if socket drops, the error which is returned by pg-native will either be 'Unable to set non-blocking to true' or error returned directly by PostgreSQL
https://github.com/brianc/node-pg-native/blob/master/index.js#L283

Since
connection._invalid = true;
is not set for these cases, it will never re-acquire new connection since it thinks socket is still alive. What is even worse, if someone defines retry logic like we did it cases an infinite loop

 retry: {
            match: [
                ConnectionError,
                ConnectionRefusedError,
                ConnectionTimedOutError,
                HostNotFoundError,
                HostNotReachableError,
                InvalidConnectionError,
                /SSL SYSCALL error: EOF detected/i, // Match SSL SYSCALL error: EOF detected
                /Local: Authentication failure/i, // Match Local: Authentication failure
                /Unable to set non-blocking to true/i, // Match: non-blocking to true error
            ],
            backoffBase: 100, // Initial backoff duration in ms : default 100
            backoffExponent: 1.1, // Exponent to increase backoff each retry : default 1.1
            timeout: 55000,
            max: Infinity, // Max no: of retries
        },

I was not sure how to exactly check from Query whether we are using native or not, but I will share pull request to incorporate two errors which will get triggered by either pg-native or PostgreSQL which resolve this issue.

 try {
      queryResult = await query;
    } catch (error) {
      // set the client so that it will be reaped if the connection resets while executing
       if (err.code === "ECONNRESET" || 
          RegExp(/SSL SYSCALL error: EOF detected/i).test(error) || 
          RegExp(/Unable to set non-blocking to true/i).test(error) ||
          RegExp(/Local: Authentication failure/i).test(error)
      ) {
        connection._invalid = true;
      }
      error.sql = sql;
      error.parameters = parameters;
      throw this.formatError(error, errForStack.stack);
    }

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start at src/dialects/postgres/query.js around line 88 and compare the socket-drop handling with pg-native's index.js around line 283. Reproduce the reported native-driver errors and verify that the connection is reacquired without an infinite retry loop; add or update the relevant query tests if the repository provides them.

Written by the indexing model from the issue text.

Assessment

Tech stack
nodejs, postgresql, typescript
Domain
backend, database
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
32/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.