(Connection Pool) Error: prepared statement "p1" already exists

[joshxyzhimself]

Heya, really enjoying this project.

When I switch to Connection Pool in DigitalOcean PostgreSQL 11, I get this error:

Error: prepared statement "p1" already exists
    at Object.generic (C:\cygwin64\home\1234\transac\node_modules\postgres\lib\types.js:200:5)
    at Object.ErrorResponse (C:\cygwin64\home\1234\transac\node_modules\postgres\lib\backend.js:124:33)
    at TLSSocket.data (C:\cygwin64\home\1234\transac\node_modules\postgres\lib\connection.js:167:25)

Any thoughts?

[joshxyzhimself]

Alternative approach I did is not use connection pools but instead set options.max to 22 (DigitalOcean limit on my db tier) from 10 (default here). But no idea still what the cause of above error is.

[porsager]

Ah interesting. Thanks for the report, I'll look into..

[tracker1]

just a thought, if it's using a name for prepared statements, maybe have the name generated by like an md5 hash of the statement's body... pg_${hash.toString(16)} or similar?

[porsager]

Prepared statements are connection specific which is why an incrementing value is fine. This must be an internal collision happening in Postgres.js.

There would be a performance issue in calculating a hash, and we already get unique query identifiers through the template strings :)

[joshxyzhimself]

Also got this separate error when doing a bunch of actions on user-side consecutively (one after another):

Fri, 28 Feb 2020 05:34:53 GMT Error posting comment. Error: remaining connection slots are reserved for non-replication superuser connections
    at Object.generic (/Users/blitz/Documents/transac/node_modules/postgres/lib/types.js:200:5)
    at Object.ErrorResponse (/Users/blitz/Documents/transac/node_modules/postgres/lib/backend.js:125:22)
    at TLSSocket.data (/Users/blitz/Documents/transac/node_modules/postgres/lib/connection.js:167:25)
    at TLSSocket.emit (events.js:305:20)
    at addChunk (_stream_readable.js:341:12)
    at readableAddChunk (_stream_readable.js:316:11)
    at TLSSocket.Readable.push (_stream_readable.js:250:10)
    at TLSWrap.onStreamRead (internal/stream_base_commons.js:186:23) {
  severity_local: 'FATAL',
  severity: 'FATAL',
  code: '53300',
  file: 'postinit.c',
  line: '789',
  routine: 'InitPostgres'
}

^ Not using DigitalOcean Connection Pool.

My instance:

const sql = postgres('postgresql://PostgresUsername:PostgresPassword@DOHostname:Port/DatabaseName?sslmode=require', {
  max: 22,
  ssl: { ca },
});

I could be wrong but it appears like it is consuming that much connections and not releasing them?

---

Update on this:

I was getting this error when I was on my Mac + 4g connection office.

When I tried it in prod, I wasn't getting any of these as of the moment.

[porsager]

Ok, so looking a bit closer at the Digital Ocean setup, it appears you can enable further connection pooling using pgbouncer. I would guess that you have that enabled @joshxyzhimself resulting in clashing prepared statements names between the connections created using Postgres.js going through pgbouncer.

That means your suggestion @tracker1 was on point, and I apologize for brushing it off too quickly, but I think it would have to be a random unique ID instead of a hash, because that would equally be able to clash with already created statements from other connections with the same hash. It would require Postgres.js to have extra logic and overhead in first asking the DB if a prepared statement with that hash existed when trying to create it.

I will most likely add in unique ids for prepared statements to solve this in the upcoming major.

[tracker1]

A uuid or really just a series of say 16-24 bytes (to hex) from crypto random is probably sufficient, since it's per connection. Not sure how valuable the reuse would be in terms of a hash though... chances of natural collisions are probably really low.

[porsager]

The problem with a hash (checksum) still suffers from collisions if I don't add a check before creating the prepared statement. There will be two processes sharing the same pg_bouncer connection trying to create the same prepared statement, so I definitely think the unique random id is the way to go...

[tracker1]

Oh, I do understand that, was only suggesting adding the check may be better, since it may optimize the query path better on the server, or keep it active? I don't know the inner details of pg though, so maybe it's not worth it or worse than having the same query in two different, or many different prepared statements with the same query.

[porsager]

Ah ok great :) Yeah, the prepared statements are cheap, and much better handled by the planner in future versions, so I would think it's not a problem to have multiple prepared statements even though they are for the same queries.