| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2020-08-05 | |||
| 16:43:38 | stephenfin | yup | |
| 16:43:52 | melwitt | ok I see | |
| 16:45:34 | stephenfin | I think I'm close to a fix at least. Worst case scenario, we put mriedem's stuff inside a conditional to only run if we still know that it exists | |
| 16:45:52 | stephenfin | i.e. if it's in the list of tracked migrations and instances | |
| 16:46:17 | stephenfin | though I haven't figured out what that will leak yet | |
| 16:46:22 | stephenfin | tbd | |
| 16:47:22 | melwitt | sorry, what's the proposed fix? being that it's intended that resources be held on source and dest until confirm or revert | |
| 16:48:13 | stephenfin | https://github.com/openstack/nova/blob/master/nova/compute/resource_tracker.py#L579-L591 | |
| 16:48:29 | stephenfin | we only do that if the instance appears in our list of tracked_migrations or tracked_instances | |
| 16:50:44 | stephenfin | afaict, '_update_usage_from_instances' wipes those and regenerates them. When we confirm the resize, the instance is marked as deleted, which means it's not picked up by '_update_usage_from_instances' | |
| 16:51:22 | stephenfin | so the race is between that (the instance getting marked as deleted and '_update_usage_from_instances' running) and the call to 'drop_move_claim' | |
| 16:51:30 | stephenfin | I _think_ | |
| 16:58:56 | sean-k-mooney | gibi: thanks ill restart with that now and see how it works | |
| 17:00:22 | melwitt | stephenfin: this is interesting bc the commit message of mriedem's change says "This fixes the issue by also updating usage in drop_move_claim when the instance is not in tracked_migrations but is in tracked_instances." (which would do part of what you're suggesting) but it appears not to be what is in the code | |
| 17:00:29 | gibi | sean-k-mooney: thanks. I will end my day about now but I will be back tomorrow morning | |
| 17:00:34 | sean-k-mooney | stephenfin: sound plausible i had not got to the point of diging into how the race happens | |
| 17:00:54 | sean-k-mooney | gibi: no worries | |
| 17:03:52 | melwitt | stephenfin: oh, nvm, I'm misreading this... it's removing usage by removing the instance from tracked_instances and then updating usage | |
| 17:08:48 | sean-k-mooney | melwitt: stephenfin if this race is happening the way you say | |
| 17:09:00 | sean-k-mooney | would this also happen with the old config options? | |
| 17:09:07 | stephenfin | sean-k-mooney: I think so | |
| 17:09:18 | sean-k-mooney | ok its just more obvious now? | |
| 17:09:25 | stephenfin | I think the bug is with that patch from mriedem | |
| 17:09:28 | sean-k-mooney | or they were just unlucky | |
| 17:09:38 | stephenfin | I don't think it's anything to do with the PCPU work | |
| 17:09:43 | sean-k-mooney | ok | |
| 17:10:16 | stephenfin | tbc, it's still a working theory but it would be easy prove out by backporting that fix to e.g. stable/stein where none of that code is present | |
| 17:10:18 | sean-k-mooney | i think the upgrade procedure we proposed for our downstream customer still makes sense | |
| 17:10:33 | sean-k-mooney | but i think they will need the backport of your two fixes | |
| 17:10:36 | sean-k-mooney | the one for this race | |
| 17:10:49 | sean-k-mooney | and the one for the isolate on smt host with new config options | |
| 17:10:51 | sean-k-mooney | right? | |
| 17:11:56 | stephenfin | the isolate on SMT fix is mostly unrelated and should be backported regardless | |
| 17:12:01 | stephenfin | still unsure about this race | |
| 17:12:14 | sean-k-mooney | ok | |
| 17:51:10 | openstackgerrit | Stephen Finucane proposed openstack/nova master: Don't unset Instance.old_flavor, new_flavor until necessary https://review.opendev.org/744958 | |
| 17:51:10 | openstack | bug 1879878 in OpenStack Compute (nova) "VM become Error after confirming resize with Error info CPUUnpinningInvalid on source node " [Medium,Confirmed] https://launchpad.net/bugs/1879878 - Assigned to Stephen Finucane (stephenfinucane) | |
| 17:51:10 | openstackgerrit | Stephen Finucane proposed openstack/nova master: tests: Add reproducer for bug #1879878 https://review.opendev.org/744950 | |
| 17:51:38 | stephenfin | melwitt: I think that's the fix. Could you sanity check at some point? | |
| 17:51:52 | melwitt | stephenfin: yeah I'll take a look | |
| 17:52:43 | stephenfin | dansmith: You should probably look too ^ It's a variant of a patch I have for vTPM and as I think I've mentioned before, your name is all over the code I'm touching | |
| 18:36:08 | dansmith | okay I'm kinda heads-down on something else right now, but melwitt I guess let me know if you're unsure | |
| 18:36:47 | melwitt | I haven't looked yet but I think I can already say I'm unsure :) | |
| 19:43:59 | lyarwood | stephenfin: https://review.opendev.org/#/c/699291/10 - finally got back to this btw if you can take a look | |
| 20:11:53 | openstackgerrit | Artom Lifshitz proposed openstack/nova master: Handle Neutron errors in _post_live_migration() https://review.opendev.org/729763 | |
| 20:29:28 | openstackgerrit | Artom Lifshitz proposed openstack/nova master: WIP: Centralize wait_for_unversioned_notification https://review.opendev.org/744985 | |
| 20:33:37 | openstackgerrit | Lee Yarwood proposed openstack/nova master: WIP zuul: nova-evacuate https://review.opendev.org/744883 | |
| 20:41:46 | openstackgerrit | Lee Yarwood proposed openstack/nova master: WIP zuul: nova-evacuate https://review.opendev.org/744883 | |
| 20:58:26 | lyarwood | melwitt: https://review.opendev.org/#/c/743319/ - would you mind +W'ing that again as CI is finally green again | |
| 20:59:38 | melwitt | lyarwood: yeah, I will | |
| 21:01:53 | lyarwood | thanks :) | |
| 21:03:08 | melwitt | np | |
| 21:08:12 | openstackgerrit | Lee Yarwood proposed openstack/nova master: WIP zuul: nova-evacuate https://review.opendev.org/744883 | |
| 21:17:34 | openstackgerrit | Sean McGinnis proposed openstack/nova master: Add lsscsi to bindep https://review.opendev.org/744992 | |
| 21:19:06 | smcginnis | Needed for an os-brick change in the latest release ^ | |
| 21:24:58 | sean-k-mooney | smcginnis: should that not be listed in os-bricks bindep | |
| 21:25:02 | sean-k-mooney | not novas | |
| 21:25:47 | sean-k-mooney | smcginnis: nova's unit tests should be mocking any calls to os-brick | |
| 21:27:37 | sean-k-mooney | smcginnis: os-brick is a deliverable of cinder not nova so if it was to be added to any project for devstack would it not be better to add it to cinders bindep | |
| 21:28:30 | smcginnis | smcginnis: It is in os-brick's bindep, but it turns out when we install libs from their released version, that doesn't do us any good. | |
| 21:28:43 | smcginnis | sean-k-mooney: Hah, oops. Talking to myself. :) | |
| 21:29:06 | sean-k-mooney | it happen i am told its only a problem if you are surpised by the answer | |
| 21:29:14 | smcginnis | sean-k-mooney: I was able to get a change in devstack so it will use bindep when installing from source (it didn't before) but still nothing to address this case. | |
| 21:29:20 | smcginnis | ;) | |
| 21:29:46 | sean-k-mooney | well im wondier if you shoudl be adding this do cinder | |
| 21:30:01 | openstackgerrit | Merged openstack/nova stable/train: Silence amqp heartbeat warning https://review.opendev.org/728057 | |
| 21:30:09 | sean-k-mooney | i mean at a minium it probaly should be lsscsi [cinder] | |
| 21:30:15 | smcginnis | That one is in https://review.opendev.org/#/c/743291/ | |
| 21:30:28 | smcginnis | Ah, didn't see there was a profile for that. | |
| 21:30:39 | sean-k-mooney | well there proably isnt | |
| 21:30:44 | sean-k-mooney | im suggesing adding one | |
| 21:31:05 | sean-k-mooney | its not a dep of nova when not using cinder right | |
| 21:31:13 | smcginnis | So then zuul playbooks would also need to be updated to use that profile. | |
| 21:31:16 | sean-k-mooney | just if you are using os-brick ? | |
| 21:31:28 | smcginnis | Yeah, so probably only applicable to compute nodes. | |
| 21:31:58 | sean-k-mooney | ya only compute nodes and if cinder is deployed | |
| 21:32:26 | sean-k-mooney | what you propose is probaly ok but just said i would ask since its not really a dep of nova | |
| 21:32:48 | smcginnis | Yeah, makes sense. | |
| 21:33:02 | smcginnis | I don't have time now, but I may be able to follow up later to make it better. | |
| 21:33:04 | sean-k-mooney | we are not really strick about listing the min dpes in bindeps | |
| 21:33:17 | smcginnis | It is a small package, so at least it's not pulling down the world for this. | |
| 21:33:24 | sean-k-mooney | ya | |
| 21:33:33 | sean-k-mooney | i would kind of prefer if we use profiles more | |
| 21:33:50 | sean-k-mooney | e.g. add a mysql and postgress profile | |
| 21:33:51 | smcginnis | That could speed things up overall if we did. | |
| 21:34:01 | smcginnis | I wonder how many places we would need to update playbooks now though. :/ | |
| 21:34:29 | sean-k-mooney | we would need a way to pass info from the job in a declaritive way | |
| 21:35:17 | sean-k-mooney | if we used it more optimally where we have a profile for each of the configurable backend a project used and then listed the profiles that correstpond to the deployment we ar testing that would be nice but also a lot of work :) | |
| 21:36:11 | smcginnis | Yeah. Good idea though. | |
| 21:42:50 | openstackgerrit | Merged openstack/nova master: Removed the host FQDN from the exception message https://review.opendev.org/743950 | |
| 22:53:54 | openstackgerrit | Merged openstack/nova master: compute: Don't delete the original attachment during pre LM rollback https://review.opendev.org/743319 | |
| 22:54:01 | openstackgerrit | Merged openstack/nova master: func: Add CinderFixture to _IntegratedTestBase https://review.opendev.org/743535 | |
| 23:56:39 | openstackgerrit | Lee Yarwood proposed openstack/nova stable/ussuri: compute: Don't delete the original attachment during pre LM rollback https://review.opendev.org/744162 | |
| #openstack-nova - 2020-08-06 | |||
| 00:04:14 | openstackgerrit | Lee Yarwood proposed openstack/nova master: WIP zuul: nova-evacuate https://review.opendev.org/744883 | |
| 03:20:27 | openstackgerrit | Tony Su proposed openstack/nova master: Provider Config File: Enable loading and merging of provider configs https://review.opendev.org/693460 | |
| 05:54:57 | brinzhang_ | alex_xu: hi, do you know why this patch didnot run zuul +2? https://review.opendev.org/#/c/744280/ | |
| 05:55:39 | brinzhang_ | is it due to the tag of Signed-off-by？ | |
| 07:18:29 | gibi | brinzhang0: I don't think that Signed-off stops zuul. | |
| 07:20:49 | brinzhang0 | gibi: thanks. I dont know which stoped zuul to go :( | |
| 07:21:10 | brinzhang0 | gibi: Do you have time to check? | |