Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2019-02-18
14:49:28 stephenfin kashyap: Is there an open bug for s390x before this patch goes in so? https://review.openstack.org/#/c/632507/12
14:50:39 stephenfin kashyap: i.e. if you have a new enough version of libvirt, you'll end up with the duplicate pty devices?
14:58:09 mriedem dansmith: in addition to config i'd think you'd want to disable any down cells via the cli and sighup the scheduler service so we don't waste time trying to pull compute nodes from them for scheduling
14:58:26 mriedem with that bug above i was wondering if filtering disabled cells could be more generally applied
14:58:30 dansmith mriedem: yep, probs
14:58:44 mriedem the min service version check specifically
15:03:56 kashyap stephenfin: No open bug is required.
15:04:08 kashyap Because:
15:06:58 kashyap stephenfin: Scratch that; I need to determine if we really one. But having one is always useful, though.
15:07:31 stephenfin kashyap: It's mostly because if there is a bug, it would be better fix that in a separate precursor patch so that we can backport that wholesale
15:08:04 kashyap stephenfin: No, there is no pre-existing bug. I discovered this the hard way (from zKVM CI), after the version bump analysis
15:08:53 kashyap stephenfin: I see what you mean, though -- I'd have _really_ preferred this to be a separate change ("one logical change per commit"). But I don't think we can do it in this instance - as it's tied to version bump :-(
15:09:20 kashyap (Aside: Do we have any active s390x contributors?)
15:13:58 mriedem no
15:16:32 kashyap Nod. In this case, we're reasonably safe: I checked with the s390x maintainer (for 10-ish years) in QEMU, and she said what I'm doing is sane, FWIW.
15:17:25 kashyap s/reasonably// :D ("Avoid tame, colorless, hesitating, noncommittal language.")
15:19:15 openstackgerrit Matt Riedemann proposed openstack/nova stable/queens: Ignore VolumeAttachmentNotFound exception in compute.manager https://review.openstack.org/637391
15:19:50 mriedem need another core on this api-ref change https://review.openstack.org/#/c/635315/ to unblock this code fix https://review.openstack.org/#/c/509206/ for the longstanding "don't allow az renames when the az has instances in it"
15:26:13 gibi mriedem: I'm looking at the heal_allocation code and I'm wondering how to handle when more than on thing is missing
15:26:16 gibi (e.g. project_id and some port allocation) from the instance allocation:
15:26:22 gibi a) do a separate PUT /allocations/{instance.uuid} for each step
15:26:24 kashyap mriedem: When you get a moment, on this: https://specs.openstack.org/openstack/nova-specs/specs/stein/approved/cpu-model-selection.html
15:26:29 gibi b) generate all the missing allocation of the instance and
15:26:44 gibi do a single PUT /allocations/{instance.uuid} at the end
15:26:45 kashyap mriedem: Do you know if Jack Ding / cfriesen et al are working on it?
15:27:00 gibi #a) has the benefit to allow only healing project_id when there is no bandwidth inventory to heal the port allocations
15:27:08 gibi #b) has the benefit of only calling PUT /allocations once
15:28:18 kashyap gibi: Sorry for breaking your chat, didn't realize it for a moment.
15:28:41 gibi kashyap: no worries I think we break each other chat :D
15:28:55 kashyap (I know there is no 'mutex' on IRC, but still :-))
15:28:59 kashyap Matt: I ask because, there is a related blueprint I filed the other day: https://blueprints.launchpad.net/nova/+spec/cpu-selection-with-hypervisor-consideration
15:32:02 kashyap Anyway, it can be worked out; as no one posted any code for it.
15:32:52 mriedem kashyap: no https://blueprints.launchpad.net/nova/+spec/cpu-model-selection
15:32:58 mriedem no patches proposed
15:33:07 kashyap mriedem: Right, I saw that.
15:33:34 kashyap But most important thing is to rip out existing two CPU-related APIs and use the more sensible APIs that take into account the hypervisor capabilities.
15:33:57 mriedem gibi: i'm assuming (a) is less change in the heal_allocations code
15:34:26 mriedem gibi: for healing missing bw allocations i would hope most of that can be a separate private method that does the check and calculates the allocations to PUT
15:34:52 gibi mriedem: (a) seems a bit less intrusive to the existing code yes
15:35:10 gibi mriedem: yes, I will definitly separate out the bw healing to methos
15:35:13 gibi methods
15:35:37 mriedem gibi: so we already do at most 2 PUTs per instance right?
15:35:40 mriedem first here https://github.com/openstack/nova/blob/master/nova/cmd/manage.py#L1892
15:35:49 mriedem second here https://github.com/openstack/nova/blob/master/nova/cmd/manage.py#L1912
15:35:57 mriedem the second is getting allocations from the flavor
15:36:25 mriedem gibi: so i think for bw allocations, you add another private method and merge the results of scheduler_utils.resources_from_flavor before calling the 2nd PUT
15:37:08 gibi mriedem: if the code needs to create allocation then it is a single PUT
15:37:29 gibi mriedem: also if there is allocation but no project_id then it is also a single PUT
15:37:48 gibi mriedem: there is a return here https://github.com/openstack/nova/blob/e3c24da89aa3e6462f1b07e00659c87f252ba4ba/nova/cmd/manage.py#L1898
15:38:34 mriedem yeah that complicates things...
15:39:09 gibi mriedem: so adding bw to both places means calling my new bw calculation function in both places
15:39:42 mriedem or, get bw allocations first, then if allocations exist but don't have bw resources, skip that return statement and merge for the 2nd PUT
15:40:15 mriedem well,
15:40:32 mriedem i guess you don't want to have to do the work of calculating the bw allocations if you can determine the existing allocations have the bw resource classes
15:41:14 gibi mriedem: I have to check each neutron port attached to the instance to see if there is resoruce_request in the port but no rp uuid in the binding
15:41:37 mriedem yeah i guess the lack of bw in the existing allocatoins does'nt tell us if you need to heal them
15:41:41 mriedem because not all instances will have bw allocations
15:41:51 gibi mriedem: yes
15:42:05 mriedem so it sounds like you need to do that calculation up front anyway
15:42:40 gibi mriedem: yeah, I can do the check and the calculation upfront and merge it to whatwhever PUT it is needed
15:43:05 mriedem we may want an option on the command itself to skip that check
15:43:06 gibi and introduce a new PUT branch to update the bw only, if other data doesn't triggered the PUT
15:43:12 mriedem --skip-bw-allocations
15:43:18 gibi mriedem: ack
15:43:39 mriedem because if my cloud doesn't enable any of that via policy, i don't want this command to waste time spamming the neutron API to figure out that information
15:44:01 mriedem note that this is another argument for stashing the port resource request information in the info_cache :)
15:44:27 mriedem if it was in the info cache we wouldn't need to hit the neutron api for each instance here
15:44:39 gibi mriedem: that cache could be stale when you turn on the policy :)
15:44:57 mriedem that's what we have _heal_instance_info_cache in the compute for
15:45:05 mriedem but sure, you could miss it
15:45:23 mriedem you'd likely know if it's not cached based on some key missing from the cache
15:45:36 gibi mriedem: I'm happy to add a flag to the CLI instead of a cache
15:45:46 mriedem i.e. if 'resource_request' not in VIF - stale cache, vs vif['resource_request'] == None means not stale, but no resource request
15:46:02 mriedem yeah i know :)
15:46:03 mriedem just saying
15:46:44 gibi I hear you. and I still afaraid of cache for this
15:48:38 gibi anyhow I will try to precreate the bw if needed and merge it in to the existing PUTs
16:24:05 openstackgerrit Matt Riedemann proposed openstack/nova master: Add microversion to expose virtual device tags https://review.openstack.org/631948
16:27:21 openstackgerrit Kashyap Chamarthy proposed openstack/nova master: libvirt: Omit needless check on 'CONF.serial_console' https://review.openstack.org/637578
16:44:01 openstackgerrit Matt Riedemann proposed openstack/python-novaclient master: Add support for microversion 2.70 - expose device tags https://review.openstack.org/636779
16:46:59 openstackgerrit Theodoros Tsioutsias proposed openstack/nova master: Enable rebuild for instances in cell0 https://review.openstack.org/570203
16:46:59 openstackgerrit Theodoros Tsioutsias proposed openstack/nova master: Add requested_networks to RequestSpec https://review.openstack.org/570201
16:47:41 openstackgerrit Theodoros Tsioutsias proposed openstack/nova master: Introduce the PENDING instance state https://review.openstack.org/566473
16:47:42 openstackgerrit Theodoros Tsioutsias proposed openstack/nova master: Allow rebuild for instances in PENDING state https://review.openstack.org/637585
16:49:53 zzzeek jaypipes: https://stackoverflow.com/questions/54751671/can-two-innodb-update-statements-against-a-pk-index-deadlock-if-they-are-given-a
16:54:20 jaypipes zzzeek: that's what we include the `WHERE id = ? AND version_id = <our expected version id>` in the WHERE statement.
16:54:27 jaypipes *why* we include... sorry
16:54:45 zzzeek jaypipes: yes, that's my versioning code so I know that part :)
16:55:04 zzzeek jaypipes: in neutron I'm helping them with a case where they want to bump version ids without checking
16:55:23 zzzeek jaypipes: and the approach of just emitting the UPDATE agaisnt PK with the versino_id=version_id+1 works.
16:55:45 zzzeek jaypipes: I think that it will *never* fail, however even if it did produce a deadlock sometimes, it would be a lot less failure than it has now
16:56:03 jaypipes zzzeek: they want to bump version codes without including the "AND version_id = <expected>" in the WHERE statement you mean?
16:56:14 zzzeek jaypipes: yes, they want the number to increment and that's it
16:56:21 zzzeek they dont care what it was previosuly
16:56:25 jaypipes yeah, that's a recipe for disaster.
16:56:28 zzzeek jaypipes: why
16:56:43 jaypipes because it will produce excessive deadlocks, as you are showing here.
16:57:25 zzzeek jaypipes: if you UPDATE against the PKs in the same order each time, it will not. also, this code previously used the "where verison_Id=<>" thing and it failed lots under stress test. now it does not fail at all
16:57:51 zzzeek jaypipes: so, my assertion is, even if it does deadlock, MySQLs deadlocks are immediate and there should be a lot less of them compared to failed compare-and-swaps right now
16:58:16 zzzeek jaypipes: obviosuly for galera multi master everything goes out the window :)

Earlier   Later