Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2018-07-25
08:36:47 openstackgerrit Lee Yarwood proposed openstack/nova master: libvirt: Do not reraise DiskNotFound exceptions during resize https://review.openstack.org/571410
08:37:17 gibi cdent: I agree that assuming consumer_generation = None in claim_resource a good first step
08:38:13 gibi cdent: I just fustrated about the hairy cases that I have to takle in my patch and I don't think we can merge support for 1.29 before solving 1.28 for the edge cases too
08:38:41 lyarwood mdbooth: ^ would you mind taking a look at that again?
08:38:48 cdent I'm not disputing that we need to merge (and understand) your 1.28 code first
08:39:18 cdent just, as I said, trying to make sure that we don't overcomplicate things when we claim, if possible
08:39:28 mdbooth lyarwood: yup
08:39:51 cdent if new: gen is none, if gen conflict bail, something went badly wrong
08:40:44 gibi cdent: when you say 'if new' do you mean checking if there is no other allocation in placement for the consumer?
08:41:02 cdent if existing allocations for this consumer uuid, do something else
08:41:27 cdent that would be one way, but I assume the caller of claim also knows?
08:42:32 cdent the "something else" looks to me to be very close to what is already being done in the code: merge the allocations, set the generation to what the server told us
08:42:36 gibi cdent: what if the caller thinks that it is not a new consumer but the claim_resources checks in placement and sees it is a new consumer and silently uses consumer_generation None instead of raise?
08:43:36 gibi cdent: I think in this case the caller needs to provider his view of the consumer generation and not let claim_resource decide
08:43:36 cdent then we should fix the caller because it sounds like the caller is broken? I'm not sure why we should overcomplicate the report client to deal with the fact that other code is flakey. What situation would cause what you're describing?
08:45:16 gibi cdent: sure it needs fix on the caller side but to know that the caller is broken we need to see the conflict and not simply use the consumer gen that will succeed in placement by asking it from placement
08:45:47 cdent I don't mean a fix at runtime, I mean a fix in the code
08:46:04 cdent Thus my question of "what situation.."
08:47:04 gibi cdent: I don't know exactly the case. I guess some parallel consumer update can happen. This is why we introduced consumer generation in the first place, isn't it?
08:47:10 cdent If we take this back to very basics there are two main situations: the consumer has existing allocations and this consumer does not have existing allocations. Are there others?
08:47:46 gibi there is the case when caller thing consumer gen is 1 but in the meantime somebody else modified the consumer
08:47:54 gibi s/thing/thinks/
08:48:31 cdent for "gen is 1 but somebody else modified" that is a specifc case of "the consumer has existing allocations"
08:48:57 gibi cdent: OK, then that is the two basic case
08:49:55 cdent From the point of the scheduler's management of allocations, consumer generations are not very important (we have to manage conflict detection, but we still just want to claim) because the act of claiming for an instance is a single task. We want to claim if at all possible. Nobody else is trying to find a destination for this instance.
08:50:36 gibi of course we can say that consumers handled by nova is owned by nova and no parallel update is done from nova on those and then we don't need to take consumers generations too seriously
08:50:58 cdent I'm saying that's true for in the scheduler. It's not true in the compute node, or in neutron agents, or whatever
08:51:11 cdent And we should keep it true in the scheudler if at all possible
08:52:05 gibi but scheduler coexists with all the others so I don't know how to ensure that the consumer the scheduler uses are not modified by other sources
08:52:40 gibi if scheduler would only ever create a new consumer during claim then it would be easier
08:52:51 cdent indeed
08:53:01 gibi due to evacuate it is not the case today
08:53:23 cdent but in the (presumably rare) cases where it doesn't the only real option we have is to get the existing allocations and do some kind of merge or choose to clobber
08:53:24 gibi there is an item in the PTG etherpad to 'fix' evacuate
08:54:01 cdent _move_operation_alloc_request is the code that is supposed to handle that sort of thing, but may not be mature enough of for nested and shared
08:55:45 cdent I think (or perhaps just hope) that we have a chance here to keep the happy path relatively smooth. I hope for that because we keep talking about how we want the claim stage to be as fast and unencumbered as possible.
08:55:46 gibi cdent: I agree that in case of non-forced evac when the scheduler claims on the target host we cannot do anything else just read the generation from placement. I will try to be really explicit in my patch about the different cases.
08:56:59 cdent tetsuro: you have anything to add to all that ^
08:57:42 gibi cdent: I share your hope. My bad feeling is about our assumption that I can identify the happy path unambigously
08:58:01 openstackgerrit Radoslav Gerganov proposed openstack/nova master: doc: add missing permission for the vCenter service account https://review.openstack.org/585683
08:58:40 cdent that we can't should be considered a bug ;)
09:00:15 gibi cdent: I guess I need to lower my expectations about my ability to cover every cases at once in my patch and let a bug born if it needs to be born
09:00:56 cdent yeah, I think that's going to be the case for a lot of this stuff because it's so hard to see clearly how things work
09:01:11 cdent And bugs are _good_. They give people a clear way to contribute.
09:02:03 gibi cdent: I thikn _these_ possible bugs will give a clear message for new contributors not to touch nova ;)
09:02:41 cdent I don't know, I think we misunderstand the mechanics of why people choose or choose not to contribute
09:02:59 cdent So often with nova if you want to contribute you have to spend 4 years understanding things
09:03:09 cdent there are few easy entries
09:03:23 cdent plenty of people are using nova because they have to and would like to be able to make it better
09:03:58 cdent but are put off
09:04:00 gibi cdent: you are right, I should not generalize to all new contributors as I don't know their motives.
09:04:59 gibi cdent: what I wanted to say that I think these bugs will be scary even for me after 4 years of nova
09:05:10 cdent :)
09:05:37 cdent It's easier to fix them after they happen then it is easy to guess when they might happen?
09:07:33 gibi cdent: yeah I agree about cost of guessing is bigger. But I also don't know the seriosuness of the effect of these unknown bugs on a live deployment
09:09:18 gibi cdent: anyhow I think I hear you and understand you to move forward with the happy path
09:09:24 gibi cdent: and I think that is reasonable
09:40:44 openstackgerrit sean mooney proposed openstack/os-vif master: add fuctional test for os-vif init https://review.openstack.org/585693
09:41:20 openstackgerrit Rajesh Tailor proposed openstack/nova stable/queens: Make host_aggregate_map dictionary case-insensitive https://review.openstack.org/585334
09:43:41 openstackgerrit Rajesh Tailor proposed openstack/nova stable/pike: Make host_aggregate_map dictionary case-insensitive https://review.openstack.org/585335
09:45:55 sean-k-mooney mriedem: melwitt i have added a fuctionl test https://review.openstack.org/#/c/585693/ to try and reproduce the error in https://bit.ly/2v0ueZp but at least locally it looks like it should work as is.
09:46:00 openstackgerrit Rajesh Tailor proposed openstack/nova master: Fix case-sensitivity for metadata keys https://review.openstack.org/504885
10:30:25 openstackgerrit Lenny Verkhovsky proposed openstack/nova master: WIP workaround for RHEL7.5 qemu 2.0 https://review.openstack.org/577280
10:37:36 openstackgerrit Balazs Gibizer proposed openstack/nova master: Functional test for booting with nested resources https://review.openstack.org/527728
10:38:46 mdbooth kashyap: My +1 to sahid's suggestions was before the subsequent discussion. I'll defer to that discussion.
10:38:58 kashyap mdbooth: Hi
10:40:13 kashyap mdbooth: So, you mean: you are OK with moving to guest.migrate() can be done as a separate patch?
10:40:48 mdbooth kashyap: I think moving to guest.migrate() is better, but if you discussed that already and decided to put it in a later change I'm ok with that.
10:41:01 kashyap mdbooth: Yeah, we discussed it, and will do it later
10:41:11 kashyap As I am dealing with too much churn in this change
10:41:15 mdbooth kashyap: That is, I don't mind what colour you paint the bike shed :)
10:41:29 kashyap I'd like to get it done before I head out for 2 weeks starting 01-Aug
10:41:35 kashyap mdbooth: Hehe, nod
10:41:41 kashyap Thanks for looking, BTW!
10:42:24 kashyap mdbooth: About the "parallels" question: it seems alright to me, but I'll go over carefully again
10:43:00 mdbooth kashyap: Ack. Specifically, is it ok to call migrateToURI3() without that parameter?
10:43:11 mdbooth I don't know the answer, btw, just worth checking.
10:43:24 kashyap mdbooth: Without 'destination_xml', you mean?
10:43:32 kashyap Yeah, I'll see
10:45:14 kashyap mdbooth: Right now debugging the test failure from hell: nova.tests.unit.virt.test_virt_drivers.LibvirtConnTestCase.test_live_migration
10:45:39 kashyap Once I'm done w/ it, will look
10:50:23 kashyap stephenfin: LOL
10:50:41 kashyap I'm dealing with a bloody obtuse stubbing failure for 2 hours!
10:50:55 stephenfin kashyap: Try 3 days :(
10:50:59 kashyap The here is incredibly dense; sheesh
10:51:05 kashyap stephenfin: I lied to you
10:51:14 kashyap 2 hours only today
10:51:21 kashyap I spent yesterday & day before too
10:51:34 kashyap After all this debugging, realized the fix is "simple" :-(
10:51:59 mdbooth stephenfin: stub -> simulator. simulators are also really useful, just not as simple.
10:57:06 stephenfin mdbooth: I think that's what I'm writing. God damn neutron
11:01:14 kashyap mdbooth: Got a minute? Does this stub look OK to you:
11:01:15 kashyap 135 self.stub_out('nova.virt.libvirt.guest.Guest.migrate',
11:01:15 kashyap 136 lambda self, destination, migrate_uri=None,
11:01:15 kashyap 137 migrate_disks=None, destination_xml=None, flags=0,
11:01:18 kashyap 138 bandwidth: None)
11:01:24 kashyap (That's from nova/tests/unit/virt/test_virt_drivers.py)
11:02:45 mdbooth kashyap: No, because bandwidth doesn't have a default, and comes after args with defaults.
11:02:58 kashyap mdbooth: Ah
11:03:01 mdbooth Is there any other reason it might not be ok?

Earlier   Later