Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2019-05-10
14:53:52 efried tssurya: thanks. If I'm wrong about that wrinkle, I'll happily +2 (will let mriedem approve though)
14:56:50 mriedem i think you're right, and likely need to use oneOf
14:57:02 mriedem where the options are null with no locked_reason or object with a required locked_reason
14:59:04 mriedem efried: i think it just copied how 2.56 works for cold migrate where you can specify null or a dict with a host
14:59:14 mriedem and it looks like that schema has the same issue where you can pass null or {}
14:59:33 efried woot
15:10:33 tssurya efried, mriedem: yea I basically did what 2.56 did for migrate host
15:10:54 tssurya I thought it was more of a feature thing than a bug :)
15:11:42 tssurya but maybe you are right, empty dict shouldn't be allowed ?
15:12:54 efried tssurya, mriedem: IMO there's no reason to allow it; just increases the test surface for no value.
15:13:23 efried And I guess we ought to have a test regardless.
15:13:28 tssurya efried: ack, I'll write a unit test like mriedem said and fix this then
15:13:32 efried ++
15:13:34 efried thanks tssurya
15:13:49 tssurya thanks for the detailed eyeying :)
15:14:12 efried handful of nits you can fix up while you're in there if you feel like it
15:14:48 tssurya yep
15:21:58 openstackgerrit Dongcan Ye proposed openstack/nova master: Raise BuildAbortException while updating instance task_state conflict https://review.opendev.org/633160
16:44:07 mriedem dansmith: seems our external event routing isn't working across multiple cells, the api figures out the instance is migrating and gets the proper hosts but for whatever reason only the source host gets the event...anyway, that's the current reason why the multi-cell resize stuff is failing in the gate, will have to dig a bit after lunch.
16:44:17 mriedem maybe i can assert that somehow in my functional test, not sure how though
16:46:07 dansmith hmm, maybe because of the batching to the compute api that it does
16:46:25 dansmith it tries to collate multiple events per host where possible
16:53:55 tssurya efried, mriedem: https://review.opendev.org/#/c/648662/11/nova/api/openstack/compute/schemas/lock_server.py I am not sure if its a good idea anymore because a lot of the actions don't have schema validation at all, that means they allow empty dicts and what not.
16:54:07 tssurya let me know what you guys thing when you have the time
16:55:10 tssurya think*
18:02:47 mriedem i guess i can't really do the same thing in the functional test because of the fake driver and neutron fixture
18:04:49 mriedem fried_rolls: replied to surya's comment but haven't gone through the rest of that change yet since the last time - if you are +2 please withold the +W so i can take a look through it again
18:18:56 mriedem dansmith: ah yes,
18:18:57 mriedem if host not in cell_contexts_by_host:
18:18:57 mriedem cell_contexts_by_host[host] = instance._context
18:19:03 mriedem that assumes all of the hosts are in the same cell
18:19:43 dansmith you mean, it assumes that for a given instance, the src/dst host will be in the same cell
18:19:45 dansmith yeah?
18:19:47 mriedem yup
18:20:05 dansmith is there no comment above saying "we assume..." ?
18:20:10 mriedem i can probably recreate/test that in my functional multi-cell test by just sending an event with a migration that has hosts in different cells
18:20:49 mriedem not really, but https://github.com/openstack/nova/blob/0cb1544106346664b4a53114458417ea62474b8c/nova/compute/api.py#L4853
18:20:54 mriedem i wouldn't really expect it to either
18:20:57 mriedem nothing has needed it yet
18:21:15 mriedem https://github.com/openstack/nova/blob/0cb1544106346664b4a53114458417ea62474b8c/nova/compute/api.py#L4869
18:21:23 dansmith right, but I was thinking we had discussed this before,
18:21:32 mriedem ah yes
18:21:33 mriedem Consequently we can currently assume that the context for # both the source and destination hosts of a migration is the # same.
18:21:33 dansmith and putting a comment like that seems like something you would have made me do :)
18:21:35 mriedem didn't read far enough
18:21:39 mriedem it was mdbooth
18:22:11 openstackgerrit Merged openstack/os-traits master: Update SEV trait docs to avoid misleading people https://review.opendev.org/655671
18:22:53 mriedem so if the migration record has cross_cell_move=true i'll have to tell it to pull the dest host mapping via the host mapping
18:23:06 mriedem so still optimized for the non-cross-cell case
18:23:55 dansmith cool
18:35:19 openstackgerrit Surya Seetharaman proposed openstack/nova master: Microversion 2.73: Support adding the reason behind a server lock https://review.opendev.org/648662
18:50:56 eandersson lol mriedem
18:54:33 zzzeek jaypipes: ping
18:56:08 mriedem hmm my genius test must not be so great http://paste.openstack.org/show/751238/
18:56:11 mriedem since it should fail, but doesn't
18:56:26 mriedem was hoping to avoid stubbing things out in the functional test to test the multi-cell event routing
18:57:45 mriedem dansmith: is there maybe something in our rpc fixture stuff that wouldn't make this easy to test in a functional test because we're using fake rpc?
18:58:16 dansmith mriedem: er, I wouldn't think so.. you're using the multi cell fixture?
18:58:31 mriedem yeah, my functional tests work fine for multi-cell
18:58:49 mriedem could be my context manager yield pattern in ^ isn't actually asserting anything
19:04:00 mriedem no that routing seems to be working at least on one host
19:04:01 mriedem b'2019-05-10 15:02:18,969 DEBUG [nova.compute.manager] Processing event network-vif-plugged-60c6c823-ed75-46a0-a0c6-30aadb90e78c'
19:04:01 mriedem b'2019-05-10 15:02:18,970 DEBUG [nova.compute.manager] Received event network-vif-plugged-60c6c823-ed75-46a0-a0c6-30aadb90e78c'
19:28:11 mriedem i think it works in functional testing because we're not using multiple rpc transports https://review.opendev.org/#/c/396417/
19:32:32 mriedem so i'm guessing in a multi-cell test we'd need to use per-cell rpc fixtures?
19:39:26 efried mriedem, tssurya: I'm +2 on https://review.opendev.org/#/c/648662/ - leaving for mriedem to +W.
19:45:04 mriedem oh the pressure is on
19:47:23 ganso mriedem: hey Matt. I am looking at https://review.opendev.org/#/c/658136/ ... it seems it needs https://github.com/openstack/nova/commit/94e620e87cb9349f799007f418ce94978bc33be1
19:47:42 ganso mriedem: Rocky also doesn't have the methods assertFlavorMatchesUsage and assertRequestMatchesUsage
19:48:05 ganso mriedem: do you think it makes sense to backport the refactor?
19:48:44 mriedem no
19:48:55 mriedem there should be assertion methods you can use in queens
19:51:06 ganso mriedem: only assertFlavorMatchesAllocation
19:51:13 ganso mriedem: so the solution would be to cut down on some asserts
19:57:05 mriedem without looking at the test patch, you could rig up your own usage assertion using https://github.com/openstack/nova/blob/stable/queens/nova/tests/functional/test_servers.py#L1514
19:57:17 mriedem not sure if you're asserting provider usage or consumer usage
19:57:39 mriedem i'm assuming the former
19:57:58 ganso mriedem: yup, doing that now. Basically re-coding the test to perform checks as it was done in queens
19:58:06 ganso mriedem: yea provider usage
20:13:21 openstackgerrit Rodrigo Barbieri proposed openstack/nova stable/queens: [DEBUG] Add functional confirm_migration_error test https://review.opendev.org/658136
20:22:17 jaypipes zzzeek: pong
20:22:24 zzzeek hey
20:22:28 jaypipes heya :)
20:22:55 zzzeek jaypipes: time warp back to http://www.joinfu.com/2015/01/understanding-reservations-concurrency-locking-in-nova/
20:24:02 zzzeek so given that galera can have a little bit of read latency: https://www.percona.com/blog/2013/03/03/investigating-replication-latency-in-percona-xtradb-cluster/ can that impact an UPDATE that says, "UPDATE ... SET x='bar' WHERE x = 'foo'" ? if two transactions both update, but write latency means transaction 2 still sees "foo" ?
20:24:34 zzzeek OR, do the UPDATE statements in a write-set get replayed such that they are serialized and it will detect this ?
20:25:00 zzzeek AND, if the latter, what if the two UPDATE statements are changing "x" to the *same* value? this is ultimately a nova question
20:25:28 zzzeek b.c. the issue is observed in the instance shelving logic
20:35:21 openstackgerrit Matt Riedemann proposed openstack/nova master: Add cross-cell resize policy rule and enable in API https://review.opendev.org/638269
20:35:21 openstackgerrit Matt Riedemann proposed openstack/nova master: WIP: Enable cross-cell resize in the nova-multi-cell job https://review.opendev.org/656656
20:35:22 openstackgerrit Matt Riedemann proposed openstack/nova master: Support cross-cell moves in external_instance_event https://review.opendev.org/658478
20:36:43 mriedem zzzeek: a la this bug? https://bugs.launchpad.net/nova/+bug/1821373
20:36:44 openstack Launchpad bug 1821373 in OpenStack Compute (nova) "Most instance actions can be called concurrently" [Undecided,New]
20:37:00 zzzeek mriedem: yes
20:37:23 zzzeek mriedem: mdbooth_ has updated me that his proposed solution won't work
20:39:22 mriedem i think he mentioned something to that effect in irc awhile back for this bug but i don't remember what or why
20:44:06 mriedem gotta run
20:48:57 cdent so many things
20:51:28 cdent zzzeek: since you're sort of around, I wanted to warn you that at some point we might be hassling you for some advice on some performance improvements in placement.
20:51:47 cdent but not now. later.

Earlier   Later