yahoo-eng-team team mailing list archive

[Maru Newby <marun@xxxxxxxxxx>]

Public bug reported:

Whenever an eventlet yield occurs in a db transaction in which one or
more db locks are held, the potential for deadlock exists.

Yields can be triggered by:
  a. network IO (e.g. a plugin calling out to another application)
  b. lock contention (e.g. when code attempts and fails to acquire a semaphore, RLock, etc)

It's not always obvious when reviewing code whether either trigger is
possible inside a db transaction.  In some cases, neither submitter nor
reviewer will be aware that a 3rd party library being used inside a
transaction relies upon a locking primitive (e.g. logging uses RLock
internally).  In other cases, it won't always be obvious from the patch
that a method being changed will be wrapped in a transaction (e.g. an
ML2 driver that implements a *_precommit method).

A suggested way of detecting yields so that the offending code can be
fixed pre-merge (provided test coverage) is to wrap transaction
initiation and locally monkey patch (via contextlib) the eventlet
methods responsible for yielding.  Detection could then be communicated
via an exception or logging.

Eventlet methods to target:

greenthread.getcurrent().switch()
eventlet.hubs.get_hub.switch()

Initially, the goal would be to serialize all db transactions.  A
refinement would be to serialize only those db transactions that held
locks.

** Affects: neutron
     Importance: High
     Assignee: Maru Newby (maru)
         Status: New

** Changed in: neutron
   Importance: Undecided => High

** Description changed:

  Whenever an eventlet yield occurs in a db transaction in which one or
  more db locks are held, the potential for deadlock exists.
  
  Yields can be triggered by:
-   a. network IO (e.g. a plugin calling out to another application)
-   b. lock contention (e.g. when code attempts and fails to acquire a semaphore, RLock, etc)
+   a. network IO (e.g. a plugin calling out to another application)
+   b. lock contention (e.g. when code attempts and fails to acquire a semaphore, RLock, etc)
  
  It's not always obvious when reviewing code whether either trigger is
  possible inside a db transaction.  In some cases, neither submitter nor
  reviewer will be aware that a 3rd party library being used inside a
  transaction relies upon a locking primitive (e.g. logging uses RLock
  internally).  In other cases, it won't always be obvious from the patch
  that a method being changed will be wrapped in a transaction (e.g. an
  ML2 driver that implements a *_precommit method).
  
- A suggested way of detecting yields so that they the offending code can
- be fixed pre-merge (provided test coverage) is to wrap transaction
+ A suggested way of detecting yields so that the offending code can be
+ fixed pre-merge (provided test coverage) is to wrap transaction
  initiation and locally monkey patch (via contextlib) the eventlet
- methods responsible for yielding.  Detection could be communicated via
- an exception or via logging as desired.
+ methods responsible for yielding.  Detection could then be communicated
+ via an exception or logging.
  
  Eventlet methods to target:
  
  greenthread.getcurrent().switch()
  eventlet.hubs.get_hub.switch()
  
  Initially, the goal would be to serialize all db transactions.  A
  refinement would be to serialize only those db transactions that held
  locks.

-- 
You received this bug notification because you are a member of Yahoo!
Engineering Team, which is subscribed to neutron.
https://bugs.launchpad.net/bugs/1294942

Title:
  eventlet should not yield inside db transactions that hold locks

Status in OpenStack Neutron (virtual network service):
  New

Bug description:
  Whenever an eventlet yield occurs in a db transaction in which one or
  more db locks are held, the potential for deadlock exists.

  Yields can be triggered by:
    a. network IO (e.g. a plugin calling out to another application)
    b. lock contention (e.g. when code attempts and fails to acquire a semaphore, RLock, etc)

  It's not always obvious when reviewing code whether either trigger is
  possible inside a db transaction.  In some cases, neither submitter
  nor reviewer will be aware that a 3rd party library being used inside
  a transaction relies upon a locking primitive (e.g. logging uses RLock
  internally).  In other cases, it won't always be obvious from the
  patch that a method being changed will be wrapped in a transaction
  (e.g. an ML2 driver that implements a *_precommit method).

  A suggested way of detecting yields so that the offending code can be
  fixed pre-merge (provided test coverage) is to wrap transaction
  initiation and locally monkey patch (via contextlib) the eventlet
  methods responsible for yielding.  Detection could then be
  communicated via an exception or logging.

  Eventlet methods to target:

  greenthread.getcurrent().switch()
  eventlet.hubs.get_hub.switch()

  Initially, the goal would be to serialize all db transactions.  A
  refinement would be to serialize only those db transactions that held
  locks.

To manage notifications about this bug go to:
https://bugs.launchpad.net/neutron/+bug/1294942/+subscriptions