Opened 5 years ago
Closed 5 years ago
#30646 closed Bug (wontfix)
close_if_unusable_or_obsolete fails to close unusable connections.
| Reported by: | Daniel Neuhäuser | Owned by: | nobody |
|---|---|---|---|
| Component: | Database layer (models, ORM) | Version: | dev |
| Severity: | Normal | Keywords: | |
| Cc: | Triage Stage: | Unreviewed | |
| Has patch: | no | Needs documentation: | no |
| Needs tests: | no | Patch needs improvement: | no |
| Easy pickings: | no | UI/UX: | no |
Description
BaseDatabaseWrapper.close_if_unusable_or_obsolete doesn't always close unusable connections. A connection is only tested for usability, if an error has occurred on that connection previously. However a connection might be unusable for other reasons such as database failover.
As we run databases in Kubernetes (using https://github.com/zalando/postgres-operator), a database failover is not an unusual scenario and not of any concern. However on employee-facing internal applications that see relatively low traffic, this leads to a number of requests failing with 500s on every failover. We've patched this in our applications but it would be nice to see this fixed in Django itself.
Fixing this does incur the cost of a roundtrip to the database with every request but this doesn't appear to have any significant impact on latency.
Change History (3)
comment:1 by , 5 years ago
comment:2 by , 5 years ago
Hello there,
Fixing this does incur the cost of a roundtrip to the database with every request but this doesn't appear to have any significant impact on latency.
I'm afraid we can't take an anecdotical report about effects on your setup to stand for most of other setups out there. What if someone reports that it significantly affects their setup?
I guess we could add a flag to close_if_unusable_or_obsolete for this purpose but it seems wasteful to perform this check on every request. If we decide that this should be done we should at least move the obsolescence check before the usable one since the former doesn't require any database interactions and will be reached on all requests when CONN_MAX_AGE=0 which is the default.
comment:3 by , 5 years ago
| Resolution: | → wontfix |
|---|---|
| Status: | new → closed |
| Summary: | close_if_unusable_or_obsolete fails to close unusable connections → close_if_unusable_or_obsolete fails to close unusable connections. |
Thanks for the report, however I agree with Simon that checking this on every request seems like an overkill.
Please write to the DevelopersMailingList if you want other opinions, we can re-open this ticket if we reach a consensus on the mailing list.
I've created a PR to fix the problem: https://github.com/django/django/pull/11573