yahoo-eng-team team mailing list archive

[Thierry Carrez <thierry.carrez+lp@xxxxxxxxx>]

** Changed in: nova
       Status: Fix Committed => Fix Released

-- 
You received this bug notification because you are a member of Yahoo!
Engineering Team, which is subscribed to OpenStack Compute (nova).
https://bugs.launchpad.net/bugs/1431571

Title:
  archive_deleted_rows_for_table relies on reflection to access the
  "default" for soft-delete columns, but this is not a server default

Status in OpenStack Compute (Nova):
  Fix Released

Bug description:
  Running subsets of Nova tests or individual tests within test_db_api
  reveals a simple error in several of the tests within ArchiveTestCase.

  A test such as test_archive_deleted_rows_2_tables attempts the
  following:

  1. places six rows into instance_id_mappings
  2. places six rows into instances
  3. runs the archive_deleted_rows_ routine with a max of 7 rows to archive
  4. runs a SELECT of instances and instance_id_mappings, and confirms that only 5 remain.

  Running this test directly with PYTHONHASHSEED=random will very easily
  encounter failures such as:

  Traceback (most recent call last):
    File "/Users/classic/dev/redhat/openstack/nova/nova/tests/unit/db/test_db_api.py", line 7869, in test_archive_deleted_rows_2_tables
      self.assertEqual(len(iim_rows) + len(i_rows), 5)
    File "/Users/classic/dev/redhat/openstack/nova/.tox/py27/lib/python2.7/site-packages/testtools/testcase.py", line 350, in assertEqual
      self.assertThat(observed, matcher, message)
    File "/Users/classic/dev/redhat/openstack/nova/.tox/py27/lib/python2.7/site-packages/testtools/testcase.py", line 435, in assertThat
      raise mismatch_error
  testtools.matchers._impl.MismatchError: 8 != 5

  
  or 

  Traceback (most recent call last):
    File "/Users/classic/dev/redhat/openstack/nova/nova/tests/unit/db/test_db_api.py", line 7872, in test_archive_deleted_rows_2_tables
      self.assertEqual(len(iim_rows) + len(i_rows), 5)
    File "/Users/classic/dev/redhat/openstack/nova/.tox/py27/lib/python2.7/site-packages/testtools/testcase.py", line 350, in assertEqual
      self.assertThat(observed, matcher, message)
    File "/Users/classic/dev/redhat/openstack/nova/.tox/py27/lib/python2.7/site-packages/testtools/testcase.py", line 435, in assertThat
      raise mismatch_error
  testtools.matchers._impl.MismatchError: 10 != 5

  
  The reason is that the archive_deleted_rows() routine looks for rows in *all* tables, in *non-deterministic order*, e.g. by searching through "models.__dict__.itervalues()".   In the "8 != 5" case, there are rows present also in the instance_types table.  By PDBing into archive_deleted_rows during the test, we can see here:

  ARCHIVED 4 ROWS FROM TABLE instances
  ARCHIVED 3 ROWS FROM TABLE instance_types
  Traceback (most recent call last):
  ...
  testtools.matchers._impl.MismatchError: 8 != 5

  that is, the archiver locates seven rows just between instances and
  instance_types, then stops.  It never even gets to the
  instance_id_mappings table.

  The serious problem with the way this test is designed, is that if we
  were to make it ignore only certain tables, or make the ordering
  fixed, or anything else, that will never keep the test from breaking
  again, any time a new table is added which contains rows when the test
  fixtures start.

  The only solution to making these tests runnable in their current form
  is to limit the listing of tables that are searched in
  archive_deleted_rows; that is, the test needs to inject a fixture into
  it.  The most straightforward way to achieve this would look like
  this:

   @require_admin_context
  -def archive_deleted_rows(context, max_rows=None):
  +def archive_deleted_rows(context, max_rows=None, _limit_tablenames_fixture=None):
       """Move up to max_rows rows from production tables to the corresponding
       shadow tables.
   
  @@ -5870,6 +5870,9 @@ def archive_deleted_rows(context, max_rows=None):
           if hasattr(model_class, "__tablename__"):
               tablenames.append(model_class.__tablename__)
       rows_archived = 0
  +    if _limit_tablenames_fixture:
  +        tablenames = set(tablenames).intersection(_limit_tablenames_fixture)
  +
       for tablename in tablenames:
           rows_archived += archive_deleted_rows_for_table(context, tablename,
                                            max_rows=max_rows - rows_archived)

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