Opened 14 years ago

Closed 11 years ago

#14298 closed Bug (duplicate)

maximum open cursors exceeded on Jython and Oracle

Reported by: stephanekonstantaropoulos Owned by: xoror
Component: Database layer (models, ORM) Version: dev
Severity: Normal Keywords: Oracle Jython
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: yes Patch needs improvement: yes
Easy pickings: no UI/UX: no

Description

I am hitting an Oracle error frequently when running Django 1.1.2 from Jython:

ORA-01000: maximum open cursors exceeded

It never does it from cPython, I don't know if it is the driver or maybe it is the way Jython and Java clear the unused objects which is less efficient than Python's.

I fixed it myself by calling systematically cursor.close() everywhere a new cursor is created.

That is in django.db.models.sql.query and in django.db.models.fields.related

Attachments (2)

cursor-close-patch.diff (2.8 KB ) - added by xoror 14 years ago.
close-cursor-v2.diff (10.4 KB ) - added by xoror 14 years ago.

Download all attachments as: .zip

Change History (19)

comment:1 by jbaker, 14 years ago

Most likely this is the problem: Jython does not do deterministic garbage collection, unlike CPython. In CPython, when a variable goes out of scope, it will be immediately GCed because of ref counting. To do the same in Jython, use either the with-statement or

try:
  # acquire resource
finally:
  # close resource

Hope that helps.

comment:2 by Paul McMillan, 14 years ago

Resolution: wontfix
Status: newclosed

I'm tentatively closing this as wontfix. I think jbaker's explanation is likely correct, and fixing problems with an implementation's GC behavior is really not something that Django should be handling. If you disagree strongly, or know of a way to fix this that doesn't involve major modifications to the general code for this one corner case, please do re-open this and/or post on django-dev.

comment:3 by Alex Gaynor, 14 years ago

Resolution: wontfix
Status: closedreopened

Sorry I'm going to disagree here, properly closing our cursors would be the right way to go. This can be sanely implemented, but it does require some thinking.

comment:4 by Paul McMillan, 14 years ago

Triage Stage: UnreviewedAccepted

I'll go ahead and mark this as accepted then.

comment:5 by Erin Kelly, 14 years ago

Django uses a signal to close the database connection after each request. Does this not also close the cursor?

>>> db = zxJDBC.connect(d, u, p, v)
>>> cursor = db.cursor()
>>> cursor.execute("select * from dual")
>>> db.close()
>>> cursor.fetchall()
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
zxJDBC.ProgrammingError: cursor is closed

comment:6 by xoror, 14 years ago

Owner: changed from nobody to xoror
Status: reopenednew

by xoror, 14 years ago

Attachment: cursor-close-patch.diff added

comment:7 by xoror, 14 years ago

Hi,

We were having problems with this issue as well. Attached to this ticket is a small proposal patch to solve this issue.
Basically we have added a dictionary of (iterators, cursor) in the sql compiler. Once iteration is completed the cursor is closed.

There's one case where the cursor is not explicitly closed. In case of a None result type the cursor is returned to be used in the subclasses. From what I could tell from the subclasses is that is probably safe to close this cursor as well in the compiler class. But please verify. I've left that case open.

comment:8 by xoror, 14 years ago

Has patch: set
Needs tests: set
Version: 1.1SVN

comment:9 by Erin Kelly, 14 years ago

Patch needs improvement: set

Agreed that it's safe to close the cursor in SQLInsertCompiler.execute_sql and SQLUpdateCompiler.execute_sql. It also needs to be closed in SQLDateCompiler.results_iter and django.db.models.sql.subqueries.DeleteQuery.do_query.

The only problem I can see is that there is no guarantee that results_iter will be run to completion. To prevent a memory leak, the code that closes the cursor should be inside a finally: block to make sure it does eventually get cleaned up.

comment:10 by xoror, 14 years ago

Ok,

I've created a new patch. There's one part in the updatecompiler that's invokes related objects that needs special attention.

def execute_sql(self, result_type):

"""
Execute the specified update. Returns the number of rows affected by
the primary update query. The "primary update query" is the first
non-empty query that is executed. Row counts for any subsequent,
related queries are not available.
"""
cursor = super(SQLUpdateCompiler, self).execute_sql(result_type)
rows = cursor and cursor.rowcount or 0
is_empty = cursor is None
# explicit close, a Del prob won't trigger immediate GC in Jython
cursor.close()
del cursor
for query in self.query.get_related_updates():

qcompiler = query.get_compiler(self.using)
aux_rows = qcompiler.execute_sql(result_type)
if is_empty:

rows = aux_rows
is_empty = False

if (result_type is None and type(qcompiler).name not in ['SQLInsertCompiler','SQLUpdateCompiler']):

# Only insert and update compilers are cursor close-safe.
# Other compilers using SQLCompiler.execute_sql(None) will return open cursor.
aux_rows.close()

return rows

The SQLCompiler base class will return a cursor for result_type = None. So all subclasses that do not override this needs to be cleaned manually. I'm not really a fan of using type() but I don't know how to solve this otherwise in python.

by xoror, 14 years ago

Attachment: close-cursor-v2.diff added

comment:11 by Alex Gaynor, 14 years ago

This patch is far too invasive IMO, I'll want to think on this.

comment:12 by xoror, 14 years ago

The ideal case would be *execute_sql() methods that closes the cursor except for queries that have iterators attached to them.
Those can be closed in the _iter() functions (this is handled by the lookup dictionary)

The patch is getting big due to the fact that cursors are returned and processed by subclasses and higher. We had to trace all of them
to ensure proper closure. The patch will be way smaller if we could somehow manage the insert/update/delete parts to behave clean with the cursors.

The current state makes django unuseable with jython/Oracle.

comment:13 by josh.smeaton@…, 14 years ago

I think I found the ultimate cause of this issue. Well, I didn't find it, but I found a reference to it elsewhere.

http://bugs.jython.org/issue1582

That describes a memory leak where cursors aren't able to be finalised if not explicitly closed. It has to do with the zx DB drivers. My assumption is that since the cursors aren't being finalised, the finaliser is not calling close. And since django doesn't explicitly call close in all cases, they aren't able to be finalized. The bug above has a patch associated with it. I'll research this more and see if it has already been fixed properly in the zxjdbc drivers, and if so, try to migrate that into the django-on-java project.

comment:14 by Julien Phalip, 13 years ago

Severity: Normal
Type: Bug

comment:15 by Aymeric Augustin, 13 years ago

UI/UX: unset

Change UI/UX from NULL to False.

comment:16 by Aymeric Augustin, 13 years ago

Easy pickings: unset

Change Easy pickings from NULL to False.

comment:17 by Tim Graham, 11 years ago

Resolution: duplicate
Status: newclosed

I think this is a duplicate of #21751 which has a patch to close cursors after they are used.

Note: See TracTickets for help on using tickets.
Back to Top