| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-07-30 | |||
| 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 | |
| 20:01:43 | mriedem | sean-k-mooney: max_concurrent_builds defaults to 10 | |
| 20:01:45 | mriedem | not 100 | |
| 20:02:04 | sean-k-mooney | mriedem: i know i used to think it wa for the conductor so used to locally set it to 100 | |
| 20:02:17 | sean-k-mooney | since its for compute that is excessive | |
| 20:03:03 | sean-k-mooney | melwitt: yes ture it does not. i have a detailed summay in the bug. ill copy the relevent bit into the commit | |
| 20:03:13 | melwitt | sounds good | |
| 20:03:14 | MultipleCrashes | looks like getting state of the retry count would be tricky, as it comes from within the RetryDecorator | |
| 20:03:40 | MultipleCrashes | for logging | |
| 20:04:25 | melwitt | MultipleCrashes: we don't need the retry count. the suggestion was just to LOG.info('Network allocation failed, retrying') so that the fact that it was retried is captured | |
| 20:04:42 | melwitt | for the benefit of operators | |
| 20:05:18 | melwitt | it's totally fine to add a save and reraise exception in _deallocate_network | |
| 20:06:47 | mriedem | rather than monkey with _deallocate_network directly, just put the call to that in an inner method | |
| 20:07:17 | melwitt | that works too | |
| 20:07:18 | MultipleCrashes | yeah, that handler is a generic exception handler there..we would be retrying only in case we get this specific exception of ConnectFailure | |
| 20:09:07 | MultipleCrashes | so logging in there for 'retrying' would bring a log line even if we have a generic exception | |
| 20:09:20 | mriedem | i left a comment | |
| 20:09:23 | melwitt | in the inner method you catch ConnectFailure only and reraise that, right? | |
| 20:09:35 | mriedem | the RetryDecorator itself will log at debug | |
| 20:10:03 | mriedem | https://github.com/openstack/oslo.service/blob/master/oslo_service/loopingcall.py#L453 | |
| 20:10:36 | mriedem | so it should be easy to do what i added in the review comment | |
| 20:10:47 | mriedem | so we only get 1 ERROR log if everything fails including the retries timing out | |
| 20:11:05 | mriedem | i don't think we need additional extra log messages b/c the decorator itself does that | |
| 20:11:05 | melwitt | I think INFO was suggested since most operators run at INFO log level | |
| 20:11:22 | mriedem | i wouldn't consider this INFO | |
| 20:11:25 | melwitt | you won't know your deallocations are retrying unless you enable debug | |
| 20:11:33 | mriedem | do you need to know that | |
| 20:11:33 | mriedem | ? | |
| 20:11:37 | mriedem | unless you're hitting failures | |
| 20:11:47 | mriedem | and if you're hitting failures, you're probably gonna enable debug on compute | |
| 20:12:10 | melwitt | based on what sean-k-mooney said, it could be a hint that neutron is under load. thought it would be good to know about that | |
| 20:12:17 | mriedem | https://specs.openstack.org/openstack/openstack-specs/specs/log-guidelines.html#log-messages-at-info-and-above-should-be-a-unit-of-work | |
| 20:13:00 | mriedem | if we want to indicate there is a problem, | |
| 20:13:04 | mriedem | i'd say it should be warning | |
| 20:13:26 | mriedem | https://specs.openstack.org/openstack/openstack-specs/specs/log-guidelines.html#definition-of-log-levels | |
| 20:13:41 | MultipleCrashes | hmm | |
| 20:14:14 | mriedem | clearly we need more highly paid engineers to paint this shed :) | |
| 20:17:10 | sean-k-mooney | well if you have an extra brush i would be glad to help :P | |
| 20:18:58 | sean-k-mooney | oh devstack stacked. back to testing stuff | |
| 20:22:04 | s10 | What is the easiest way to change hostname/nodename of the nova-compute host? If this host has instances, and all this instances should think after power on, that they are on host with a new hostname. | |
| 20:23:57 | sean-k-mooney | s10: instance can only tell they moved to a new host because the hashed hostid in the api will change | |
| 20:25:03 | sean-k-mooney | if you change the hosts actull host name and restart the nova compute agent its going to endup with a different uuid as the uuid is based off the hostname by default | |
| 20:26:30 | sean-k-mooney | the instaces would still be assocated with the old compute host so the only simple and safe way to do it that i can think of would be to evactuate or migrate the instances first | |
| 20:27:16 | melwitt | yeah, I was going to say, I think the only supported way to change the hostname stored in an instance record is to migrate the instance | |
| 20:28:52 | s10 | I'm talking about different thing. Lets assume, that /var/lib/nova/instances with vms disks are on the separate md (/dev/md1) on host1.openstack.example.org. Then this host breaks because of the memory or cpu issue, or something else. I want to save data of this vm, so evacuation is not the thing, so I extract disks of the /dev/md1 and put them in another server. | |
| 20:29:19 | s10 | And then I had to set hostname of this new node to the hostname of old server. | |
| 20:29:43 | s10 | But what if I don't want to name other physical machine as old one? | |
| 20:31:17 | s10 | I had to rename node in compute_nodes, services, nova_api, nova.instances, ml2_neutron port bindings... | |
| 20:31:39 | melwitt | yeah, what you're talking about it migrating things manually, so if you're doing that, you'd have to figure out where all you'd need to update database records manually. we don't recommend this | |
| 20:32:18 | sean-k-mooney | s10: you also would have to update placement and the cells dbs in addtion to the normal nova one | |
| 20:33:45 | s10 | Yes, and this is too inconvenient. Would it be better to have some time of evacuate command, that would do all this steps, but without rebuilding instances? | |
| 20:34:22 | sean-k-mooney | s10: the simpelste thing to do would be to pull the disk form the failed node. put them in a working node. migrate off the workloads then rename the new node to whatever it should be called give its physical location | |
| 20:39:09 | melwitt | s10: maybe. I don't recall anyone proposing a spec like that. and I don't know if there are reasons why we don't have a command like that | |
| 20:39:54 | s10 | This is how we do it now. But what if system disk (/) is on small m.2 ssd inside the server. So after the failure of this host we couldn't just take disks of the server, we need to pull server out of rack, open it, take this m.2 out and then put it in another server. | |
| 20:40:27 | melwitt | dansmith: as the evacuate guru, do you happen to know why we don't have a command like "migrate all" from a fenced compute node that's forced down, similar to evacuate but moves disks too? are there tech difficulties around it? I don't remember if it's been proposed before | |
| 20:41:13 | s10 | All this operation will take much more time than just execution of evacuate --force --target-host command to move instances to the reserved compute node, that could be even be powered on all the time | |
| 20:42:58 | dansmith | melwitt: evacuate only works if the node is completely dead | |
| 20:43:11 | dansmith | melwitt: it's destructive, doesn't preserve anything | |