| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-10-22 | |||
| 08:52:59 | alex_xu | when I travel to US, the app store is connecting back to china server, that is crazy slow... | |
| 08:54:12 | bauzas | anyway, a good opportunity for de-googling myself :) | |
| 09:05:18 | openstackgerrit | kangyufei proposed openstack/python-novaclient master: Switch to Ussuri jobs https://review.opendev.org/690013 | |
| 10:44:12 | SonPham | Hi. I want response data from nova (server.py) to nova-pythonclient after nova-pythonclient request to nova | |
| 10:44:20 | SonPham | How can i do it | |
| 10:44:46 | SonPham | Hi. I want response data from nova (server.py) to novaclient after novaclient request to nova | |
| 10:47:01 | sean-k-mooney | bauzas: hi can you review this quickly, i tried to get this in to the train RCs but i think if fell through the cracks so it would be nice to land and backport it quickly https://review.opendev.org/#/c/675776/7 | |
| 12:05:14 | efried | gibi: can I get a quick +A on https://review.opendev.org/#/c/688979/ please? | |
| 12:05:43 | gibi | efried: looking... | |
| 12:07:05 | efried | you were +2 before the reno | |
| 12:07:23 | efried | Oh, I guess the predecessor still needs reviews | |
| 12:08:15 | gibi | I'm +A on the trait applying patch | |
| 12:09:33 | efried | gibi: +2 on the ItemsMatcher fup. If you want to +2 the bottom one, maybe we can convince bauzas to +A both and all three of these will merge. | |
| 12:16:49 | gibi | efried: I can add some unit tests for that fup. But as you are happy with the fup I'm happy with your original patch. | |
| 12:18:55 | efried | gibi: like I said, I think we can live without test-for-test-util; when we use it in tests we'll uncover any bugs and fix them then. And it won't impact prod. | |
| 12:19:11 | efried | but if you're losing sleep over it, I'll happily review tests :) | |
| 12:22:28 | gibi | efried: sure, I got your comment. I would like to add test cases | |
| 12:23:07 | gibi | efried: if the patch merges before I can add test cases then I will add those in a separate patch | |
| 12:23:20 | efried | now we need to get that spy stuff merged | |
| 12:23:30 | gibi | efried: that will be harder I think | |
| 12:23:32 | efried | been at least three or four cases that could have made use of it. | |
| 12:23:51 | efried | since it was proposed | |
| 12:26:32 | gibi | efried: I think I saw two of those cases. It think not everybody sees the same value we thing the spy adds | |
| 12:27:21 | efried | gibi: I'm almost convinced it should be added to the mock library itself. If we did that, we could probably iron out those last pesky wrinkles that are annoying us... | |
| 12:40:37 | openstackgerrit | Alex Xu proposed openstack/nova master: Add test for the race between post live migration and update available resource https://review.opendev.org/690066 | |
| 12:41:16 | alex_xu | efried: ^ I feel the race is case due to the post live migration, not the pre live migration | |
| 12:41:39 | efried | alex_xu: ack, I was about to get around to trying to grok your comment :) | |
| 12:42:12 | efried | Did you see the repro though? That triggers the race by running the periodic before the dest claim. | |
| 12:42:53 | alex_xu | efried: yes, but I think that is due to the no numa instance without migration context, I feel it isn't race problem | |
| 12:43:32 | alex_xu | efried: we assign migration.dest and instance.migration_context under the same lock, so it shouldn't happen race | |
| 12:43:46 | efried | Only on cold migration | |
| 12:44:01 | efried | on live migration the former happens on the conductor, the latter on the compute. | |
| 12:44:02 | alex_xu | oh, I only see LM, didn't look at cold one | |
| 12:44:43 | alex_xu | but it should be same, since both cold migration and LM using same move claim | |
| 12:46:34 | alex_xu | efried: here is https://github.com/openstack/nova/blob/master/nova/compute/resource_tracker.py#L306 and https://github.com/openstack/nova/blob/master/nova/compute/resource_tracker.py#L370 | |
| 12:49:54 | efried | alex_xu: cold migration uses _claim_existing_migration but live migration uses _create_migration -- or the other way around, now I can't remember :( | |
| 12:50:32 | alex_xu | really...i will hate that | |
| 12:51:47 | alex_xu | efried: quick grep the code, both cold and live using _claim_existing_migration | |
| 12:51:59 | alex_xu | they create the migration obj in conductor | |
| 12:52:09 | efried | alex_xu: If you can, probably best to wait for mriedem -- he understands these paths way better than I do. | |
| 12:52:39 | alex_xu | efried: yea, no hurry, whatever the fix is right | |
| 12:52:47 | efried | I put together these patches to further my understanding, but he was leading the way | |
| 12:53:40 | efried | Like, mission accomplished, I understand things a little bit better than I did before, but it's still like |...me..................mriedem..| | |
| 12:55:22 | efried | btw, I ran into the same COMPUTE_RESOURCE_SEMAPHORE deadlock while I was experimenting yesterday, and considered working around it by manually calling _populate_assigned_resources -- but I didn't think that was a realistic test. | |
| 12:56:41 | alex_xu | yea...that deadlock spend hours for me, totally not worth... | |
| 12:59:33 | efried | originally I was using a cold migration and injecting update_available_resource before _prep_resize, but that didn't trigger the problem. So I tried to inject it lower down, after _claim_existing_migration, and ran into the deadlock. | |
| 13:00:05 | efried | That's when mriedem suggested I instead use a live migration and inject before pre_live_migration instead. | |
| 13:00:14 | efried | That reproduced the race | |
| 13:00:24 | efried | or... *a* race, anyway :) | |
| 13:13:12 | bauzas | sean-k-mooney: ah sorry, was at the gym, will review it ASAP | |
| 13:16:19 | openstackgerrit | Ghanshyam Mann proposed openstack/nova master: Fix policy doc for host_status and extended servers attribute https://review.opendev.org/689833 | |
| 13:16:56 | openstackgerrit | Ghanshyam Mann proposed openstack/nova master: Fix policy doc for host_status and extended servers attribute https://review.opendev.org/689833 | |
| 13:16:57 | mriedem | hear ye hear ye we need stable/stein reviews https://review.opendev.org/#/q/status:open+project:openstack/nova+branch:stable/stein | |
| 13:17:11 | bauzas | mriedem: ack | |
| 13:17:19 | bauzas | sean-k-mooney: urgent revert, I guess ? | |
| 13:17:28 | bauzas | sean-k-mooney: I can fast-approve it as per our policy | |
| 13:21:10 | sean-k-mooney | bauzas: its not a pure revert | |
| 13:21:35 | sean-k-mooney | e.g. we updated the comments to reflect our understanding of why we cant make the chagne | |
| 13:22:03 | sean-k-mooney | bauzas: i also want to see the meloxn ci run an pass on it | |
| 13:22:26 | bauzas | sean-k-mooney: ergh ok | |
| 13:22:42 | bauzas | I'd have preferred a classic revert so we could spin another change | |
| 13:22:57 | bauzas | anyway, it's done | |
| 13:23:20 | sean-k-mooney | well we could just do a strait revert | |
| 13:23:40 | sean-k-mooney | and then apply this after on master to fix the comments | |
| 13:24:16 | sean-k-mooney | without the patch we get a traceback in nova while a vm is running with a macvtap interface | |
| 13:24:30 | sean-k-mooney | it does not break the vm | |
| 13:24:50 | sean-k-mooney | one of the periodic task that track resouce usage is unhappy | |
| 13:27:14 | bauzas | cool | |
| 13:44:59 | mriedem | gmann: in a multinode job, was is the "peers" group for hosts? and why would that need to be configured over just the "subnode" group vars to get configuration into the subnode compute? | |
| 13:45:57 | gmann | mriedem: i think that was old name i remember, later we did 'peers' -> 'subnode' | |
| 13:48:25 | mriedem | ok i was wondering why we have both subnode and peers configured in some multinode jobs and it sounds like we could just move all the 'peers' config to 'subnode' vars config | |
| 13:49:06 | mriedem | e.g. https://github.com/openstack/nova/blob/master/.zuul.yaml#L340-L352 | |
| 13:50:23 | gmann | let me check if any other reason to keep that. | |
| 13:51:30 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Make nova-next multinode and drop tempest-slow-py3 https://review.opendev.org/683988 | |
| 13:57:19 | mriedem | efried: want to do the backports for https://review.opendev.org/#/c/689842/ and the func test patch below it? | |
| 13:59:28 | mriedem | sean-k-mooney: i'm assuming you'll be backporting this revert to train yeah? https://review.opendev.org/#/c/675776/ | |
| 14:00:02 | mriedem | not sure why that wasn't flagged for train-rc-potential | |
| 14:03:53 | mriedem | so we have 2 train regressions identified and fixed already, we should probably do a patch release soon | |
| 14:04:47 | mdbooth | This is weird. utils.synchronized(instance.uuid) seems to be ignored in my functional test | |
| 14:04:55 | sean-k-mooney | mriedem: yes i can do that if adrianc does not | |
| 14:05:24 | mdbooth | i.e. I'm hitting breakpoints in critical sections locked by the same key concurrently | |
| 14:05:39 | sean-k-mooney | mdbooth: we might be mocking the lock | |
| 14:05:51 | sean-k-mooney | or the file acess | |
| 14:06:07 | mdbooth | sean-k-mooney: Yeah, I was hoping somebody would shout if that was the case :) However, that's definitely where I'm heading next. | |
| 14:06:17 | mdbooth | But why would we do that? | |
| 14:06:22 | mriedem | https://github.com/openstack/nova/blob/master/nova/test.py#L239 | |
| 14:06:52 | sean-k-mooney | mdbooth: we make most things serial in the fucntional tests | |
| 14:06:54 | mdbooth | mriedem: Thanks! Why would you ever turn that off, btw? | |
| 14:07:07 | sean-k-mooney | so i guess locking is not always needed | |
| 14:07:10 | mdbooth | Except if you didn't want it to work | |
| 14:07:19 | mriedem | see the note above the condition | |
| 14:08:15 | mdbooth | mriedem: Guessing that was written in the days before functional/ ? | |
| 14:08:17 | sean-k-mooney | oh i guess it would also cause issues because we use the same instance uuids | |
| 14:08:28 | sean-k-mooney | e.g. for parallel test executions on the same host | |
| 14:08:48 | mdbooth | Because non-test code uses locks, and we're not going to 'fix' that code to not use locks | |
| 14:08:55 | mdbooth | Just so we can execute it from a test | |
| 14:10:44 | mdbooth | mriedem: For reference, the locks which are being ignored are the locks in _do_build_and_run_instance, and terminate_instance in ComputeManager | |
| 14:11:03 | mriedem | mdbooth: it's not before functional tests existed | |
| 14:11:07 | mdbooth | My test assumes that the locks works as expected. It would be weird to expect otherwise. | |
| 14:12:02 | sean-k-mooney | mdbooth: most fucntional tests assume that there are not cuncuretn action taking place | |