| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-05-10 | |||
| 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 | cell_contexts_by_host[host] = instance._context | |
| 18:18:57 | mriedem | if host not in cell_contexts_by_host: | |
| 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 | dansmith | and putting a comment like that seems like something you would have made me do :) | |
| 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: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,970 DEBUG [nova.compute.manager] Received event network-vif-plugged-60c6c823-ed75-46a0-a0c6-30aadb90e78c' | |
| 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: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: WIP: Enable cross-cell resize in the nova-multi-cell job https://review.opendev.org/656656 | |
| 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: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. | |
| 20:51:54 | zzzeek | cdent: that should be good since placement looks to be straightforward | |
| 20:52:03 | cdent | it _was_ | |
| 20:52:53 | cdent | but the nested stuff being added this cycle are going to be mind bending | |
| 20:55:10 | zzzeek | cdent: what kind of nesting | |
| 21:07:09 | edleafe | zzzeek: representing compute nodes containing NUMA nodes containing memory, for example | |