Opened 2 years ago

Closed 2 years ago

Last modified 2 years ago

#23074 closed Bug (fixed)

django.db.transaction.atomic() leaks savepoints on exception

Reported by: hyperair@… Owned by: Aymeric Augustin
Component: Database layer (models, ORM) Version: master
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: yes
Easy pickings: no UI/UX: no

Description (last modified by Aymeric Augustin)

When savepoints are rolled back after an exception in a transaction.atomic() block, the savepoint is leaked. Specifically, "ROLLBACK TO SAVEPOINT xx" is done, but not "RELEASE SAVEPOINT xx". The savepoint ID is then thrown away by Django and leaked.

See http://www.postgresql.org/docs/9.1/static/sql-release-savepoint.html and http://www.postgresql.org/docs/9.1/static/sql-rollback-to.html for more information.

When spinning in a get_or_create() loop in a single transaction, this has the potential to max out the shared memory of the PostgreSQL instance resulting in this:

  File "/usr/lib/python2.7/dist-packages/django/db/models/manager.py", line 157, in create
    return self.get_queryset().create(**kwargs)
  File "/usr/lib/python2.7/dist-packages/django/db/models/query.py", line 319, in create
    obj.save(force_insert=True, using=self.db)
  File "/usr/lib/python2.7/dist-packages/django/db/models/base.py", line 545, in save
    force_update=force_update, update_fields=update_fields)
  File "/usr/lib/python2.7/dist-packages/django/db/models/base.py", line 573, in save_base
    updated = self._save_table(raw, cls, force_insert, force_update, using, update_fields)
  File "/usr/lib/python2.7/dist-packages/django/db/models/base.py", line 654, in _save_table
    result = self._do_insert(cls._base_manager, using, fields, update_pk, raw)
  File "/usr/lib/python2.7/dist-packages/django/db/models/base.py", line 687, in _do_insert
    using=using, raw=raw)
  File "/usr/lib/python2.7/dist-packages/django/db/models/manager.py", line 232, in _insert
    return insert_query(self.model, objs, fields, **kwargs)
  File "/usr/lib/python2.7/dist-packages/django/db/models/query.py", line 1511, in insert_query
    return query.get_compiler(using=using).execute_sql(return_id)
  File "/usr/lib/python2.7/dist-packages/django/db/models/sql/compiler.py", line 898, in execute_sql
    cursor.execute(sql, params)
  File "/usr/lib/python2.7/dist-packages/django/db/backends/util.py", line 69, in execute
    return super(CursorDebugWrapper, self).execute(sql, params)
  File "/usr/lib/python2.7/dist-packages/django/db/backends/util.py", line 53, in execute
    return self.cursor.execute(sql, params)
  File "/usr/lib/python2.7/dist-packages/django/db/utils.py", line 99, in __exit__
    six.reraise(dj_exc_type, dj_exc_value, traceback)
  File "/usr/lib/python2.7/dist-packages/django/db/backends/util.py", line 53, in execute
    return self.cursor.execute(sql, params)
django.db.utils.OperationalError: out of shared memory
HINT:  You might need to increase max_locks_per_transaction.

Attachments (1)

0001-Fixed-23074-Release-savepoints-after-doing-a-savepoi.patch (1.5 KB) - added by Chow Loong Jin <hyperair@…> 2 years ago.
Patch to call savepoint_commit after savepoint_rollback in Atomic.exit

Download all attachments as: .zip

Change History (7)

Changed 2 years ago by Chow Loong Jin <hyperair@…>

Patch to call savepoint_commit after savepoint_rollback in Atomic.exit

comment:1 Changed 2 years ago by Chow Loong Jin <hyperair@…>

Has patch: set

comment:2 Changed 2 years ago by Aymeric Augustin

Description: modified (diff)
Owner: changed from nobody to Aymeric Augustin
Status: newassigned

comment:3 Changed 2 years ago by Tim Graham

Needs tests: set
Triage Stage: UnreviewedAccepted

comment:4 Changed 2 years ago by Aymeric Augustin

Needs tests: unset
Patch needs improvement: set
Triage Stage: AcceptedReady for checkin

The fix was correct. I added a test.

PR: https://github.com/django/django/pull/2992

I suspect it will throw some assertNumQueries off. I'll let Jenkins run the tests and see what happens.

I don't plan to backport it to 1.7, because we've already released RC2 and it appears to be an fairly rare case (otherwise it would have been reported earlier). To work around the problem, you can:

  • create smaller batches and wrap each batch in its own transaction;
  • use a patched copy of Django.
Last edited 2 years ago by Aymeric Augustin (previous) (diff)

comment:5 Changed 2 years ago by Aymeric Augustin

Triage Stage: Ready for checkinAccepted

comment:6 Changed 2 years ago by Aymeric Augustin <aymeric.augustin@…>

Resolution: fixed
Status: assignedclosed

In 729e4ae4f0730585ac4640e7fa3aa06374677ff2:

Fixed #23074 -- Avoided leaking savepoints in atomic.

Thanks Chow Loong Jin for the report and the initial patch.

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