| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-08-12 | |||
| 12:30:53 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: update allocation in binding profile during migrate https://review.opendev.org/656422 | |
| 12:30:53 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Extend NeutronFixture to handle migrations https://review.opendev.org/655114 | |
| 12:34:43 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: prepare func test env for moving servers with bandwidth https://review.opendev.org/655109 | |
| 12:36:42 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: func test for migrate server with ports having resource request https://review.opendev.org/655113 | |
| 12:36:42 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Allow migrating server with port resource request https://review.opendev.org/671497 | |
| 12:50:52 | openstackgerrit | Eric Fried proposed openstack/nova master: Introduces SDK to IronicDriver and uses for node.get https://review.opendev.org/642899 | |
| 12:55:47 | openstackgerrit | Merged openstack/nova master: Cache host to cell mapping in HostManager https://review.opendev.org/663388 | |
| 12:55:55 | openstackgerrit | Merged openstack/nova master: Update comments in HostManager._get_instance_info https://review.opendev.org/674929 | |
| 13:29:11 | stephenfin | sean-k-mooney: | |
| 13:29:20 | stephenfin | sean-k-mooney: I think we just skip them? | |
| 13:29:49 | sean-k-mooney | ok | |
| 13:29:59 | stephenfin | The alternative is to hardcode a mapping of vif types to traits, but maybe that's needlessly complex? | |
| 13:30:10 | stephenfin | Or we can just add a unit test to validate things | |
| 13:30:23 | sean-k-mooney | well i am validating them | |
| 13:30:26 | sean-k-mooney | just later | |
| 13:30:39 | sean-k-mooney | when i am doing the actual update | |
| 13:30:59 | sean-k-mooney | sorry im validating them in the filter | |
| 13:31:06 | sean-k-mooney | not in the dirver | |
| 13:31:26 | stephenfin | Oh, if they're already validated somewhere then it doesn't matter | |
| 13:31:27 | sean-k-mooney | anyway ill skip them and log a warning | |
| 13:31:40 | sean-k-mooney | i should filter them out in the driver | |
| 13:32:08 | sean-k-mooney | but i want to cache the validation function as its not that cheap and i dont want to pay the cost on every request/update | |
| 13:32:53 | sean-k-mooney | it would also be nice to be able to add new vif_models without needing ot update placement first | |
| 13:33:50 | sean-k-mooney | i should still poing out however the validation only ensure that the version of placemetn install in the nova compute node thinks its valid not that the plamcent service thinks its valid | |
| 13:35:11 | sean-k-mooney | we really dont want to be making restcall to placemetn to validate it direclty | |
| 13:35:32 | sean-k-mooney | that would be more expesive then the current validation | |
| 13:38:07 | mriedem | lyarwood: can you hit this queens backport https://review.opendev.org/#/c/659247/ and then we can release queens | |
| 13:38:34 | alex_xu | efried: dansmith, good morning, I updated the vpmem spec https://review.opendev.org/675024 | |
| 13:39:23 | efried | noted | |
| 13:39:53 | sean-k-mooney | oh disabling limit for affinity. i remember talking about that at the ptg | |
| 13:40:22 | lyarwood | mriedem: ack done | |
| 13:42:00 | sean-k-mooney | lyarwood: actully we had a customer issue related to no valid host with anti-affinity that coudl be releated too that? although have said that i dont think we set a limmit so the defualt would be 1000 and then had like 50 host so proably not | |
| 13:42:38 | sean-k-mooney | that would be good for larger deployment in anycase. | |
| 13:43:11 | lyarwood | sean-k-mooney: I can't recall it if we did | |
| 13:43:58 | sean-k-mooney | dhill was handeling it but i tink we concluded they were just running out of space. | |
| 13:44:30 | sean-k-mooney | e.g. the only host that still had enough capsaity were the host the anti afinity filter elminated | |
| 13:45:31 | sean-k-mooney | i dont think there was actully something broken in the end. | |
| 13:52:50 | openstackgerrit | Eric Fried proposed openstack/nova master: Multiple API cleanup changes https://review.opendev.org/666889 | |
| 13:54:48 | efried | stephenfin and/or johnthetubaguy, That ^ is pretty much a "rebase" if you want to reapprove. | |
| 14:00:31 | stephenfin | efried: Sure thing | |
| 14:01:21 | efried | thanks | |
| 14:06:28 | openstackgerrit | Stephen Finucane proposed openstack/nova master: DRY get_flavor in flavor manage tests https://review.opendev.org/668281 | |
| 14:16:05 | efried | stephenfin, johnthetubaguy: If y'all are still in a reviewy mood, this series has my +2 all the way up and its runway slot expires today: https://review.opendev.org/#/q/topic:bp/placement-req-filter-forbidden-aggregates+(status:open+OR+status:merged) | |
| 14:16:42 | efried | for whatever reason, I think it's a pretty cool feature | |
| 14:17:20 | efried | mriedem: Wanna move cross-cell resize into the runway? | |
| 14:18:50 | mriedem | sure | |
| 14:20:03 | openstackgerrit | Filippo Inzaghi proposed openstack/nova-specs master: Add Python 3 Train unit tests https://review.opendev.org/675908 | |
| 14:21:36 | stephenfin | mriedem, efried: Dumb question, but the semaphore here means literally every other kind of operation (delete, (live) migrate, resize, etc.) on that instances is prevented until this eventually, returns, yeah? https://github.com/openstack/nova/blob/master/nova/compute/manager.py#L1867-L1868 | |
| 14:21:50 | stephenfin | Assuming those operations are all wrapped with the same semaphore | |
| 14:22:13 | mriedem | stephenfin: you can't live migrate/resize an instance that's being built | |
| 14:22:31 | mriedem | delete yes https://github.com/openstack/nova/blob/master/nova/compute/manager.py#L2801 | |
| 14:23:54 | stephenfin | Okay, cool. I'm trying to invent reproducers for both https://review.opendev.org/#/c/663382/ and https://review.opendev.org/#/c/667294/ | |
| 14:24:14 | stephenfin | Something that doesn't involve Heat, that is | |
| 14:25:06 | stephenfin | and trying to grok what could be stomping on what. Fun times :) | |
| 14:28:53 | tssurya | efried, mriedem: whenever you have time could you circle back to https://review.opendev.org/#/c/645611/ ? I think you were mostly ok on it, but just want to confirm if there is anything more to do there before I go on vacation | |
| 14:28:57 | tssurya | thanks in advance | |
| 14:35:37 | mriedem | tssurya: i won't +2 that until the ironic side is happy, since without that there is no point in the nova changes | |
| 14:36:15 | tssurya | mriedem: they are afaik, I will ask them to leave a comment in today's meeting | |
| 14:38:22 | dansmith | mriedem: tssurya who is/are the ironic person(s) that we should look to for sign-off there? | |
| 14:38:47 | mriedem | tssurya: also, you're going to have to rebase https://review.opendev.org/#/c/666889/ | |
| 14:39:23 | tssurya | dansmith: dtantsur, TheJulia are the ones who have been reviewing it | |
| 14:39:30 | dansmith | okay | |
| 14:39:54 | tssurya | mriedem: gdi, thanks for letting me know | |
| 14:40:06 | dansmith | hah | |
| 14:40:08 | tssurya | that means I need to again change the version on the ironic side also | |
| 14:40:13 | dansmith | we've ruined tssurya :) | |
| 14:40:27 | tssurya | dansmith: tear emoji | |
| 14:40:28 | mriedem | used to be so innocent | |
| 14:40:45 | mriedem | https://review.opendev.org/#/q/file:%255Enova/api/openstack/compute/rest_api_version_history.rst+status:open | |
| 14:40:49 | dansmith | tssurya: I meant ruined because we've got you swearing when api versions change :P | |
| 14:41:05 | tssurya | dansmith: true that | |
| 14:41:19 | dansmith | tssurya: I say rebase that one on the cleanup to fix the version, and the ironic side, and let's push to get the acks in place so we can try to get it landed before it happens again | |
| 14:41:22 | tssurya | my office mate thinks I am swearing at him | |
| 14:41:28 | dansmith | heh | |
| 14:41:49 | tssurya | dansmith: ack, on it | |
| 14:45:52 | TheJulia | tssurya: will order of merging matter? Trying to figure out if I should try and get more eyes on the ironic side this week | |
| 14:46:32 | tssurya | nova one has to land first technically, but they just want to know if the ironic client side change looks good | |
| 14:47:06 | tssurya | TheJulia: more eyes this week would be great | |
| 14:47:31 | dansmith | what tssurya said | |
| 14:47:52 | TheJulia | tssurya: ack | |
| 14:50:39 | efried | tssurya: Do you have someone to take over for you if changes are needed while you're sipping mai tais on the beach? | |
| 14:58:05 | cdent | mriedem, dansmith : What do you imagine the consequences of moving the get_by_host_node in _update_available_resource out from under the COMPUTE_RESOURCE_SEMAPHORE would be? https://github.com/openstack/nova/blob/52b9359d6c6f387b8b9728c723c9a2501136d605/nova/compute/resource_tracker.py#L744 | |
| 14:58:25 | cdent | on a host with many insgtn | |
| 14:58:28 | cdent | oopes | |
| 14:58:46 | cdent | oh a host with many instances, that lock blocks new instance creation | |
| 15:00:09 | cdent | and a large portion of the time under the lock is spent in get_by_host_and_node assuming the is_bfv cache is hot (if it's cold it is _all_ there) | |
| 15:01:11 | sean-k-mooney | cdent: you might be able to optimise it without fully removing the lock. e.g. caulate the usage for each instance without the lock and keep track of the instace checked. aquire the lock and check that all instace check are still present and no others have been added then update the usage and release the lock. | |
| 15:01:39 | sean-k-mooney | if an instance has been added or removed then you can just make that update under the lock. | |
| 15:02:27 | mriedem | cdent: the consequence could be a race where we don't report usage or claim properly because the underlying resource usage changed since we got the lock | |
| 15:03:26 | cdent | mriedem: are you certain? I'm not saying remove the lock. I'm saying not use the lock while getting the list of instances used by _update_available_resources | |
| 15:03:37 | mriedem | i know what you're asking | |
| 15:03:37 | cdent | lock _after_ getting the list | |
| 15:03:57 | mriedem | you're saying move the get here https://github.com/openstack/nova/blob/52b9359d6c6f387b8b9728c723c9a2501136d605/nova/compute/resource_tracker.py#L710 | |
| 15:04:05 | mriedem | and pass the instances into _update_available_resource | |
| 15:04:20 | mriedem | sean-k-mooney: how would you know if an instance has been added or removed? | |
| 15:04:28 | mriedem | between the time you got the list and got the lock? | |
| 15:05:03 | mriedem | cdent: presumably you could have: | |
| 15:05:07 | sean-k-mooney | you would have to key two list of instances. one that you got outside the lock and then inside it you would get the list again | |
| 15:05:39 | sean-k-mooney | you can then caulated teh added and removed instances | |
| 15:05:46 | mriedem | t0: thread 1 is getting the list of instances on the host, t1: thread 2 gets the lock and is adding a new instance claim to the host which won't be in the list returned in thread 1, t2: thread 1 gets the lock with a list of instances that doesn't include the one from thread2 | |