Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2019-12-02
14:56:30 sean-k-mooney or just make it less bad
14:56:58 openstack Launchpad bug 1844296 in OpenStack Compute (nova) "Stale BDM records remain in the DB after n-api to n-cpu RPC timeouts during reserve_block_device_name" [Undecided,In progress] - Assigned to Lee Yarwood (lyarwood)
14:56:58 sean-k-mooney ye are talking about https://bugs.launchpad.net/nova/+bug/1844296 right
14:57:15 sean-k-mooney and this patch https://review.opendev.org/#/c/682594/
14:57:25 lyarwood sean-k-mooney: yeah, it cleans up after one type of failure, it doesn't stop the failure from happening.
14:57:38 sean-k-mooney ah i see
14:57:51 sean-k-mooney so its treating the symtoms not the cause
14:58:11 lyarwood sean-k-mooney: yeah and very specific symptoms at that.
14:58:47 sean-k-mooney you could make it partial-bug if you plan to try and fix it someother way on master in a follow up patch
14:59:19 lyarwood ah yeah I forgot about partial
14:59:21 lyarwood that might do
14:59:37 sean-k-mooney related bug makes sense too but i can see why you might what cloeses bug for stabel if you dont think any complete fix woudl be backportable
15:00:41 sean-k-mooney Partial-Bug i think is the most correct but that wont mark it as "fixed" on stable
15:00:45 mriedem see my comments from PS4
15:00:50 mriedem 1. Can we sort out the various bugs I've mentioned to see which ones this fixes and then duplicate whatever is a duplicate and decide on one (probably the oldest) bug for the same issue.
15:00:55 mriedem 2. I don't trust unit tests for this type of stuff since it involves (a) more than one service and (b) the database. Can we write a functional test to recreate the bug and then assert the fix resolves it?
15:01:26 mriedem i believe i sorted out the bazillion related bugs here https://review.opendev.org/#/c/692940/
15:02:35 lyarwood gah, I'm not sure how I missed that, sorry mriedem
15:03:03 mriedem probably distracted by all of the comments inline about races and locks and such that might mean this doesn't fix a bug
15:04:14 openstackgerrit Dan Smith proposed openstack/nova master: Add a way to exit early from a wait_for_instance_event() https://review.opendev.org/695985
15:10:00 mriedem if you want i could write the functional test if you don't want to bother with that
15:11:40 mriedem lyarwood: since the bdm.create is the last thing that happens on the compute, in what case would this cleanup catch and actually clean anything up?
15:11:55 mriedem like just the split second that we actually created the record and then got a messaging timeout?
15:13:03 lyarwood mriedem: happy to write the func test
15:13:26 lyarwood mriedem: and yeah but that's asssuming that is a split second and n-cpu isn't stuck somehow
15:13:38 lyarwood mriedem: as mdbooth suggested
15:14:02 lyarwood mriedem: FWIW we have seen this downstream at least, so either way it's possible to hit and reasonable to cleanup
15:14:16 mriedem sure, so if n-cpu is stuck on let's say getting the instance uuid lock, or doing something in the driver like _get_device_name_for_instance, we wouldn't create the bdm in that case anyway
15:14:32 mriedem where/how was it triggered downstream?
15:14:48 mriedem slow compute and the api timed out?
15:15:05 mriedem because if so, the change to make reserve_block_device_name using the long_rpc_timeout will fix that
15:15:54 lyarwood mriedem: https://bugzilla.redhat.com/show_bug.cgi?id=1752734 - I didn't get a root cause out of this as the env wasn't logging in DEBUG unfortunatley
15:15:55 openstack bugzilla.redhat.com bug 1752734 in openstack-nova "Invalid bdm record remains when reserve_block_device_name rpc call times out" [High,On_dev] - Assigned to lyarwood
15:16:16 lyarwood mriedem: but we did end up with stale BDM records in the DB after seeing timeouts
15:16:53 sean-k-mooney lyarwood: ok so there is a customer case attached to that so its not a ci issue
15:17:38 mriedem 3. nova-api again requests to nova-compute to do attaching operation 4. nova-compute updates bdm with retrieved connection info 5. if some error like timeout detected between 3-4, remove bdm record created at 2
15:17:43 mriedem so that's not the same thing
15:18:35 mriedem that's (1) reserve_block_device_name worked and created a bdm and then (2) when the api cast to attach_volume something timed out and the bdm was orphaned
15:19:37 lyarwood mriedem: yeah I think that description is wrong, there's reserve_block_device_name failures documented in the next private comment of the bug
15:20:26 lyarwood sean-k-mooney: ^ can confirm that, however I don't think we can share the actual logs here.
15:20:49 mriedem even so, the change to make reserve_block_device_name using the long_rpc_timeout will fix that
15:20:58 mriedem but i don't think that long rpc timeout goes back to queens
15:21:21 sean-k-mooney am i was reading it and im not sure the comment need to be private. we cant share the raw logs but i do not see antynig in comment 1 that is really an issue
15:21:36 mriedem if it's just a messagingtimeout i can understand that
15:21:48 sean-k-mooney MessagingTimeout: Timed out waiting for a reply to message ID fa3d55d2d8c248ba82a13c940350ea9c
15:21:49 lyarwood super secret volume uuids
15:21:52 lyarwood ;)
15:22:10 sean-k-mooney so the long rpc might fix it ya
15:22:14 mriedem it's a 60 second timeout by default with a blocking api - if the compute is slow or we're waiting on the instance uuid lock (maybe because a periodic is doing something with the instance at the time of attach), we could reasonably timeout
15:22:37 mriedem https://review.opendev.org/#/c/566696/ only goes back to rocky though, so you can't get the long_rpc_timeout fix to queens
15:22:43 mriedem so no use in saying that's the backportable fix
15:23:33 mriedem anyway, it just seems like a really small window where the fix in the api would handle it since the bdm create is the last thing to happen - if we timeout before that we still orphan the bdm record
15:23:54 mriedem and i know i'm contradicting my "what's the harm?" replies to mdbooth in the comments :)
15:24:07 mriedem the better longer term fix is the wip i posted, but that's not backportable
15:24:45 lyarwood yeah agreed on that part, we could backport long_rpc_timeout downstream *cough*
15:24:51 lyarwood but yeah
15:25:07 openstack Launchpad bug 1425352 in OpenStack Compute (nova) "A volume remains attached and cannot be detached after attaching it fails" [Low,Confirmed]
15:25:07 mriedem also your bug is definitely a duplicate of https://bugs.launchpad.net/nova/+bug/1425352
15:25:59 sean-k-mooney lyarwood: the issue would be the oslo.messaging min version bump
15:26:24 mriedem sean-k-mooney: correct we're not backporting that to queens-em upstream
15:27:14 lyarwood okay that's likely not an issue downstram
15:27:19 lyarwood downstream*
15:27:26 sean-k-mooney i dont know
15:27:37 sean-k-mooney i would not nessisarliy be comfrotable with that
15:27:46 sean-k-mooney but we could explore it as an option
15:28:07 sean-k-mooney it would depend on what version of oslo.messaging we are actully using downstream
15:28:16 lyarwood >= 5.29.0
15:28:29 lyarwood sean-k-mooney: when did this come in?
15:28:34 sean-k-mooney 6.3.0
15:29:08 lyarwood ah
15:29:11 lyarwood that would be an issue
15:29:17 sean-k-mooney https://github.com/openstack/requirements/blob/stable/queens/upper-constraints.txt#L145
15:29:23 sean-k-mooney so queens was capped a 5.35.6
15:29:31 sean-k-mooney so 6.3.0 proably will break things
15:29:31 lyarwood yeah guess what we use downstream
15:29:39 lyarwood okay that's dream is dead
15:29:42 lyarwood that*
15:30:14 sean-k-mooney is it higher then the upper-constraints for queens? cause i hate when we do that
15:30:42 lyarwood no it's not
15:30:52 sean-k-mooney cool
15:30:57 mriedem you could create your own workaround option as a timeout for this specific call that overrides the default rpc_response_timeout just for your queens-only change and drop it for long_rpc_timeout when you start using that for this call from upstream
15:32:44 sean-k-mooney ya is there an advantage to not useing the same config option
15:33:01 sean-k-mooney is it jsut to make it clear it applies to just this one call
15:37:24 mriedem lyarwood: i left some comments in the patch to try and summarize irc discussion and duplicated the bug to the existing one for the same issue. i'd hold off on writing a functional test until more people weigh in on what the correct direction is here given backports and such.
15:38:02 gibi sean-k-mooney: could you quickly respin https://review.opendev.org/#/c/695118/ to fix stephenfin nits in the conf doc and the release notes? The change looks good to me too but I guess you want to backpor it so a follow up won't help
15:38:58 sean-k-mooney yep i can do that in 5 mins thanks for taking a look
15:39:35 gibi sean-k-mooney: thanks. Ping me and I will +2
15:39:50 mriedem would be nice if any of the blizzard people would ack that change since they reported it
15:45:01 mriedem stephenfin: tbc i'm waiting on vmware ci results on https://review.opendev.org/#/c/696503/ before moving it forward
15:45:33 mriedem if that ci is busted (let's say we don't get results within the next 24 hours or something) then i'm cool with moving forward since if they can't maintain a working ci then we can't maintain their driver for them
15:45:43 stephenfin coolness
15:55:35 sean-k-mooney ... the release notes tox env takes forever to run
15:59:30 openstackgerrit sean mooney proposed openstack/nova master: add [libvirt]/max_queues config option https://review.opendev.org/695118
15:59:45 sean-k-mooney gibi: stephenfin ^ nits addressed no other changes
15:59:53 gibi sean-k-mooney: looking...
16:01:31 gibi eandersson: the bug is coming from blizzard ^^ could you check the bugfix from your perspective?
16:02:26 sean-k-mooney i will be filing and fixing a related bug for vhost-user seperatly but that should cover the case in the current bug if not let me know.
16:08:32 sean-k-mooney actully thinking about that i wonder if i should also add extra code to handel updating this on live migration?
16:11:12 sean-k-mooney i dont think we have code for the queue lenghts on live migration so i kind of feel a seperate patch to handel both queue lenght and max queues would make sense since that will need object changes and wont be backportable where as this simple fix is
16:13:15 sean-k-mooney gibi: stephenfin do you think that would be good to do as a followup? looking at the migrate data object we dont pass any of that info today

Earlier   Later