Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2017-08-22
20:06:53 cburgess @kbringard Did we look at the earlier schema migrates to see if one of them changes the table schema and we some how misses that one?
20:06:55 kbringard is there some other, earlier, place we should modifying the object
20:06:56 mriedem flavor_values['deleted_at'] = utils.strtime(flavor_values['deleted_at']) is wrong
20:07:06 mriedem there is no deleted_at column in the nova_api.flavors table
20:07:07 kbringard cburgess: yea, I looked at it, they're both datetime
20:07:22 cburgess kbringard k... ok well file a bit and propose the patch I guess... lets see what they say.
20:07:29 mriedem maybe "if 'deleted_at' in flavor_values" is just always False so you never hit that
20:07:31 kbringard mriedem: sure, but it exists in the original nova one
20:07:40 mriedem right, but we don't migrate that column
20:07:47 kbringard so if they're all NULL then it works fine
20:07:58 kbringard if created_at or updated_at isn't NULL then it fails
20:08:11 kbringard and like you said it just drops deleted_at
20:08:17 kbringard I only added it in the patch for sanity
20:08:26 kbringard and I mean, I don't think this is necessarily a good patch
20:08:28 dansmith I'm confused how do you think this is working upstream if the code is wrong for the column data type?
20:08:37 kbringard well, I don't know
20:08:40 kbringard that's what we're trying to figure out
20:08:44 dansmith there have been oslo changes around timeutils, maybe you've got something crossed there?
20:10:03 kbringard yea, I dunno, that's what I'm trying to get some insight into… I don't know this code very well (or really at all)
20:10:27 mriedem besides the deleted and deleted_at columns, is the table schema the same between nova.instance_types and nova_api.flavors?
20:10:42 kbringard https://github.com/openstack/nova/blob/stable/ocata/nova/objects/base.py#L154-L164
20:10:45 kbringard this is the base object
20:11:13 kbringard https://github.com/openstack/nova/blob/stable/ocata/nova/objects/flavor.py#L733-L758
20:11:18 kbringard this is the migration object code
20:11:34 kbringard specifically when it does this: https://github.com/openstack/nova/blob/stable/ocata/nova/objects/flavor.py#L744-L745
20:11:51 kbringard the field: it creates has the TZ as +00:00
20:12:13 kbringard so when you dump the JSON you can see the string is "wrong"
20:12:43 mriedem yeah the objects aren't tz-aware
20:12:52 cburgess Actually..
20:12:54 kbringard of course that's the print stuff doing the strings, so you know
20:12:55 cburgess @mriedem @kbringard https://github.com/openstack/oslo.versionedobjects/blob/master/oslo_versionedobjects/fields.py#L459-L460
20:13:08 cburgess Thats in master.
20:13:11 kbringard right, so there's the other thing I was looking at cburgess
20:13:30 kbringard that same code has existed for awhile, iirc
20:13:32 cburgess So I wonder if the issue here is the version of oslo_versionedobjects.. along the lines of what dansmith said.
20:14:02 mriedem that's been around forever https://github.com/openstack/oslo.versionedobjects/commit/b6e71f4524c0fece34e2f7e2a8d3176538538912
20:14:06 kbringard ^^
20:14:08 cburgess kbringard What version of versionedobjects do we have?
20:14:20 kbringard when we're running this it's stable/ocata
20:14:50 cburgess We are checking that out from git and building it in a venv or using something RedHat provided?
20:15:03 kbringard python-oslo-versionedobjects-1.21.0-1.el7ost.noarch
20:15:03 kbringard python-oslo-versionedobjects-lang-1.21.0-1.el7ost.noarch
20:15:14 kbringard vanilla Rh package
20:15:24 dansmith it's likely a timeutils thing not an o.vo thing right?
20:15:53 cburgess Good point
20:15:58 cburgess We use oslo.timeutiles to do the conversion
20:16:21 cburgess kbringard so what version of oslo_utils do we have?
20:16:52 kbringard python-oslo-utils-lang-3.22.0-1.el7ost.noarch
20:16:52 kbringard python-oslo-utils-3.22.0-1.el7ost.noarch
20:17:49 kbringard my initial thought was if we maybe don't have enough test coverage?
20:17:58 cburgess kbringard netwon was constrained to 3.16.1
20:18:01 kbringard like, the default flavors we used to create had NULL for all those guys
20:18:07 kbringard and that all works
20:18:42 kbringard so I'm wondering if this has just been a bug that we'd not caught before, and if no one was doing these migrations (or said anything when they did) then maybe we just never discovered it before?
20:18:51 cfriesen_ mriedem: did you want me to do a fix for the live migration cell restriction on top of your fix at https://review.openstack.org/#/c/49603 ?
20:19:02 cfriesen_ mriedem: or were you going to propose a patch for that?
20:22:32 mriedem cfriesen_: wrong link? i've got a patch though
20:22:36 mriedem just finishing unit tests
20:22:57 cfriesen_ mriedem: gah, I meant https://review.openstack.org/#/c/496031
20:23:11 cfriesen_ anyways, cool
20:23:23 mriedem and yes it's on top of that series
20:23:27 mriedem to avoid merge conflicts
20:27:29 cfriesen_ mriedem: looking at that commit, in the future if we pass "skip-filters" to the scheduler wouldn't we also want to skip the initial placement checks? (since they're essentially what used to be filters) Presumably the "claim resources on destination" step would fail though.
20:28:58 mriedem cfriesen_: tbd
20:29:09 mriedem cfriesen_: conductor already essentially does the RamFilter
20:29:15 mriedem which is replaced by Placement's check on MEMORY_MB
20:29:30 mriedem so the thing that would be different would be filtering, via placement, on VCPU and DISK_GB
20:29:37 cfriesen_ mriedem: I think that should be pulled out since it was probably legacy code from before we ran the scheduler filters
20:29:49 mriedem ?
20:30:02 cfriesen_ having conductor do the memory check, I mean
20:30:09 mriedem pretty sure that was added in 2.30
20:30:25 cfriesen_ guess I didn't review that patch
20:30:30 mriedem because that happens in the force host flow
20:30:36 mriedem which doesn't call select_destinations
20:30:50 cfriesen_ ah, sure. in the force flow it makes sense
20:31:27 mriedem actually looks like you're right
20:31:32 mriedem https://review.openstack.org/#/c/29077/
20:31:35 mriedem that's way old
20:31:55 cfriesen_ I was just thinking that in the force case you could skip the placement checks and filters in the scheduler and just jump right to the resource claim.
20:32:43 mriedem predates select_destinations
20:32:45 mriedem goes back to havana
20:33:08 mriedem so yeah that ram check probably shouldn't even be in conductor
20:34:20 cfriesen_ arguably it makes sense in the current flow since it's a critical resource...hard to overcommit ram the way you can with cpu
20:58:45 openstackgerrit Matt Riedemann proposed openstack/nova master: Restrict live migration to same cell https://review.openstack.org/496419
20:58:55 mriedem cfriesen_: dansmith: there is the live migration restricted to same cell fix ^
22:05:30 cfriesen_ mriedem: you're a machine with all these fixes. :)
22:07:04 mriedem all these 2 fixes
22:08:36 cfriesen_ is there a way to query the hosts in a cell?
22:08:54 cfriesen_ don't see anything in the api-ref
22:10:51 cfriesen_ where I'm going with this is...how does the admin know which hosts are part of the same cell?
22:20:53 mriedem cfriesen_: https://docs.openstack.org/nova/latest/user/cells.html#faqs
22:21:43 mriedem we probably need another cli in queens for that
22:21:54 mriedem or build on list_cells
22:33:04 cfriesen_ mriedem: okay, makes sense
22:36:34 cfriesen_ mriedem: would we need a spec for nova-manage? (does it count as a public API?)
22:57:04 mriedem nope
23:37:11 openstackgerrit Matt Riedemann proposed openstack/nova master: Remove source node allocation after live migration completes https://review.openstack.org/496032
23:37:12 openstackgerrit Matt Riedemann proposed openstack/nova master: Restrict live migration to same cell https://review.openstack.org/496419
23:37:13 openstackgerrit Matt Riedemann proposed openstack/nova master: Allocate resources on forced dest host during live migration https://review.openstack.org/496031
#openstack-nova - 2017-08-23
00:04:06 cali_boxer hello all,

Earlier   Later