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

Add db-pool-query-wait-timeout config #2348

Closed
steve-chavez opened this issue Jun 24, 2022 · 1 comment
Closed

Add db-pool-query-wait-timeout config #2348

steve-chavez opened this issue Jun 24, 2022 · 1 comment
Labels
enhancement a feature, ready for implementation QOS

Comments

@steve-chavez
Copy link
Member

steve-chavez commented Jun 24, 2022

Problem

  • PostgREST doesn't have a timeout for how long a request should wait for obtaining a connection from the db pool.
  • Nginx(or other proxy) has a request timeout.

When a timeout occurs, this usually causes an error on the client side that has poor information: usually just a 502 Bad Gateway or 408 Request Timeout. This happens on servers with high amount of traffic.

The problem can be reproduced with an RPC that has a pg_sleep() inside plus Nginx with the req timeout.

(I haven't checked if the request will still reach the database once the proxy timeout occurs, it's possible)

Proposal

Have a db-pool-query-wait-timeout config that produces a timeout from our side, similar to the pgbouncer query_wait_timeout. The pgbouncer config has a default of 120 but in our case it should be lower since Nginx has a 60 second default. Maybe it should be like 50 seconds.

On the http side, when this happens we should return a 529 Site is overloaded status. Optionally with a Retry-After header, maybe we can use the pool waiter queue size to determine a good value.

Edit: For now maybe we should just keep it simple and return a 504 Gateway Timeout with a descriptive error message.


This would require a patch in hasql-pool.

@steve-chavez steve-chavez added QOS idea Needs of discussion to become an enhancement, not ready for implementation dependency-patch labels Jun 24, 2022
@steve-chavez steve-chavez added enhancement a feature, ready for implementation and removed idea Needs of discussion to become an enhancement, not ready for implementation labels Jul 8, 2022
@steve-chavez
Copy link
Member Author

steve-chavez commented Jul 8, 2022

To Reproduce

Start a postgrest with a sleep function:

nix-shell

cat << 'EOF' >> test/spec/fixtures/schema.sql

CREATE OR REPLACE FUNCTION sleep() returns void as $$
  select pg_sleep(50);
$$ language sql;

EOF

PGRST_DB_POOL=1 PGRST_DB_ANON_ROLE='postgrest_test_anonymous' postgrest-with-postgresql-14 postgrest-run

On other terminals do:

# take the single pool connection
curl 'localhost:3000/rpc/sleep'
# This one will wait 50 seconds until the sleep request finishes
curl 'localhost:3000/projects'

@steve-chavez steve-chavez reopened this Jul 8, 2022
robx added a commit to robx/postgrest that referenced this issue Aug 30, 2022
The configuration option db-pool-acquisition-timeout
specifies the time in seconds to wait for the pool to
free up a connection slot. Otherwise, a 504 error is
returned. By default, there is no timeout.
robx added a commit to robx/postgrest that referenced this issue Aug 31, 2022
The configuration option db-pool-acquisition-timeout
specifies the time in seconds to wait for the pool to
free up a connection slot. Otherwise, a 504 error is
returned. By default, there is no timeout.
robx added a commit to robx/postgrest that referenced this issue Aug 31, 2022
The configuration option db-pool-acquisition-timeout
specifies the time in seconds to wait for the pool to
free up a connection slot. Otherwise, a 504 error is
returned. By default, there is no timeout.
@robx robx closed this as completed in ba1fcfd Aug 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement a feature, ready for implementation QOS
Development

No branches or pull requests

1 participant