On 03/13/2015 07:57 AM, Sean Dague wrote: > On 03/13/2015 07:07 AM, Sean Dague wrote: >> On 03/13/2015 05:54 AM, Thomas Herve wrote: >>> >>> [snip] >>>> If we assume that all of our tables are filled up with zeroes for those >>>> deleted columns, because that’s the default, this **wipes the whole table >>>> clean**. >>>> >>>> How do the tests pass? Well the tests are in test_db_api->ArchiveTestCase, >>>> and actually, they don’t. But they don’t fail every time, because the test >>>> suite here runs with a database that is almost completely empty anyway, so >>>> the broken archival routine doesn’t find too many rows to blow away except >>>> for the rows in “instance_types”, which it only finds sometimes because the >>>> tests are only running it with a small number of things to delete and the >>>> order of the tables is non-deterministic. >>>> >>>> I’ve posted the bug report at https://bugs.launchpad.net/nova/+bug/1431571 >>>> where I started out not knowing much about how this worked except that my >>>> tests were failing, and slowly stumbled my way to come to this conclusion. >>>> A >>>> patch is at https://review.openstack.org/#/c/164009/, where we look at the >>>> actual Python-side default. However I’d recommend that we just hardcode the >>>> zero here, since that’s how our soft-delete columns work. >>> >>> Hi Mike, >>> >>> Thanks for the investigation. I was wondering when that behavior was >>> introduced and it seems that >>> http://git.openstack.org/cgit/openstack/nova/commit/?id=ecf74d4c0a5a8a4290ecac048fc437dafe3d40fc >>> is the likely culprit, which would mean that only Kilo is affected. Can >>> you confirm? >>> >>> Thanks, >> >> Yes, that looks like the problematic patch. I'd rather actually revert >> that patch instead. >> >> Also, real tests would be nice to actually prevent future regression. >> >> -Sean >> > > Ok, we've done a straight revert here - > https://review.openstack.org/#/c/164140/ > > I'm also working on a test enhancement that ensures that all the shadow > tables except the one we believe should contain entries are empty. That > seems to specifically expose and nail this bug. Needs some cleanup, but > should be posted by midday.
The test additions are now posted here - https://review.openstack.org/#/c/164178/ -Sean -- Sean Dague http://dague.net __________________________________________________________________________ OpenStack Development Mailing List (not for usage questions) Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev