Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2019-07-19
14:14:43 efried kisses
14:14:44 gibi :)
14:19:49 gibi I'm +2 on the provider config spec
14:20:24 gibi and I guess it is good time to inform you that I will disappeare for two weeks of vacation starting in an hour
14:22:52 efried phew, then I'm glad I nabbed you for that +2. Have a great vacation gibi!
14:23:02 gibi efried: thanks!
14:28:46 cdent enjoy your time off gibi
14:28:59 gibi cdent: thanks
14:31:51 dansmith artom: you have to fix your many typos anyway, so I think you should move the code to a single check to LOG.warning on startup and then I'd be +2 if not for needing some diversity on it
14:32:16 artom dansmith, in _init_host?
14:32:27 dansmith that'd be startup yeah :)
14:32:27 artom Yeah, that'd make sense
14:32:46 sean-k-mooney if you put it in _init_host i would perfonally make it stop the agent booting
14:32:57 dansmith no
14:33:04 artom It wouldn't stop it
14:33:10 artom But... it might take a long time
14:33:12 dansmith this should be a LOG.warning
14:33:13 sean-k-mooney im fine with it just being a warning to
14:33:21 artom Not sure if that's acceptable
14:33:35 dansmith artom: also a reno, just in case anyone is relying on this to do their homework for them
14:34:20 dansmith artom: how long does this take? I'm not sure why it'd be slow really
14:34:29 artom The bug's env has > 1000 nics, they timed it to over 2 minutes
14:34:40 artom ==========
14:34:41 artom $ time python -c "import nova.compute.utils; nova.compute.utils.get_machine_ips()"
14:34:41 artom real 2m32.642s
14:34:41 artom user 0m4.358s
14:34:41 artom sys 2m27.918s
14:34:41 artom ==========
14:34:59 efried wow
14:35:14 dansmith which thing takes long? listing the nics themselves?
14:35:20 dansmith because that really shouldn't take that long
14:35:26 efried yeah, is there not a quicker way to find out if my IP is on the system?
14:35:35 efried ifconfig -a | grep ??
14:35:53 artom efried, could be that "dumb", yeah. Would we be cool with this?
14:36:03 dansmith if it's the list, then not much can be done, but if it's our processing, we could limit to 100 nics and bail after, or something
14:36:05 artom ifconfig isn't on every system nowadays, but `ip` is
14:36:12 efried considering what we're shooting for here, I would be totally fine with something that "dumb".
14:36:14 dansmith no, don't do ip|grep
14:36:27 efried if we were relying on this for real functionality that would be one thing
14:36:30 dansmith this has been in place for four years and one person has complained,
14:36:34 efried but this is a best-effort sanity check
14:36:40 dansmith so I think once at startup which is way less impactful should be fine
14:36:47 artom dansmith, well, thousands of nics aren't exactly common :)
14:36:53 dansmith right
14:37:07 dansmith which is why it's not a problem to do this for the 99.999% I think
14:37:15 efried and as dansmith noted in the original, it's possible for it to fail and everything still be alright.
14:37:17 artom I'm just worried there might be something that implicitly relies on compute service starting up in less than a few seconds
14:37:33 dansmith artom: we have tons of things that are long-running in compute startup,
14:37:35 dansmith so that's not a concern
14:37:54 dansmith we block compute startup until we can talk to conductor in both directions, we clean up failed migrations, do disk IO, etc, etc
14:38:35 artom dansmith, ack, sounds like we have a way forward with moving the warning to _init_host then
14:42:07 artom (dansmith, btw, I suspect it's https://github.com/openstack/nova/blob/master/nova/compute/utils.py#L977 that's the time sink, not https://github.com/openstack/nova/blob/master/nova/compute/utils.py#L975, but that's moot, and unless we ask the reporter we'll never know)
14:42:44 dansmith artom: yeah I just don't know _why_ it'd be slow, unless it's trying to reverse lookup all the addresses too
14:43:10 artom Yeah, good question...
14:43:24 openstackgerrit Eric Fried proposed openstack/nova-specs master: Followup for provider config file spec https://review.opendev.org/671749
14:43:35 artom If it was doing reverse lookups I think it'd be slower, because a single loopkup takes forever to timeout
14:43:36 efried dansmith: Put the ironic NOTE in a fup so as not to lose gibi's +2 ^
14:44:18 dansmith artom: that call is implemented in C even
14:47:31 alex_xu stephenfin: sean-k-mooney thanks in person
14:47:57 alex_xu sean-k-mooney: buyin your proposal about 1 extraspec and mask
14:52:10 alex_xu artom: ah...claim depends on migrate_data, just aware that, I need to dig into your patch more
14:52:40 artom alex_xu, hey, if you find a way to make it work, I'm all for it :)
14:53:28 alex_xu at least understand the dependence...and I pretty sure people will hate that I added a interface call get_vpmems :)
14:55:04 artom alex_xu, yeah, that'd make 2 libvirt-specific interfaces that no other driver uses
14:55:22 alex_xu yea
14:56:51 artom Maybe consolidate them into something like "get_driver_specific_migrate_data"?
14:56:58 artom Not super happy with that either
14:57:49 efried fwiw, I think we need *something* like this ^
14:58:08 efried although... I thought we already *had* driver-specific migrate data
14:58:20 artom Exactly
14:58:29 artom It's more like... claims-dependant driver-specific
14:58:32 artom Which is a mouthful
14:59:27 artom Or... post-claim migrate_data
14:59:30 artom Would that work?
15:00:04 openstackgerrit Stephen Finucane proposed openstack/nova master: Use a less chipper title for release notes https://review.opendev.org/671752
15:02:01 artom driver.migrate_data_from_claim(context, instance, claim)
15:02:11 artom efried, alex_xu ^^ how does that sound?
15:03:47 efried artom: What are these LiveMigrateData subclasses for, if not for this?
15:04:01 dansmith I'm not following this,
15:04:09 dansmith but we do have per-driver migrate data for exactly this reason
15:04:11 artom efried, it's a sequencing problem
15:04:15 dansmith and that's what those subclasses are
15:04:18 artom See https://review.opendev.org/#/c/634606/44/nova/compute/manager.py@6482
15:04:50 artom It's not a migrate_data class problem, it's "we need to call the driver twice, once for check_can_live_migrate_destination, and once after we've done claims"
15:05:04 artom In my patch, that second call is very use-case specific "get_dst_numa_config"
15:05:36 sean-k-mooney for what its woth we do something similar for sriov
15:05:39 artom alex_xu will need to solve the same problem, so rather than adding "get_vpmem_config" or whatever to the driver interface, we're trying to come up with a more generic single method for that second call to driver
15:06:22 alex_xu I'm trying to figure out why you need the migration_data for the claim
15:06:40 artom Other way around, we need to claim to update the migrate data
15:06:51 artom Because we get the NUMA topology on the dest host from the claim
15:09:42 alex_xu artom: looks like you only want to know whether instance has numa topo https://review.opendev.org/#/c/634606/44/nova/compute/manager.py@6470
15:10:13 sean-k-mooney for sriov migration we added https://review.opendev.org/#/c/620115/35/nova/compute/resource_tracker.py which we callin in https://review.opendev.org/#/c/620115/35/nova/conductor/tasks/live_migrate.py
15:10:28 sean-k-mooney * in check_can_live_migrate_destination
15:11:34 artom alex_xu, in my case, yeah
15:12:29 alex_xu artom: I don't understand this https://review.opendev.org/#/c/634606/44/nova/virt/libvirt/driver.py@7463
15:13:00 artom alex_xu, it's to handle the N/N-1 problem
15:13:04 alex_xu what different with we check instance.numa_topology at dest host compute manager
15:13:13 alex_xu oh, upgrade issue
15:13:18 artom Exactly
15:13:35 sean-k-mooney alex_xu old host wont populate that field

Earlier   Later