| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-12-02 | |||
| 14:26:49 | gibi | efried: in case of revert_resize we can either start using multiple port binding for resize which is hard but nice, or persist the mapping somewhere | |
| 14:27:01 | gibi | if we go to the later then I would persist it to the Migration object | |
| 14:27:29 | gibi | as the migration object has the proper lifecycle for this type of data | |
| 14:27:37 | gibi | the RequestSpec lives longer | |
| 14:41:25 | sean-k-mooney | gibi: i would like to move to useing multiple port bindings for resize for what its worth | |
| 14:41:38 | sean-k-mooney | gibi: we will be using it for cross cell resize | |
| 14:41:58 | sean-k-mooney | so i think it makes sense to just use it always for resize/cold migrate | |
| 14:46:02 | sean-k-mooney | efried: gibi when ye have time could yet take a look at https://review.opendev.org/#/c/674072/ and maybe https://review.opendev.org/#/c/695118/ | |
| 14:47:31 | sean-k-mooney | ill be going on vacation in 2 weeks but i have a 4 patches i really want to get merged if i can before i do those are two of them and im resping the other two now | |
| 14:49:56 | efried | stephenfin, lyarwood: thoughts on bug tagging here? https://review.opendev.org/#/c/682594/ | |
| 14:51:41 | efried | sean-k-mooney: that might be a bit out of my bailiwick, but I'll give it a try. | |
| 14:53:01 | stephenfin | looking | |
| 14:53:32 | lyarwood | efried: yeah that's fair, I can also address the nits before we merge this afternoon | |
| 14:53:41 | efried | okay | |
| 14:53:48 | lyarwood | efried: I think I was trying to avoid any suggestion that this was the only fix required here | |
| 14:53:58 | stephenfin | Your call. I could go either way. I'm guessing it was left that way since mriedem has a better albeit not backportable fix | |
| 14:54:04 | stephenfin | yup | |
| 14:54:28 | efried | right, I'm looking at it from the perspective of stable, where if we don't Closes-Bug on this patch, there will never be a "fix". | |
| 14:54:58 | efried | I guess if you really want, we could keep this one Related-Bug and switch it to Closes-Bug on the backports | |
| 14:55:07 | efried | can't believe there's not a precedent for something like this. | |
| 14:56:03 | lyarwood | Yeah not that I recall but I'd be okay with that from a stable point of view | |
| 14:56:15 | lyarwood | related on master, closes on stable that is | |
| 14:56:23 | sean-k-mooney | well does it actully fix the bug | |
| 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* | |