Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2017-12-22
16:17:35 mdbooth The most important aspect of it was to test that the compare-and-swap works. I can do that without the mocking.
16:17:58 artom mdbooth, think of it as teaching me :)
16:19:53 figleaf mriedem: you can't remove a zip tie without cutting it (and usually the hose)
16:20:01 artom mdbooth, a mock side_effect...
16:20:09 artom Maybe that's the part I'm not getting
16:20:17 artom Does it still call the original function?
16:20:31 artom And call the side_effect before/after/at the same time?
16:20:41 mdbooth It calls the side effect both times
16:20:45 mdbooth That's why flip is required
16:20:49 artom mdbooth, no, in general I mean
16:21:03 mdbooth flip makes it call the race first time only
16:21:16 mdbooth That prevents infinite recursion by the race functino
16:21:21 artom Like, if I mock foo() and mock.side_effect = bar, and I call foo(), does the real foo() still get called, or only bar()?
16:21:37 mdbooth No, only the side effect is called
16:21:40 mdbooth It's a bad name
16:21:42 mriedem figleaf: found a problem in our devstack setup for the alternate hosts stuff :)
16:21:50 artom mdbooth, So it's effectively replacing the mocked object
16:21:53 figleaf oh joy!
16:22:07 mdbooth It's replacing the mocked function in this case
16:22:19 mdbooth That's why we explicitly store a reference to the original
16:22:22 artom mdbooth, gotcha. Back to looking at the code
16:22:27 mdbooth So we can still call it.
16:22:36 mdbooth artom: I'm really going to simplify it.
16:22:46 mdbooth I no longer think it's worth it myself ;)
16:22:48 artom mdbooth, sure, but I still want to understand this
16:22:49 mriedem figleaf: pretty simple
16:22:50 mriedem http://logs.openstack.org/89/527289/1/check/ironic-tempest-dsvm-ipa-wholedisk-agent_ipmitool-tinyipa-multinode/570a3c9/logs/screen-n-cond-cell1.txt.gz#_Dec_21_23_49_28_813934
16:23:09 mriedem figleaf: the ironic job failed the first node and was rescheduling, but the cell conductor couldn't talk to placement b/c placement isn't configured in nova_cell1.conf in devstack
16:23:17 artom mdbooth, I shall wear your down with the stubbornness of my ignorance ;)
16:23:22 mriedem figleaf: working on a devstack patch
16:24:22 figleaf mriedem: yeah, that would hose things
16:27:56 artom mdbooth, maybe it'd be easier if you point out the error in http://paste.openstack.org/show/629605/ ?
16:29:28 mdbooth artom: The error is that the invocations of get_by_instance_uuid overlap
16:29:45 mdbooth The one top left goes all the way to top right
16:30:01 mdbooth There's another invocation of get_by_instance_uuid in the middle of it
16:30:15 mdbooth IOW, they are executing 'concurrently'
16:31:42 artom Heh, maybe discussing ASCII art pseudo sequence diagrams on IRC wasn't the best idea
16:31:56 artom A debugger then...
16:32:05 artom Confessions: I have never used a Python debugger
16:32:14 mdbooth So we've got a big call to get_by_instance_uuid()
16:32:47 mdbooth We stick a mock somewhere in the middle of it which interrupts the flow to call get_by_instance_uuid() again, before continuing with the original call, which still hasn't finished.
16:34:03 artom So far so good.
16:40:19 openstackgerrit Merged openstack/nova master: objects: Add PCI NUMA policy fields https://review.openstack.org/527470
16:42:31 mriedem figleaf: i think this should do the trick https://review.openstack.org/529857
16:47:04 artom mdbooth, is that all we're testing? That if one get_by_uuid starts up, but while it's running another get_by_uuid is called, they're get the same value in the end?
16:47:19 mdbooth artom: Yep
16:47:23 artom mdbooth, christ
16:47:30 artom mdbooth, ok no, please get rid of it :)
16:47:31 openstackgerrit Merged openstack/nova master: Make conductor pass and use host_lists https://review.openstack.org/511358
16:47:36 mriedem woot ^
16:48:04 mriedem #success nova merged alternate hosts support for server build
16:48:06 openstackstatus mriedem: Added success to Success page
16:48:11 artom mdbooth, I'd see the point if we let them both run to completion in different processes
16:48:27 artom mdbooth, but in the end, all your test does it call _create_uuid twice in successions
16:48:29 mdbooth artom: They do both run to completion!
16:48:35 mdbooth In effectively different processes.
16:48:52 artom ...
16:49:07 artom mdbooth, but when the second one is called via the mock, the first's flow is interreupted
16:49:11 artom Until the second one returns
16:49:12 artom No?
16:49:13 mdbooth By mocking _create_uuid specifically, it triggers an execution order which causes a race
16:49:30 figleaf mriedem: It's a xmas miracle!
16:49:38 openstackgerrit Merged openstack/nova master: doc: update supported drivers for cpu topology https://review.openstack.org/529294
16:49:54 openstackgerrit Matt Riedemann proposed openstack/python-novaclient master: Add support for the 2.57 microversion https://review.openstack.org/528128
16:50:14 mdbooth artom: Yes. That's an execution order which would cause incorrect behaviour if we didn't handle it.
16:50:34 openstackgerrit Matthew Booth proposed openstack/nova master: Make BlockDeviceMapping object support uuid https://review.openstack.org/242603
16:50:35 openstackgerrit Matthew Booth proposed openstack/nova master: Add an online migration for BDM.uuid https://review.openstack.org/525599
16:50:35 openstackgerrit Matthew Booth proposed openstack/nova master: DriverBlockDevice: make subclasses inherit _proxy_as_attr https://review.openstack.org/524167
16:50:36 openstackgerrit Matthew Booth proposed openstack/nova master: Expose BDM uuid to drivers https://review.openstack.org/529037
16:50:42 mdbooth artom: However, it's gone ^^^ ;)
16:50:51 artom mdbooth, dammit
16:51:03 artom mdbooth, I still want to understand you
16:51:17 mdbooth artom: It's a good test, but a totally agree that it's more complicated that it needs to be
16:52:00 mdbooth Anyway, I'm going to run for the hills!
16:52:13 mdbooth Merry Christmas, all! See you in the New Year.
16:55:06 openstackgerrit Sylvain Bauza proposed openstack/nova master: libvirt: create vGPU for instance https://review.openstack.org/528832
16:57:49 artom mdbooth, enjoy your ugly sweaters ;)
17:12:48 mriedem figleaf: heh my bash fu always fails the first few times
17:14:25 figleaf mriedem: don't feel bad - I felt uneasy +1'ing that because my bash-fu is so weak
17:14:37 figleaf it looked logical, thougn
17:14:39 figleaf though
17:15:29 mriedem it does do the correct thing for nova_cell1.conf
17:15:30 mriedem so that's good
17:19:22 mriedem someone was mentioning issues with the NumInstances filter yesterday, and i got thinking about that one, if you ran multiple filter scheduler processes, that filter would be totally racey
17:19:38 mriedem depending on which process gets a request, or if both are processing requests at the same time for the same host
17:20:26 mriedem if the limit is 10 and you've got 9 on a host already, and both workers get a request at the same time, they'd both think there is room for one more and could choose it
17:20:32 mriedem placement doesn't fix that
17:20:37 mriedem only reschedules would fix that
17:21:30 mriedem seems like a totally useless filter anyway assuming you have any non-trivial number of flavors
17:21:48 kashyap mriedem: Hey, when you have a moment, you know why this is failing? (I know it's coming from my change) -- http://logs.openstack.org/11/529611/3/check/openstack-tox-py27/f611b46/job-output.txt.gz#_2017-12-21_17_29_32_011876
17:21:54 kashyap ( https://review.openstack.org/#/c/529611/ )
17:22:09 kashyap It's just doing something super simple; adding a config class & test that XML element
17:22:22 kashyap Ah, I see it
17:22:28 kashyap My test is trying to do it as:
17:22:29 kashyap 2017-12-21 17:29:32.012197 | ubuntu-xenial | <smm>
17:22:29 kashyap 2017-12-21 17:29:32.012215 | ubuntu-xenial | <smm state="on"/>
17:22:29 kashyap 2017-12-21 17:29:32.012231 | ubuntu-xenial | </smm>
17:22:34 kashyap However, it should be:
17:23:26 kashyap ...
17:23:26 kashyap <features>

Earlier   Later