Melange uses sqlalchemy incorrectly making with_lockmode not work as expected

[Johannes Erdfelt]

The code in melange/db/sqlalchemy/api.py is intended to lock the selected row for update using the sqlalchemy with_lockmode method, but this only works if the same session is used for the select and the delete.

As a result, the lock is dropped and a race condition exists between selecting and deleting the row, allowing two threads to possibly try to delete the same row. This can result in this exception:

Traceback (most recent call last):
  File "/usr/lib/python2.6/dist-packages/melange/common/wsgi.py", line 146, in execute_action
    **action_args)
  File "/usr/lib/python2.6/dist-packages/melange/openstack/common/wsgi.py", line 312, in execute_action
    return self.dispatch(self.controller, action, request, **action_args)
  File "/usr/lib/python2.6/dist-packages/melange/openstack/common/wsgi.py", line 321, in dispatch
    return method(*args, **kwargs)
  File "/usr/lib/python2.6/dist-packages/melange/ipam/service.py", line 456, in update_all
    **iface)
  File "/usr/lib/python2.6/dist-packages/melange/ipam/models.py", line 822, in create_and_allocate_ips
    **kwargs)
  File "/usr/lib/python2.6/dist-packages/melange/ipam/models.py", line 839, in create_and_configure
    MacAddressRange.allocate_next_free_mac(interface_id=interface.id)
  File "/usr/lib/python2.6/dist-packages/melange/ipam/models.py", line 712, in allocate_next_free_mac
    return range.allocate_mac(**kwargs)
  File "/usr/lib/python2.6/dist-packages/melange/ipam/models.py", line 728, in allocate_mac
    next_address = generator.next_mac()
  File "/usr/lib/python2.6/dist-packages/melange/mac/db_based_mac_generator/generator.py", line 29, in next_mac
    models.AllocatableMac, mac_address_range_id=self.mac_range.id)
  File "/usr/lib/python2.6/dist-packages/melange/db/sqlalchemy/api.py", line 194, in pop_allocatable_address
    delete(address_rec)
  File "/usr/lib/python2.6/dist-packages/melange/db/sqlalchemy/api.py", line 68, in delete
    db_session.flush()
  File "/usr/lib/python2.6/dist-packages/sqlalchemy/orm/session.py", line 1346, in flush
    self._flush(objects)
  File "/usr/lib/python2.6/dist-packages/sqlalchemy/orm/session.py", line 1427, in _flush
    flush_context.execute()
  File "/usr/lib/python2.6/dist-packages/sqlalchemy/orm/unitofwork.py", line 299, in execute
    rec.execute(self)
  File "/usr/lib/python2.6/dist-packages/sqlalchemy/orm/unitofwork.py", line 466, in execute
    uow
  File "/usr/lib/python2.6/dist-packages/sqlalchemy/orm/mapper.py", line 2045, in _delete_obj
    (c.rowcount, len(del_objects))
ConcurrentModificationError: Deleted rowcount 0 does not match number of objects deleted 1

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.openstack.org/6886

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/6886
Committed: http://github.com/openstack/melange/commit/daaa222ab6057bb51bf0d6e3a95041960fbd3488
Submitter: Jenkins
Branch: master

commit daaa222ab6057bb51bf0d6e3a95041960fbd3488
Author: Johannes Erdfelt <email address hidden>
Date: Fri Apr 27 21:27:02 2012 +0000

    Use sqlalchemy sessions correctly so locking works

    Fixes bug 990118

    When using with_lockmode, it's necessary to use the same session for both
    the select and the subsequent update/delete. Since a new session was
    being used for each part, the lock was becoming an effective noop and
    two requests could race with each other. Make sure we use one session
    to prevent this from happening.

    Change-Id: I6765c97266d13b795f1153b440cfc556df065787

[Sean Dague (sdague)]

Moving to invalid state because melange is no longer a project, and it would be nice to have the critical bugs in openstack only be for things we are still fixing. :)

[Johannes Erdfelt (johannes.erdfelt)]

I don't get moving these bugs to Invalid, especially when they are already fixed.

Why not just abandon the bugs?

[Jason Kölker (jason-koelker)]

Yea the fix is released, both in the legacy openstack repo and the rackspace upstream repo. Marking as fix-released.