Opened 11 months ago

Closed 11 months ago

Last modified 11 months ago

#23074 closed Bug (fixed)

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

Reported by: hyperair@… Owned by: aaugustin
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 aaugustin)

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@…> 11 months ago.
Patch to call savepoint_commit after savepoint_rollback in Atomic.exit

Download all attachments as: .zip

Change History (7)

Changed 11 months ago by Chow Loong Jin <hyperair@…>

Patch to call savepoint_commit after savepoint_rollback in Atomic.exit

comment:1 Changed 11 months ago by Chow Loong Jin <hyperair@…>

  • Has patch set
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

comment:2 Changed 11 months ago by aaugustin

  • Description modified (diff)
  • Owner changed from nobody to aaugustin
  • Status changed from new to assigned

comment:3 Changed 11 months ago by timo

  • Needs tests set
  • Triage Stage changed from Unreviewed to Accepted

comment:4 Changed 11 months ago by aaugustin

  • Needs tests unset
  • Patch needs improvement set
  • Triage Stage changed from Accepted to Ready 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 11 months ago by aaugustin (previous) (diff)

comment:5 Changed 11 months ago by aaugustin

  • Triage Stage changed from Ready for checkin to Accepted

comment:6 Changed 11 months ago by Aymeric Augustin <aymeric.augustin@…>

  • Resolution set to fixed
  • Status changed from assigned to closed

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