Posted	Nick	Remark
#openstack-nova - 2018-07-25
08:33:18	gibi	cdent: plus when we assume that it is a new consumer we can still need to prepare that we get consumer generation conflict and handle that (most probably fail the build)
08:33:45	cdent	clearly there needs to be some conflict detection, but bar that the logic is mostly right unless there's a detail I'm not aware of (which is very likely)
08:34:22	cdent	yes, a generation conflict on a new build would mean that UUIDs are broken :)
08:34:49	gibi	cdent: or that our assumption in the claim_resources about the fact that it is a new build is invalid
08:36:11	gibi	cdent: even if we are coming from the scheduer select_destination it is possible that the consumer exists as evacuate does not use migration.uuid to hold the allocation of the source host
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,

Earlier   Later