hasura / graphql-engine

Blazing fast, instant realtime GraphQL APIs on your DB with fine grained access control, also trigger webhooks on database events.
https://hasura.io
Apache License 2.0
31.07k stars 2.76k forks source link

Error when restarting hasura instance pointed to database pool connection when using prepared statements #7741

Open BenoitRanque opened 2 years ago

BenoitRanque commented 2 years ago

Version Information

Server Version: 2.0.10-cloud.1

Environment

Tested on Cloud, probably an issue on all platforms

What is the expected behaviour?

Hasura should be able to connect to database connection pools. Alternatively, if hasura expects to be connected directly to databases when using prepared statements, it should be documented.

Keywords

Hasura, Cloud, Prepared Statement, Inconsistency, Digital Ocean, PGBouncer

What is the current behaviour?

When a hasura instance that is connected to a database via a PGBouncer connection pool is restarted, the following errors will be shown in console and the instance enters an inconsistent state: image

(error text for searchability)

{
  "statement": "BEGIN ISOLATION LEVEL READ COMMITTED READ ONLY",
  "prepared": true,
  "error": {
    "exec_status": "FatalError",
    "hint": null,
    "message": "prepared statement \"0\" already exists",
    "status_code": "42P05",
    "description": null
  },
  "arguments": []
}

How to reproduce the issue?

  1. Create a digital ocean postgres database
  2. Create a connection pool for it. Use pool mode transaction, which is default & recomended
  3. Create a hasura cloud project, and configure it to connect to the database via the connection pool.
  4. Open console, and make a change that will cause the database connection to be re-established
  5. (in my case I changed the environment variable to an invalid one that did not exists, then changed it back)
  6. You will now see the error mentioned above To the team: I have setup a project which has this issue and can provide access to it.

Screenshots or Screencast

image image

Any possible solutions?

Connecting to the database directly solves the problem

Can you identify the location in the source code where the problem exists?

The error seems to indicate that hasura is attempting to create a stored procedure that already exists. This leads me to think that stored procedures outlive the hasura connection to the pgbouncer pool, and thus still exist when hasura re-establishes connection with the database.

I do not know if this is a bug of hasura, pgbouncer, or both.

tirumaraiselvan commented 2 years ago

In transaction pooling mode, prepared statements cannot be used: https://www.pgbouncer.org/features.html

Hence, if transaction pooling mode is desired then prepared statements should be disabled via use_prepared_statements field in source configuration.

shahidhk commented 2 years ago

Okay, we do have this mentioned in our DigitalOcean getting started guide: https://hasura.io/docs/latest/graphql/core/deployment/deployment-guides/digital-ocean-one-click.html#connection-pooling

Should we also have this elsewhere probably and also convert this to a discussion so that this can be searchable?

BenoitRanque commented 2 years ago

Folks a question: I understand that

if transaction pooling mode is desired then prepared statements should be disabled

But what should the user do, for a typical use case? Given that hasura will use pools internally? In the ticket that lead to this issue being created, the user was connecting to the pool they had created mostly for other software to connect to the database with.

Should we recommend that, where possible, the user connect hasura directly to the db, and use the pool for other software to connect to the database?

robx commented 2 years ago

@BenoitRanque I think there's some confusion here -- it may be on my side trying to parse the discussion, apologies if so. Anyway, trying to clarify:

graphql-engine's internal connection pooling should handle prepared statements just fine, assuming the database endpoint supports them. Whether or not it tries to use prepared statements depends on the data source setting, e.g. "use_prepared_statements": true.

So I think what we should recommend to the user is, "if you're running pgbouncer, disabled use_prepared_statements", which is what @tirumaraiselvan said above.

BenoitRanque commented 2 years ago

@robx I was actually wondering if we should recommend that the user not use an external proxy with connection pooling, and instead prefer using hasura's pooling, which, combined with prepared statements, would give better performance than the alternative (an external pool and no prepared statements)

robx commented 2 years ago

Ah, now I understand, thank you. That's a good question... I'm sure we'd have benchmarked with vs without use_prepared_statements before but I'm not aware of such numbers. I'll ask for input.