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

Database connections can outlive SIGUSR1 reconfiguration #2401

Closed
robx opened this issue Aug 2, 2022 · 0 comments
Closed

Database connections can outlive SIGUSR1 reconfiguration #2401

robx opened this issue Aug 2, 2022 · 0 comments
Labels

Comments

@robx
Copy link
Contributor

robx commented Aug 2, 2022

The intended behaviour when receiving SIGUSR1 is for the PostgREST process to refresh the state of the database (reload schema cache); the new state is expected to take effect for any requests received from that point onwards.

E.g., in the sequence

  1. alter role authenticator set statement_timeout = ...
  2. kill -USR1 $postgrest
  3. curl http://postgrest/...
    the updated timeout should be in effect for the request in step 3.

To that aim, the signal handler "releases" the postgresql connection pool. However, with the present version of hasql-pool, releasing the pool merely destroys idle connections. Any busy connections survive, and can be returned to the pool at a later time.

Thus, we get e.g. the following failure scenario:

  • any time before step 2 above, issue a request to postgrest that results in a slow postgresql query (e.g. pg_sleep(...)); suppose this request uses postgresql connection C
  • suppose this request finishes between steps 2 & 3 above; C is returned to the pool
  • suppose the request of step 3 picks up connection C from the pool; C still has the old timeout setting, violating the expectation.

(This should be fixed during the upgrade to a newer release of hasql-pool #2391, where we have to touch the pool release logic anyhow. This issue is just for tracking the bug.)

robx added a commit to robx/postgrest that referenced this issue Aug 4, 2022
This version of hasql-pool is a simplified rewrite that doesn't use
the resource-pool package. The major API changes are that idle
connections are no longer timed out (and the corresponding setting
is gone), and that `release` makes the pool unusable, where it used
to remain usable and only flushed idle connections.

- This change removes the db-pool-timeout option, since new
  hasql-pool doesn't provide timing out of idle connections.
  Given that we were typically running with very high
  timeout settings, I don't anticipate the lack of timeout
  to introduce new issues, though we might want to consider
  introducing some retry-logic down the line when we
  encounter connection failures.
- To recover the ability to flush idle connections, we add a
  somewhat painful pool wrapper that allows replacing the
  pool. This fixes PostgREST#2401 by ensuring that flushing the pool
  also prevents any active connections from being reused in
  the future.
robx added a commit to robx/postgrest that referenced this issue Aug 4, 2022
…tgREST#2401)

This version of hasql-pool is a simplified rewrite that doesn't use
the resource-pool package. The major API changes are that idle
connections are no longer timed out (and the corresponding setting
is gone), and that `release` makes the pool unusable, where it used
to remain usable and only flushed idle connections.

- This change removes the db-pool-timeout option, since new
  hasql-pool doesn't provide timing out of idle connections.
  Given that we were typically running with very high
  timeout settings, I don't anticipate the lack of timeout
  to introduce new issues, though we might want to consider
  introducing some retry-logic down the line when we
  encounter connection failures.
- To recover the ability to flush idle connections, we add a
  somewhat painful pool wrapper that allows replacing the
  pool. This fixes PostgREST#2401 by ensuring that flushing the pool
  also prevents any active connections from being reused in
  the future.
robx added a commit to robx/postgrest that referenced this issue Aug 4, 2022
…tgREST#2401)

This version of hasql-pool is a simplified rewrite that doesn't use
the resource-pool package. The major API changes are that idle
connections are no longer timed out (and the corresponding setting
is gone), and that `release` makes the pool unusable, where it used
to remain usable and only flushed idle connections.

- This change removes the db-pool-timeout option, since new
  hasql-pool doesn't provide timing out of idle connections.
  Given that we were typically running with very high
  timeout settings, I don't anticipate the lack of timeout
  to introduce new issues, though we might want to consider
  introducing some retry-logic down the line when we
  encounter connection failures.
- To recover the ability to flush idle connections, we add a
  somewhat painful pool wrapper that allows replacing the
  pool. This fixes PostgREST#2401 by ensuring that flushing the pool
  also prevents any active connections from being reused in
  the future.
robx added a commit to robx/postgrest that referenced this issue Aug 4, 2022
…tgREST#2401)

This version of hasql-pool is a simplified rewrite that doesn't use
the resource-pool package. The major API changes are that idle
connections are no longer timed out (and the corresponding setting
is gone), and that `release` makes the pool unusable, where it used
to remain usable and only flushed idle connections.

- This change removes the db-pool-timeout option, since new
  hasql-pool doesn't provide timing out of idle connections.
  Given that we were typically running with very high
  timeout settings, I don't anticipate the lack of timeout
  to introduce new issues, though we might want to consider
  introducing some retry-logic down the line when we
  encounter connection failures.
- To recover the ability to flush idle connections, we add a
  somewhat painful pool wrapper that allows replacing the
  pool. This fixes PostgREST#2401 by ensuring that flushing the pool
  also prevents any active connections from being reused in
  the future.
robx added a commit to robx/postgrest that referenced this issue Aug 24, 2022
…tgREST#2401)

This version of hasql-pool is a simplified rewrite that doesn't use
the resource-pool package. The major API changes are that idle
connections are no longer timed out (and the corresponding setting
is gone), and that `release` makes the pool unusable, where it used
to remain usable and only flushed idle connections.

- This change removes the db-pool-timeout option, since new
  hasql-pool doesn't provide timing out of idle connections.
  Given that we were typically running with very high
  timeout settings, I don't anticipate the lack of timeout
  to introduce new issues, though we might want to consider
  introducing some retry-logic down the line when we
  encounter connection failures.
- To recover the ability to flush idle connections, we add a
  somewhat painful pool wrapper that allows replacing the
  pool. This fixes PostgREST#2401 by ensuring that flushing the pool
  also prevents any active connections from being reused in
  the future.
robx added a commit to robx/postgrest that referenced this issue Aug 24, 2022
This version of hasql-pool is a simplified rewrite that doesn't use
the resource-pool package. The major API changes are that idle
connections are no longer timed out (and the corresponding setting
is gone), and that `release` makes the pool unusable, where it used
to remain usable and only flushed idle connections.

We depend on a PostgREST fork of 0.7.2 that gives us reliable
flushing, compare PostgREST/hasql-pool#1

- This change removes the db-pool-timeout option, since new
  hasql-pool doesn't provide timing out of idle connections.
  Given that we were typically running with very high
  timeout settings, I don't anticipate the lack of timeout
  to introduce new issues, though we might want to consider
  introducing some retry-logic down the line when we
  encounter connection failures.
robx added a commit to robx/postgrest that referenced this issue Aug 24, 2022
This version of hasql-pool is a simplified rewrite that doesn't use
the resource-pool package. The major API changes are that idle
connections are no longer timed out (and the corresponding setting
is gone), and that `release` makes the pool unusable, where it used
to remain usable and only flushed idle connections.

We depend on a PostgREST fork of 0.7.2 that gives us reliable
flushing, compare PostgREST/hasql-pool#1

- This change removes the db-pool-timeout option, since new
  hasql-pool doesn't provide timing out of idle connections.
  Given that we were typically running with very high
  timeout settings, I don't anticipate the lack of timeout
  to introduce new issues, though we might want to consider
  introducing some retry-logic down the line when we
  encounter connection failures.
robx added a commit to robx/postgrest that referenced this issue Aug 24, 2022
This version of hasql-pool is a simplified rewrite that doesn't use
the resource-pool package. The major API changes are that idle
connections are no longer timed out (and the corresponding setting
is gone), and that `release` makes the pool unusable, where it used
to remain usable and only flushed idle connections.

We depend on a PostgREST fork of 0.7.2 that gives us reliable
flushing, compare PostgREST/hasql-pool#1

- This change removes the db-pool-timeout option, since new
  hasql-pool doesn't provide timing out of idle connections.
  Given that we were typically running with very high
  timeout settings, I don't anticipate the lack of timeout
  to introduce new issues, though we might want to consider
  introducing some retry-logic down the line when we
  encounter connection failures.
robx added a commit to robx/postgrest that referenced this issue Aug 26, 2022
This version of hasql-pool is a simplified rewrite that doesn't use
the resource-pool package. The major API changes are that idle
connections are no longer timed out (and the corresponding setting
is gone), and that `release` makes the pool unusable, where it used
to remain usable and only flushed idle connections.

We depend on a PostgREST fork of 0.7.2 that gives us reliable
flushing, compare PostgREST/hasql-pool#1

- hasql-pool 0.7 removes timing out of idle connections, so
  this change removes the db-pool-timeout option.
  Given that we were typically running with very high
  timeout settings, I don't anticipate the lack of timeout
  to introduce new issues, though we might want to consider
  introducing some retry-logic down the line when we
  encounter connection failures.
- See PostgREST#2422 for a
  discussion on depending on a forked dependency. Besides adding
  the dependency to the nix overlay, we're also adding it to
  stack.yaml and a new cabal.project to allow stack/cabal users
  to build the project.
robx added a commit to robx/postgrest that referenced this issue Aug 29, 2022
This version of hasql-pool is a simplified rewrite that doesn't use
the resource-pool package. The major API changes are that idle
connections are no longer timed out (and the corresponding setting
is gone), and that `release` makes the pool unusable, where it used
to remain usable and only flushed idle connections.

We depend on a PostgREST fork of 0.7.2 that gives us reliable
flushing, compare PostgREST/hasql-pool#1

- hasql-pool 0.7 removes timing out of idle connections, so
  this change removes the db-pool-timeout option.
  Given that we were typically running with very high
  timeout settings, I don't anticipate the lack of timeout
  to introduce new issues, though we might want to consider
  introducing some retry-logic down the line when we
  encounter connection failures.
- See PostgREST#2422 for a
  discussion on depending on a forked dependency. Besides adding
  the dependency to the nix overlay, we're also adding it to
  stack.yaml and a new cabal.project to allow stack/cabal users
  to build the project.
robx added a commit to robx/postgrest that referenced this issue Aug 29, 2022
This version of hasql-pool is a simplified rewrite that doesn't use
the resource-pool package. The major API changes are that idle
connections are no longer timed out (and the corresponding setting
is gone), and that `release` makes the pool unusable, where it used
to remain usable and only flushed idle connections.

We depend on a PostgREST fork of 0.7.2 that gives us reliable
flushing, compare PostgREST/hasql-pool#1

- hasql-pool 0.7 removes timing out of idle connections, so
  this change removes the db-pool-timeout option.
  Given that we were typically running with very high
  timeout settings, I don't anticipate the lack of timeout
  to introduce new issues, though we might want to consider
  introducing some retry-logic down the line when we
  encounter connection failures.
- See PostgREST#2422 for a
  discussion on depending on a forked dependency. Besides adding
  the dependency to the nix overlay, we're also adding it to
  stack.yaml and a new cabal.project to allow stack/cabal users
  to build the project.
@robx robx closed this as completed in 90eaaef Aug 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging a pull request may close this issue.

2 participants