Opened 5 years ago

Closed 15 months 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: master
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 4 years ago.
close-cursor-v2.diff (10.4 KB) - added by xoror 4 years ago.

Download all attachments as: .zip

Change History (19)

comment:1 Changed 5 years ago by jbaker

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

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 Changed 5 years ago by PaulM

  • Resolution set to wontfix
  • Status changed from new to closed

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 Changed 5 years ago by Alex

  • Resolution wontfix deleted
  • Status changed from closed to reopened

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 Changed 5 years ago by PaulM

  • Triage Stage changed from Unreviewed to Accepted

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

comment:5 Changed 4 years ago by ikelly

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 Changed 4 years ago by xoror

  • Owner changed from nobody to xoror
  • Status changed from reopened to new

Changed 4 years ago by xoror

comment:7 Changed 4 years ago by xoror

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 Changed 4 years ago by xoror

  • Has patch set
  • Needs tests set
  • Version changed from 1.1 to SVN

comment:9 Changed 4 years ago by ikelly

  • 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 Changed 4 years ago by xoror

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.

Changed 4 years ago by xoror

comment:11 Changed 4 years ago by Alex

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

comment:12 Changed 4 years ago by xoror

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 Changed 4 years ago by josh.smeaton@…

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 Changed 4 years ago by julien

  • Severity set to Normal
  • Type set to Bug

comment:15 Changed 3 years ago by aaugustin

  • UI/UX unset

Change UI/UX from NULL to False.

comment:16 Changed 3 years ago by aaugustin

  • Easy pickings unset

Change Easy pickings from NULL to False.

comment:17 Changed 15 months ago by timo

  • Resolution set to duplicate
  • Status changed from new to closed

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