Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2018-07-30
18:52:15 efried hypothetically if this change was made, even with a microversion
18:52:16 mriedem set with unicode on newer microversion, retrieve with older microversion?
18:52:23 dansmith mriedem: yeah
18:52:37 mriedem dansmith: idk, detecting that would suck
18:52:42 dansmith mriedem: yup
18:52:46 jaypipes efried: I asked a question on the patch.
18:52:47 dansmith mriedem: and be super confusing for people
18:53:28 mriedem this is generally why we have specs for api behavior changes.... :)
18:53:39 dansmith ahyup
18:53:43 efried jaypipes: In the discussion from a few weeks ago (linked in the bug report) we talked about it not being a good idea for extra specs. I think it was in reaction to that that they reverted that part.
18:53:45 jaypipes although I do like dansmith's pile of poo resource.
18:53:56 dansmith jaypipes: one pile of poo please, affined to numa node #2
18:54:07 jaypipes side of fries with that, dansmith?
18:54:17 dansmith jaypipes: only after some hand sanitizer
18:54:21 jaypipes :)
18:54:29 efried affinitizations for 914 points
18:55:25 sean-k-mooney mriedem: does the api activly reject unicode?
18:55:46 efried sean-k-mooney: We're talking about in metadata/extra_specs keys, where the schema is patters
18:55:51 efried pattern-limited to ascii.
18:55:58 efried so yeah
18:56:24 melwitt efried: I'm going to comment on the patch
18:56:41 efried melwitt: Okay, I was about to update the bug.
18:56:42 sean-k-mooney efried: oh ok i was going to say we dont mandate a coralation type for the db so someone could have created a db with utf-8 set and would be able to store it
18:56:59 mriedem sean-k-mooney: can you answer this question to stephen? "where does the libvirt driver actually translate hw_cpu_policy and hw_cpu_thread_policy into something that goes in the guest xml?"
18:57:04 melwitt efried: feel free to do that
18:57:13 openstackgerrit Chris Dent proposed openstack/nova master: [placement] Use oslotest CaptureOutput fixture https://review.openstack.org/587129
18:57:14 openstackgerrit Chris Dent proposed openstack/nova master: [placement] Use a non-nova log capture fixture https://review.openstack.org/587130
18:57:15 openstackgerrit Chris Dent proposed openstack/nova master: [placement] Use a simplified WarningsFixture https://review.openstack.org/587131
18:57:35 efried melwitt: Procedurally, if we've deemed this to need a bp/spec, do I set the bug to Won't Fix?
18:57:55 mriedem i don't think so, it would be a wishlist bug
18:58:01 mriedem invalid -> wishlist or something
18:58:05 sean-k-mooney mriedem: let me see if i can find it. i can i can give you the relevent xml snipit it generates
18:58:12 efried ight
18:58:37 mriedem all i mostly see is the giant hardware.py methods,
18:58:43 mriedem but can't link those up to where it's used by a driver
18:59:14 mriedem maybe it's not directly set in the guest xml? maybe it's just used to determine which cpus to pin?
19:00:01 sean-k-mooney mriedem: its burried in the numa code
19:00:19 sean-k-mooney mriedem: yes it just used to determin the pinning
19:00:21 efried melwitt, mriedem: Do we have a helpful contributor link to the bp/spec process?
19:00:28 mriedem yes
19:00:34 sean-k-mooney it never gets into the xml itself
19:00:40 mriedem https://docs.openstack.org/nova/latest/contributor/blueprints.html
19:00:48 mriedem sean-k-mooney: ok then, that answers that, thanks
19:01:11 efried ack
19:02:14 sean-k-mooney mriedem: bassicaly we generate teh pinning here https://github.com/openstack/nova/blob/master/nova/virt/libvirt/driver.py#L4471-L4480
19:07:13 MultipleCrashes Anyone free to take up this review further : https://review.openstack.org/#/c/563418/
19:21:04 openstackgerrit Eric Fried proposed openstack/nova master: Updated AggregateImagePropertiesIsolation filter illustration https://review.openstack.org/586317
19:25:45 openstackgerrit karim proposed openstack/nova master: Updated AggregateImagePropertiesIsolation filter illustration https://review.openstack.org/586317
19:26:55 melwitt MultipleCrashes: are you asking for review or help with updating the patch or both?
19:28:10 MultipleCrashes I am new to it, in my knowledge we need a +2 for a merge ..mostly looking for a possibility of merge
19:30:37 sean-k-mooney MultipleCrashes: just looking at the bug you are getting a keysonte error form calling neutronport delete in a bulk delete of nova instnaces.
19:31:01 sean-k-mooney this almost looks like we are ddosing the neuron api with too many requests at once
19:31:24 sean-k-mooney retry is certenly one want to solve it but perhaps we should be ratelimiting
19:33:04 MultipleCrashes yeah , apparently this happens when too many instances are simultaneously deleted , if we try a rate limiting there is possibility that the task of deleting the VMs might get interrupted.
19:33:38 MultipleCrashes Eg:lets say we are deleting 1000 instances and the problem occurs at 550th (say) instance
19:34:21 MultipleCrashes we would still like to continue the process, probably rate limit might stop further execution
19:34:30 melwitt MultipleCrashes: the last comment on the review suggests a change to avoid logging error per retry and instead log info for the retry and then if all retries have failed, log error. otherwise the operator gets a false log error if one of the retries succeeds
19:35:14 melwitt and by false I mean, operators consider "error" to mean action should be taken
19:36:06 sean-k-mooney MultipleCrashes: well the error is stemming from data = neutron.list_ports(**search_opts). so in your case we would be doing 1000 concurrent requets to netron to list the ports for the 1000 instnaces.
19:36:11 MultipleCrashes yeah, since with the retries we would also be having instance ids, so we would be able to figure out that the retires are being done for this particular intance
19:37:25 openstackgerrit Matt Riedemann proposed openstack/nova master: Fix formatting for vcpu_pin_set and reserved_huge_pages https://review.openstack.org/587206
19:37:39 sean-k-mooney melwitt: we have a rate limit for creatign new instaces. do you know is there an equivalent for delete?
19:37:53 MultipleCrashes the deletion of instances would happen one-at-a-time and hence, only the retry will be for that particular instance.
19:38:20 dansmith we don't have rate limiting for any API methods anymore, that I know of
19:39:26 sean-k-mooney MultipleCrashes: im not sure about that. i would expect the api/conductor to call down to the compute nodes to do the delete and for those deleteions to work in paralle but i have not looked at that code path.
19:39:47 sean-k-mooney dansmith: well this would not be an api ratelimit it would be a limit in the conductor i guess
19:39:55 dansmith sean-k-mooney: MultipleCrashes means we have no bulk delete api call
19:39:57 mriedem nova-api does an rpc cast to the compute that is hosting the instance
19:40:00 dansmith sean-k-mooney: so of course, all of them happen in parallel
19:40:15 dansmith sean-k-mooney: we have no rate limits in conductor either
19:40:27 dansmith sean-k-mooney: we have the build and migrate counters in compute, but those aren't per-tenant
19:40:36 dansmith but definitely don't have any such limits on delete
19:40:53 sean-k-mooney dansmith: oh ok then ya i guess retry is the best we can currently do then.
19:41:08 MultipleCrashes yeah, mostly is done via autoscale feature
19:41:36 MultipleCrashes while scaling down
19:41:41 melwitt MultipleCrashes: the suggestion isn't to use instance ids to figure out whether it's a retry. the suggestion is to move the retry decorator to the inner method _deallocate_network, so that the log error in _try_deallocate_network won't happen each retry attempt
19:42:20 MultipleCrashes yeah, moving to _deallocate_network would mean we will have to save_and_reraise exception in that function
19:42:46 mriedem "otherwise the operator gets a false log error if one of the retries succeeds" is definitely annoying and a red herring when you're actually trying to debug something,
19:42:47 MultipleCrashes as we are doing retry based on exception which we are handling in _try_deallocate_network
19:42:59 mriedem i know there is a persistent case of that in cinder-volume during volume delete i think which always throws me off
19:43:05 mriedem because it logs an error, then retries and succeeds
19:44:13 MultipleCrashes this seems to be an intermittent issue, once in many time..possibly caused by transient network connectivity problem.
19:45:12 melwitt MultipleCrashes: why? won't the RetryDecorator catch the ConnectFailure and retry and once retries expire it will propagate ConnectFailure up to _try_deallocate_network?
19:46:02 sean-k-mooney MultipleCrashes: it could be connectivity but its more likely that its due to the number of neutron api calls.
19:46:46 mriedem i thought at one point the bug said it was a keystone issue?
19:47:06 sean-k-mooney mriedem: its a keysone connection failure on list port
19:47:19 mriedem does it re-use the same token to delete all 1000 instances and the token times out?
19:48:34 sean-k-mooney mriedem: i cant tell form "ConnectFailure: Unable to establish connection to http:/somehost:someport/v2.0/ports.json?device_id=someid"
19:48:35 MultipleCrashes yeah , tried the way with _deallocate_network RetryDecorator .. doesn't function properly.Yeah agree neutron load via no of api calls could be a likely reason
19:49:48 sean-k-mooney mriedem: the error is propagating form the keystone auth session _send_request method but i dont think its a keysone issue
19:50:17 melwitt MultipleCrashes: ok, it would be helpful to reply to the review comment and let the reviewer know why their suggestion doesn't work. fwiw, I thought it would have worked too
19:51:18 melwitt oh, my mistake, I guess the suggestion does say to add log info and reraise
19:52:25 melwitt in the inner method. what I said, it wouldn't be possible to log the info part to say "retrying"
19:53:24 melwitt it would be better to log the "retrying ..." so operators can know if they have retries going on for the network deallocation
19:54:27 sean-k-mooney dansmith: ya the compute node max_concurrent_builds config option is the one i was thinking of originally but i had tought that was in the conductor. i guess not.
19:59:12 sean-k-mooney melwitt: mriedem by the way do we care about https://review.openstack.org/#/c/584999/ for rocky or will i loop back to it in stien?
20:00:24 mriedem sean-k-mooney: i can -1 it for any release based on the commit message if you want
20:00:57 sean-k-mooney hehe well if you want any changes please do
20:01:18 mriedem done
20:01:32 melwitt sean-k-mooney: bugs can be fixed any time so you don't need to target it to a specific release. that said, I agree the commit message doesn't explain anything about what's wrong or how/why the patch fixes it

Earlier   Later