| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-07-28 | |||
| 00:25:39 | mnaser | melwitt: looks like a db server issue just a few seconds after the build request was created | |
| 00:26:04 | melwitt | interesting. so possible that the instance mapping INSERT failed? | |
| 00:26:20 | mnaser | let me look at the stack traces of the failures | |
| 00:26:21 | melwitt | I mean, if it's really a few seconds, that would be too long after | |
| 00:26:26 | mnaser | hopefully it shows where | |
| 00:27:59 | mnaser | melwitt: AHA | |
| 00:28:06 | mnaser | stacktrace in db fail | |
| 00:28:06 | mnaser | inst_mapping.create() | |
| 00:28:12 | melwitt | holy crap | |
| 00:28:24 | mnaser | and looks like in the exception handling | |
| 00:28:27 | mnaser | it does self._cleanup_build_artifacts(None, instances_to_build) | |
| 00:28:58 | mnaser | so if a create failed, we do _cleanup_build_artifacts but maybe we need to make sure that we assume things don't exist in _cleanup_build_artifacts | |
| 00:29:02 | melwitt | and that *didn't* delete the build request? another db trace? | |
| 00:29:07 | melwitt | we do | |
| 00:29:17 | mnaser | well i assume the exception handling failed too | |
| 00:29:21 | mnaser | because the db was out | |
| 00:29:26 | melwitt | oh, yeah ok | |
| 00:29:28 | mnaser | let me share the stack trace | |
| 00:30:08 | mnaser | http://paste.openstack.org/show/726772/ | |
| 00:30:13 | melwitt | this is wild. I really didn't think it was going to be that the create() failed due to db issues | |
| 00:30:34 | mnaser | is there a transactional system in nova.objects? | |
| 00:30:50 | mnaser | like maybe we can actually make the create build request and instance mapping a single transaction with commit | |
| 00:31:15 | melwitt | I've had it on my todo list to write a functional test that raises during the instance_mapping.create() to repro the bug, and to show how the orphan build request can't be deleted | |
| 00:31:39 | mnaser | it would still fail but at least the build request wouldn't be there anymore | |
| 00:31:59 | melwitt | hm ... we could if we create a new method that does them both under the same oslo.db session decorator thing, I believe | |
| 00:32:09 | mnaser | i guess this http request should have returned 500 | |
| 00:32:13 | melwitt | to make it be one transaction | |
| 00:32:16 | mnaser | so its not like the instance disappears under the user too | |
| 00:32:35 | mnaser | "HTTP exception thrown: Unexpected API Error. Please report this at http://bugs.launchpad.net/nova/ and attach the Nova API log if possible." yup | |
| 00:32:54 | melwitt | very interesting. this data is gold | |
| 00:32:57 | mnaser | melwitt: is there any example of somewhere where this happens? i can write a test with failure of instance_mapping.create() and then make it in a single transaction | |
| 00:33:04 | mnaser | this == single transaction | |
| 00:33:11 | melwitt | oh, lemme see | |
| 00:33:58 | mnaser | i should find other places where the db writes had failed and find their stack traces | |
| 00:34:03 | mnaser | ill do that too | |
| 00:37:18 | melwitt | there are some things in here https://github.com/openstack/nova/blob/master/nova/api/openstack/placement/objects/resource_provider.py | |
| 00:37:46 | melwitt | specifically this one https://github.com/openstack/nova/blob/master/nova/api/openstack/placement/objects/resource_provider.py#L4039 | |
| 00:38:06 | melwitt | the note there explains it well | |
| 00:40:12 | melwitt | awesome mnaser, thanks | |
| 00:47:58 | mnaser | ok i got a gold mine | |
| 00:48:23 | mnaser | i retrieved a list of all the .save or .create that failed because of db errors | |
| 00:48:34 | mnaser | which should be useful to group up together into a single transaction | |
| 00:48:56 | mnaser | im just deduping the list | |
| 00:49:06 | mnaser | fwiw -- grep _handle_dbapi_exception /var/log/nova/nova-api-wsgi.log -B70 | egrep 'nova.api.openstack.wsgi .*\.(save|create)' -B1 | |
| 00:50:42 | melwitt | sweet | |
| 00:54:51 | melwitt | mnaser: do you want to write up a bug for this and include the trace paste and all? else I can do it later, I have to run soon | |
| 00:55:23 | mnaser | melwitt: https://bugs.launchpad.net/nova/+bug/1784074 i added them all here | |
| 00:55:23 | openstack | Launchpad bug 1784074 in OpenStack Compute (nova) "Instances end up with no cell assigned in instance_mappings" [Medium,In progress] - Assigned to Matt Riedemann (mriedem) | |
| 00:56:05 | melwitt | okay | |
| 00:57:43 | melwitt | think I'm going to create a new bug since the db fail thing is a bit different | |
| 00:58:17 | mnaser | melwitt: it's up to you to how you see fit :) | |
| 00:58:26 | melwitt | :) | |
| 00:59:36 | mnaser | https://github.com/openstack/nova/blob/ff5cded9afa65cd4317ba0e3401692ed92c1d150/nova/api/openstack/placement/objects/project.py#L83-L92 | |
| 00:59:47 | mnaser | i think this is an easier pattern especially for create this stuff | |
| 00:59:53 | mnaser | rather than a top level function (i think) | |
| 01:00:27 | melwitt | yeah | |
| 01:01:13 | mnaser | https://github.com/openstack/nova/blob/16a2db6f23d150330dc82a56d0cf10366b004210/nova/objects/aggregate.py#L67-L78 | |
| 01:01:19 | mnaser | this seems like an interesting pattern too | |
| 01:01:26 | mnaser | im digging around code | |
| 01:02:34 | mnaser | im just curious/worry that wrapping things in the writer twice might break things | |
| 01:02:40 | mnaser | i.e.: https://github.com/openstack/nova/blob/16a2db6f23d150330dc82a56d0cf10366b004210/nova/objects/instance_mapping.py#L75-L86 | |
| 01:02:58 | melwitt | it will, don't nest it | |
| 01:03:00 | melwitt | well, | |
| 01:03:06 | melwitt | you can't nest a write under a read | |
| 01:03:23 | mnaser | can you nest two writes? or should i drop it from _create_in_db | |
| 01:03:33 | melwitt | so otherwise nesting is probably ok | |
| 01:03:43 | mnaser | ok cool | |
| 01:03:44 | melwitt | but I don't know if I've tried it, intentionally anyway | |
| 01:04:03 | mnaser | ill make sure i have a failing test first to make sure it actually gets fixed | |
| 01:05:58 | melwitt | awesome, sounds great | |
| 01:17:15 | melwitt | mnaser: https://bugs.launchpad.net/nova/+bug/1784093 fyi, I assigned it to you | |
| 01:17:15 | openstack | Launchpad bug 1784093 in OpenStack Compute (nova) "Build requests can be orphaned without instance mappings" [Medium,Confirmed] - Assigned to Mohammed Naser (mnaser) | |
| 01:18:22 | mnaser | melwitt: cool, i'm building up a test right now, i should probably have a patch later today if this doesnt end up much more complicated :> | |
| 01:19:12 | melwitt | okay, good luck. we'll take a look at it next week | |
| 01:19:26 | mnaser | thanks, have a good weekend! | |
| 01:19:37 | melwitt | thanks and likewise! | |
| 02:39:20 | openstackgerrit | Merged openstack/nova master: Add nova-status upgrade check for request spec migrations https://review.openstack.org/581813 | |
| 03:13:51 | zzzeek | does the nova placement database get built as part of nova-manage db sync or nova-manage api_db sync ? | |
| 03:14:51 | zzzeek | api_db....damn | |
| 03:54:52 | openstackgerrit | Merged openstack/nova master: Fix create_all() to replace_all() in comments https://review.openstack.org/586396 | |
| 04:04:12 | openstackgerrit | Mohammed Naser proposed openstack/nova master: Create request spec, build request and mappings in one transaction https://review.openstack.org/586742 | |
| 04:05:03 | mnaser | ^ tests failing but i dont know if i'm on the right track, i dont wanna dig in too deep and be in the wrong | |
| 04:41:44 | openstackgerrit | Merged openstack/nova master: libvirt: Revert non-reporting DISK_GB if sharing https://review.openstack.org/586614 | |
| 10:39:38 | openstackgerrit | Chris Dent proposed openstack/nova master: [placement] Retry allocation writes server side https://review.openstack.org/586048 | |
| 11:59:43 | openstackgerrit | Thomas Goirand proposed openstack/nova master: Remove "async" as variable name https://review.openstack.org/586782 | |
| 12:10:07 | openstackgerrit | Thomas Goirand proposed openstack/nova master: Remove "async" as variable name https://review.openstack.org/586782 | |
| 12:50:28 | openstackgerrit | Merged openstack/nova master: libvirt: Start populating NUMACell.network_metadata field https://review.openstack.org/564441 | |
| 12:50:34 | openstackgerrit | Merged openstack/nova master: Add functional tests for numa-aware-vswitches https://review.openstack.org/583288 | |
| 12:50:43 | openstackgerrit | Merged openstack/nova master: Consider network NUMA affinity for move operations https://review.openstack.org/585339 | |
| 14:55:41 | openstackgerrit | xulei proposed openstack/nova stable/queens: Disable limits if force_hosts or force_nodes is set https://review.openstack.org/584616 | |
| 15:18:40 | openstackgerrit | Merged openstack/nova stable/queens: Make host_aggregate_map dictionary case-insensitive https://review.openstack.org/585334 | |
| 15:54:10 | openstackgerrit | Mohammed Naser proposed openstack/nova master: Create request spec, build request and mappings in one transaction https://review.openstack.org/586742 | |
| 15:54:11 | openstackgerrit | Mohammed Naser proposed openstack/nova master: Check for build request even if instance mapping doesn't exist https://review.openstack.org/586824 | |
| 16:51:52 | openstackgerrit | Merged openstack/nova master: Fix accumulated nits from port binding for live migration series https://review.openstack.org/583994 | |
| 19:46:07 | openstackgerrit | Vieri proposed openstack/osc-placement master: Add image link in README.rst https://review.openstack.org/586839 | |
| #openstack-nova - 2018-07-29 | |||
| 02:11:00 | openstackgerrit | Thomas Goirand proposed openstack/nova master: Remove "async" as variable name https://review.openstack.org/586782 | |
| 02:42:32 | openstackgerrit | Merged openstack/nova master: scheduler: Start utilizing RequestSpec.network_metadata https://review.openstack.org/564452 | |
| 03:41:16 | openstackgerrit | Merged openstack/nova master: Assorted cleanups from numa-aware-vswitches series https://review.openstack.org/582651 | |
| 06:36:43 | openstackgerrit | Merged openstack/nova master: Fix ServerMigrationSampleJsonTests to use sample files from version dir https://review.openstack.org/582702 | |
| 11:48:11 | openstackgerrit | Ghanshyam Mann proposed openstack/nova master: Remove the deprecated API extensions policies https://review.openstack.org/586872 | |