* ui_ux: => 0
* easy: => 0
Comment:
That SQLAlchemy code looks fragile in the medium-term. I tend to agree
with comment 4: we make a decent attempt to close the connection and then
call it "closed" anyway. Before committing something along those lines,
can anybody think of a case where that is going to bite us (I can't)?
--
Ticket URL: <https://code.djangoproject.com/ticket/15802#comment:5>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
* owner: nobody => Honza_Kral
* status: new => assigned
Comment:
Replying to [comment:5 mtredinnick]:
> That SQLAlchemy code looks fragile in the medium-term. I tend to agree
with comment 4: we make a decent attempt to close the connection and then
call it "closed" anyway. Before committing something along those lines,
can anybody think of a case where that is going to bite us (I can't)?
Neither can I, I have been thinking about it since I wrote that ugly hack
of a patch and I cannot think of anything going wrong. We could be leaking
open connections, but those should get garbage collected and even that
would be better than disabling that process/thread forever (for example
gunicorn can't recover from this so this can kill production alltogether).
I will wait one more day for someone to speak up and then I will commit
the patch (just connection = None, not the pseudo detection).
--
Ticket URL: <https://code.djangoproject.com/ticket/15802#comment:6>
Comment (by russellm):
I agree with Malcolm. Checking exception messages is fragile, and I can't
think of an obvious downside to just treating a closed connection as
closed, especially if we log/report the fact that we tried and failed to
close the connection (so the process isn't completely transparent).
--
Ticket URL: <https://code.djangoproject.com/ticket/15802#comment:7>
Comment (by Rick van Hattem <Rick.van.Hattem@…>):
For the record, I have had the code running in production since opening
this ticket and I have not seen any problems yet.
So as far as empirical evidence goes, it is safe to commit a patch like
that.
--
Ticket URL: <https://code.djangoproject.com/ticket/15802#comment:8>
* status: assigned => closed
* resolution: => fixed
Comment:
In [16708]:
{{{
#!CommitTicketReference repository="" revision="16708"
Fixed #15802 -- pyscopg2 sometimes fail to close the connection when it's
already closed by the server, Thanks Rick van Hattem
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/15802#comment:9>
* status: closed => reopened
* cc: dekkers (added)
* has_patch: 0 => 1
* resolution: fixed =>
Comment:
I'm hitting the same problem, but when getting the cursor instead of
closing the connection:
{{{#!python
File "/home/jener-www/jener/yatemod/backend.py", line 38, in
get_account_hashes
account = Account.objects.get(username=username, domain=domain)
File "/usr/lib/python2.6/dist-packages/django/db/models/manager.py",
line 131, in get
return self.get_query_set().get(*args, **kwargs)
File "/usr/lib/python2.6/dist-packages/django/db/models/query.py", line
338, in get
num = len(clone)
File "/usr/lib/python2.6/dist-packages/django/db/models/query.py", line
81, in __len__
self._result_cache = list(self.iterator())
File "/usr/lib/python2.6/dist-packages/django/db/models/query.py", line
268, in iterator
for row in compiler.results_iter():
File "/usr/lib/python2.6/dist-
packages/django/db/models/sql/compiler.py", line 701, in results_iter
for rows in self.execute_sql(MULTI):
File "/usr/lib/python2.6/dist-
packages/django/db/models/sql/compiler.py", line 755, in execute_sql
cursor = self.connection.cursor()
File "/usr/lib/python2.6/dist-packages/django/db/backends/__init__.py",
line 281, in cursor
cursor = util.CursorWrapper(self._cursor(), self)
File "/usr/lib/python2.6/dist-
packages/django/db/backends/postgresql_psycopg2/base.py", line 173, in
_cursor
cursor = self.connection.cursor()
InterfaceError: connection already closed
}}}
The problem is again that self.connection isn't None, but it isn't usable
either, and we will loop over this error again and again. The attached
patch will catch the exception and call close(), so that we can continue
with creating a new connection.
--
Ticket URL: <https://code.djangoproject.com/ticket/15802#comment:10>
Comment (by Honza_Kral):
Replying to [comment:10 dekkers]:
> ...
>
> The problem is again that self.connection isn't None, but it isn't
usable either, and we will loop over this error again and again. The
attached patch will catch the exception and call close(), so that we can
continue with creating a new connection.
>
> ...
And does it occur then at every request or just once? If just once I
consider that fine - we wouldn't want to hide the fact that something bad
happened to the connection from the user.
--
Ticket URL: <https://code.djangoproject.com/ticket/15802#comment:11>
Comment (by dekkers):
Replying to [comment:11 Honza_Kral]:
> And does it occur then at every request or just once? If just once I
consider that fine - we wouldn't want to hide the fact that something bad
happened to the connection from the user.
It happens at every request. With my patch I still get a DatabaseError in
execute() once, but I agree that that is fine.
--
Ticket URL: <https://code.djangoproject.com/ticket/15802#comment:12>
Comment (by anonymous):
I'm running django/postgres on an unreliable infrastructur (All network is
in principle unreliable!). Of course I am hitting this problem too. Please
help me understand the state of the issue and possible workarounds.
=== State ===
* Changeset [16708] has been committed to the trunk on 29/Aug/11 and
provides an improvement but no complete fix
* Django 1.3.1 has been released 9/Sep/11
* Django 1.3.1 does NOT contain the fix. What a pity!
* The second patch completes the fix but it is not committed anywhere
* Postgres & Django without workaround or fix == no good idea
=== Workarounds ===
* Apply the attached patches to my production systems (Somehow I don't
like this!)
* Ease the pain by adding a connection pool on each postgres client and
connect locally
=== Solutions ===
* Convince the psycopg2 guys that their interpretation of pep-0249 is
wrong as it seems that the mysql guys interpreted the following different
(from pep-0249):
.close()
Close the connection now (rather than whenever !__del!__ is
called). The connection will be unusable from this point
forward; an Error (or subclass) exception will be raised
if any operation is attempted with the connection.
* Wait for Django 1.4
Is patching my installation the preferred solution at the moment? Is there
some other workaround?
What is the 'real' solution to this issue? Should django just deal with
the different behavior? Should psycopg2 change its behavior?
Since many people seem to have problems with that behavior(just google
'!InterfaceError: connection already closed') it seems to be a common
understanding that closing a closed connection should not raise an
exception. Therefore I'd say we should open a ticket at psycopg2's issue
tracker.
Do you agree? Or did I misunderstand the issue and its background?
-
Sebastian
--
Ticket URL: <https://code.djangoproject.com/ticket/15802#comment:13>
* cc: sannies (added)
Comment:
Replying to [comment:13 anonymous]:
I wasn't logged in.
--
Ticket URL: <https://code.djangoproject.com/ticket/15802#comment:14>
Comment (by dekkers):
The problem that is solved by my patch is that the connection is closed by
the DB without Django knowing it (this happens when you for example
restart the DB). Django tries to use self.connection and doesn't handle
the exception. You could change the psycopg2 behaviour to automatically
reconnect, but that might hide other problems, so I don't think that's a
good idea.
I currently run Django 1.3.1 with both patches applied without any
problems.
--
Ticket URL: <https://code.djangoproject.com/ticket/15802#comment:15>
Comment (by sannies):
No, I don't mean to automatically reconnect - that goes way to far. I
think psycopg2 should **not** raise an exception if an already closed
connection is closed again. That's all.
> You could change the psycopg2 behaviour to automatically reconnect, but
that might hide other problems, so I don't think that's a good idea.
--
Ticket URL: <https://code.djangoproject.com/ticket/15802#comment:16>
* cc: mike@… (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/15802#comment:17>
* cc: hv@… (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/15802#comment:18>
Comment (by dekkers):
Is there anything I can do to get my patch applied before 1.4 is released?
--
Ticket URL: <https://code.djangoproject.com/ticket/15802#comment:19>
* cc: anssi.kaariainen@… (added)
* needs_better_patch: 0 => 1
Comment:
In normal HTTP serving situations the connection doesn't get stuck into
closed state. When the request is finished, the connections are closed and
the already committed .close() fix will remove the broken connection. I am
of course talking about 1.4 here, as it has the .close() fix already
applied.
However, if using the connection in a long running task, then there will
be problems. In that setting, the connection is usually never closed. We
should really document that you should call connection.close() after you
are done with it. Actually, we should document how to safely use Django's
connections in task-running setting, I don't think we have that and there
are multiple potential problems in that setting (Idle in TX for example).
The attached patch doesn't apply cleanly (15802-2.diff). It doesn't apply
to the base directory, and even if applied into the django/ directory, it
doesn't apply cleanly. In addition, if I am reading it correctly it is
downright dangerous. I think what happens is this.
{{{
obj.save() # succeeds
# connection is closed for some reason
obj.save() # Hey, ._cursor() gave me a new connection! No error raised
anywhere, perhaps I am even in a different transaction...
}}}
I don't believe there to be any safe way to automatically create a new
connection without the user calling .close() in between. Surprising things
could happen otherwise. The only possible exception would be that when
leaving transaction management the connection could be closed on error.
This could also help in many situations, because usually when you get an
error (possibly due to closed connection), you will rollback the
transaction (leave transaction management), and thus the connection would
get unstuck.
Still one helpful thing would be to return a consistent
`ConnectionClosedError` when the connection is closed outside Django's
control.
In short: I don't think there is anything to do to get this into 1.4. Too
big changes required. The only solution you have is somehow detecting
closed connections in your code and closing the connection manually in
those cases.
--
Ticket URL: <https://code.djangoproject.com/ticket/15802#comment:20>
Comment (by piro):
Replying to [comment:13 anonymous]:
> * Convince the psycopg2 guys that their interpretation of pep-0249 is
wrong as it seems that the mysql guys interpreted the following different
(from pep-0249):
I've tried to do that: close() should be idempotent. But the reference
DBAPI test suite fails as it explicitly check that the second close()
raises an exception. I've discussed about the problem in the DBSIG
<http://mail.python.org/pipermail/db-sig/2011-October/005811.html>, but
never got to the point to fix the test suite. I should really write to
Stuart Bishop about that.
--
Ticket URL: <https://code.djangoproject.com/ticket/15802#comment:21>
* status: reopened => new
--
Ticket URL: <https://code.djangoproject.com/ticket/15802#comment:22>
* status: new => assigned
* owner: Honza_Kral => aaugustin
--
Ticket URL: <https://code.djangoproject.com/ticket/15802#comment:23>
* cc: reinout@… (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/15802#comment:24>
* status: assigned => closed
* resolution: => fixed
Comment:
- connection.close() was made idempotent in Django, regardless of whether
this gets fixed or not in pyscopg2.
- Django should not provide automatic reconnection within a request
because that can break transaction integrity.
- The problem of long running processes is covered by #17887.
The fix seems to work and the discussion after the ticket was reopened
didn't bring up any actionnable item.
--
Ticket URL: <https://code.djangoproject.com/ticket/15802#comment:25>
* cc: depaolim@… (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/15802#comment:26>
Comment (by Aymeric Augustin <aymeric.augustin@…>):
In [changeset:"25860096f981b0b3026b38329e4b69c72b1d8db9"]:
{{{
#!CommitTicketReference repository=""
revision="25860096f981b0b3026b38329e4b69c72b1d8db9"
Fixed #21239 -- Maintained atomicity when closing the connection.
Refs #15802 -- Reverted #7c657b24 as BaseDatabaseWrapper.close() now
has a proper "finally" clause that may need to preserve self.connection.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/15802#comment:27>
Comment (by Aymeric Augustin <aymeric.augustin@…>):
In [changeset:"4ea02bdb0dc6776fc29d8164e417469c9ba10f18"]:
{{{
#!CommitTicketReference repository=""
revision="4ea02bdb0dc6776fc29d8164e417469c9ba10f18"
[1.6.x] Fixed #21239 -- Maintained atomicity when closing the connection.
Refs #15802 -- Reverted #7c657b24 as BaseDatabaseWrapper.close() now
has a proper "finally" clause that may need to preserve self.connection.
Backport of 25860096 from master.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/15802#comment:28>
Comment (by Aymeric Augustin <aymeric.augustin@…>):
In [changeset:"2e42c859da42a871244aca36162a0aad2b63c02b"]:
{{{
#!CommitTicketReference repository=""
revision="2e42c859da42a871244aca36162a0aad2b63c02b"
[1.7.x] Fixed #21239 -- Maintained atomicity when closing the connection.
Refs #15802 -- Reverted #7c657b24 as BaseDatabaseWrapper.close() now
has a proper "finally" clause that may need to preserve self.connection.
Backport of 25860096 from master.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/15802#comment:29>
Comment (by aaugustin):
#23325 was a duplicate.
--
Ticket URL: <https://code.djangoproject.com/ticket/15802#comment:30>
* status: closed => new
* resolution: fixed =>
Comment:
I'm reopening this ticket because I want to think about reopening the
connection automatically when no transaction is in progress.
There's probably a bunch of issues with that technique, like retrying a
query that failed.
But since "connection closed" errors are a problem many users struggle
with, it's worth making sure I haven't overlooked a solution.
--
Ticket URL: <https://code.djangoproject.com/ticket/15802#comment:31>
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/15802#comment:32>
* cc: aivus (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/15802#comment:33>
* cc: pdina (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/15802#comment:34>
* cc: rckclmbr@… (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/15802#comment:35>
* version: 1.3 => 1.6
Comment:
I'm seeing this in a Django 1.6.8 project served via uWSGI 2.0.9. The
exception is:
{{{
Traceback (most recent call last):
File "/srv/myproject/local/lib/python2.7/site-
packages/django/core/handlers/wsgi.py", line 194, in __call__
signals.request_started.send(sender=self.__class__)
File "/srv/myproject/local/lib/python2.7/site-
packages/django/dispatch/dispatcher.py", line 185, in send
response = receiver(signal=self, sender=sender, **named)
File "/srv/myproject/local/lib/python2.7/site-
packages/django/db/__init__.py", line 91, in close_old_connections
conn.abort()
File "/srv/myproject/local/lib/python2.7/site-
packages/django/db/backends/__init__.py", line 381, in abort
self.rollback()
File "/srv/myproject/local/lib/python2.7/site-
packages/django/db/backends/__init__.py", line 180, in rollback
self._rollback()
File "/srv/myproject/local/lib/python2.7/site-
packages/django/db/backends/__init__.py", line 144, in _rollback
return self.connection.rollback()
File "/srv/myproject/local/lib/python2.7/site-
packages/django/db/utils.py", line 99, in __exit__
six.reraise(dj_exc_type, dj_exc_value, traceback)
File "/srv/myproject/local/lib/python2.7/site-
packages/django/db/backends/__init__.py", line 144, in _rollback
return self.connection.rollback()
django.db.utils.InterfaceError: connection already closed
}}}
I can reliably replicate it by running `ab` against the app and doing a
postgresql restart in the middle of it. All future requests fail until the
uWSGI process is restarted. I tried changing the `lazy-apps` option in
uWSGI, but the issue persists.
--
Ticket URL: <https://code.djangoproject.com/ticket/15802#comment:36>
* cc: AkosLadanyi (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/15802#comment:37>
* type: Bug => Cleanup/optimization
Comment:
Since 1.7 there is a test dedicated on it. See commit
[changeset:"2e42c859da42a871244aca36162a0aad2b63c02b"]
The aaugustin proposal probably should be marked as
"Cleanup/optimization", I don't see the bug still open since 1.7.
--
Ticket URL: <https://code.djangoproject.com/ticket/15802#comment:38>
Comment (by aaugustin):
Replying to [comment:36 baumer1122]:
> I'm seeing this in a Django 1.6.8 project. The exception is: (...)
This is a valid bug report.
The same symptoms as the bug originally reported here can be triggered
from another location.
The code where this error originates was deprecated in Django 1.6 and
removed in Django 1.8:
-
https://github.com/django/django/blob/stable/1.6.x/django/db/__init__.py#L88-L94
-
https://github.com/django/django/blob/stable/1.8.x/django/db/__init__.py#L63-L64
As a consequence, this variant of the issue can be considered fixed in
Django 1.8.
--
Ticket URL: <https://code.djangoproject.com/ticket/15802#comment:39>
* status: assigned => closed
* resolution: => fixed
Comment:
Replying to [comment:31 aaugustin]:
> I'm reopening this ticket because I want to think about reopening the
connection automatically when no transaction is in progress.
Since this ticket was getting long and confusing, I've opened another
ticket for this improvement: #24810.
--
Ticket URL: <https://code.djangoproject.com/ticket/15802#comment:40>