| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-12-22 | |||
| 13:49:45 | bauwser | mriedem: also, note I'll be off until Jan 8th | |
| 13:49:53 | finucannot | I'm hoping leakypipes will come around on the middle one. I'm almost certain I'm correct in what I'm saying | |
| 13:50:34 | bauwser | finucannot: I did, and I saw you answered to leakypipes | |
| 13:50:42 | bauwser | I need a bit more time to think about the concern | |
| 13:50:56 | leakypipes | finucannot: currently trying to diagnose https://bugs.launchpad.net/nova/+bug/1739517 | |
| 13:50:57 | openstack | Launchpad bug 1739517 in OpenStack Compute (nova) "TestNovaMigrationsMySQL randomly fails with "sqlalchemy.exc.ResourceClosedError: This result object does not return rows. It has been closed automatically."" [High,Confirmed] | |
| 13:51:08 | finucannot | Oh, that beauty | |
| 13:51:12 | figleaf | leakypipes: it would have been cheaper to fly me to FLA and install your dishwasher for you | |
| 13:51:23 | leakypipes | figleaf: yeah, no shit.. | |
| 13:52:33 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Document testing guide for new API contributions https://review.openstack.org/529618 | |
| 13:55:53 | bauwser | mriedem: tell him some children in the world are still awake for already like 9 hours and that's his time | |
| 13:56:46 | mriedem | i just run in screaming that the house is on fire and we have to get out | |
| 13:58:54 | bauwser | looks like it's an universal NP-problem then | |
| 14:00:34 | openstackgerrit | Ryota MIBU proposed openstack/nova master: vif: look up 'ovs_interfaceid' in vif_details https://review.openstack.org/529831 | |
| 14:02:13 | bauwser | finucannot: leakypipes: remember me, what happens if we want to add a new item for an o.vo Enum field type, do we need to bump the object version using that field type ? | |
| 14:02:24 | bauwser | finucannot: leakypipes: context being https://review.openstack.org/#/c/527470/4/nova/objects/instance_pci_requests.py | |
| 14:02:59 | finucannot | bauwser: You mean if we change 'numa_policy' at a future date? | |
| 14:03:16 | bauwser | if we change the Enum field type, yes | |
| 14:03:24 | bauwser | not the field itself | |
| 14:03:37 | bauwser | like, removing the 'legacy' choice from the enum | |
| 14:03:54 | bauwser | (because I guess that's the direction we want) | |
| 14:12:08 | Roamer` | hm, no response in #openstack, so let me butt in here :) Does anybody know of a tool that reads devstacklog.txt, splits it into phases, and lets me know how long each phase took? Trying to figure out why my latest stack.sh runs take about twice as long as before | |
| 14:12:44 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Use volume shared_targets to lock during attach/detach https://review.openstack.org/529695 | |
| 14:12:58 | finucannot | bauwser: tbh, I've no idea. We haven't done that yet (that I know of), so it depends | |
| 14:13:00 | Roamer` | of course it would be trivial to write such a tool once I know what phases there are, but I haven't looked so deeply into stack.sh yet... has anybody else? | |
| 14:13:08 | mriedem | Roamer`: i'd ask in #openstack-qa | |
| 14:13:18 | Roamer` | mriedem, thanks, that might work | |
| 14:13:20 | mriedem | sdague is the person to ask but he's out for the year, so maybe mtreinish | |
| 14:13:27 | leakypipes | finucannot: we've done that with the ResourceClassField enum. | |
| 14:13:56 | bauwser | finucannot: I guess the problem is more about removing the legacy option | |
| 14:13:59 | leakypipes | finucannot: and I don't believe we've incremented the version for objects that use it... | |
| 14:14:07 | bauwser | say we could just deprecate that option, that's it | |
| 14:14:11 | leakypipes | finucannot: then again, we don't use objects over RPC for placement.. | |
| 14:14:46 | bauwser | leakypipes: well, the ResourceClassFieldType is specific because indeed we don't pass it thru RPC | |
| 14:15:27 | bauwser | we just use o.vo to have a coercing | |
| 14:17:44 | bauwser | finucannot: leakypipes: FWIW, I just +W'd that change because I think it's not a problem | |
| 14:21:00 | cdent | leakypipes: have you seen this and the associated bug: https://review.openstack.org/#/c/529397/ | |
| 14:21:12 | cdent | there was a lot of associated irc discussion about other options, but that was the simplest | |
| 14:21:28 | cdent | wanted to be sure you saw it before I leave to do some dreaded shopping | |
| 14:25:34 | leakypipes | finucannot: it's not that I don't like your code. I'm just having a hard time with the whole cascading policy loop thing... I agree with you that pools != filtered_pools and that my original complaint about coupling is not valid. however it still feels a bit clever for my taste. I'll remove my -1 and leave it to bauzas and gibi to +W | |
| 14:33:00 | leakypipes | mriedem: morning. | |
| 14:39:08 | openstackgerrit | Matthew Booth proposed openstack/nova master: Add uuid column to BlockDeviceMapping https://review.openstack.org/242602 | |
| 14:39:09 | openstackgerrit | Matthew Booth proposed openstack/nova master: Add an online migration for BDM.uuid https://review.openstack.org/525599 | |
| 14:39:09 | openstackgerrit | Matthew Booth proposed openstack/nova master: Make BlockDeviceMapping object support uuid https://review.openstack.org/242603 | |
| 14:39:10 | openstackgerrit | Matthew Booth proposed openstack/nova master: Expose BDM uuid to drivers https://review.openstack.org/529037 | |
| 14:39:10 | openstackgerrit | Matthew Booth proposed openstack/nova master: DriverBlockDevice: make subclasses inherit _proxy_as_attr https://review.openstack.org/524167 | |
| 14:42:54 | mdbooth | mriedem: Updated ^^^ I haven't completely re-run all tests locally, though. Just the obvious ones. Been bitten by that a couple of times recently, but I'm being optimistic anyway. | |
| 15:01:16 | cfriesen | what do we expect to happen if you do a resize/migrate, and after completion but before confirmation the dest node dies and we do an evacuate? Should the resize/migrate revert, should the evacuation pass without error, or something else? | |
| 15:23:12 | finucannot | leakypipes: Let me take a second shot at it. If it's too clever for you, it's going to be too clever for others too | |
| 15:23:31 | leakypipes | finucannot: maybe. I'm pretty dumb, though. | |
| 15:23:56 | finucannot | Ah, so am I. All the more reason | |
| 15:23:57 | finucannot | leakypipes: I assume you're gone next week? | |
| 15:24:28 | leakypipes | finucannot: nope, I'm working all next week. I got no vacation this year when i joined Verizon... | |
| 15:24:52 | finucannot | Ouch :( | |
| 15:25:16 | finucannot | four days is _more_ than enough for all involved | |
| 15:25:57 | leakypipes | heh | |
| 15:26:23 | finucannot | I had a look at that bug you mentioned too. Totally out of my area of expertise, I'm afraid, so best of luck with that, heh | |
| 15:27:03 | leakypipes | finucannot: yeah, it's a weird one alright... | |
| 15:28:26 | finucannot | and intermittent too. The worst kind of bug :( | |
| 15:38:34 | mriedem | cfriesen: you can't evacuate an instance in VERIFY_RESIZE state can you? | |
| 15:38:43 | mriedem | leakypipes: hi | |
| 15:38:51 | mriedem | leakypipes: did your xmas dinner party turn out ok? | |
| 15:39:10 | mriedem | cfriesen: @check_instance_state(vm_state=[vm_states.ACTIVE, vm_states.STOPPED, | |
| 15:39:11 | mriedem | def evacuate(self, context, instance, host, on_shared_storage, | |
| 15:39:11 | mriedem | vm_states.ERROR]) | |
| 15:39:46 | mriedem | cfriesen: you'd likely want to revert the resize to get the instance back to the source host | |
| 15:41:47 | mriedem | mdbooth: +2 on https://review.openstack.org/#/c/242602/ | |
| 15:41:51 | mriedem | despite that terrible blank space | |
| 15:42:04 | mdbooth | mriedem: Thank you, Sir! | |
| 15:43:16 | mdbooth | Hah! I do that when I want to comment branches separately. Hadn't really thought about it too hard, tbh. | |
| 15:43:45 | mdbooth | I guess the comments can also live inside the block | |
| 15:46:10 | mdbooth | artom: So, that concurrency test | |
| 15:46:33 | mriedem | efried_cya_jan: how does one recheck the powervm CI? it's not on the wiki https://wiki.openstack.org/wiki/ThirdPartySystems/IBM_PowerVM_CI | |
| 15:46:34 | artom | mdbooth, right | |
| 15:47:21 | mriedem | "recheck powervm" i guess | |
| 15:47:45 | mdbooth | It sounded like I hadn't convinced you, yet. I could be persuaded to simplify it, but I'm still thinking about that. | |
| 15:47:46 | artom | mdbooth, so, I grok the Python threads stuff - they don't run concurrently, one will run when another yields to do IO or whatever | |
| 15:48:03 | artom | mdbooth, it's not so much convincing, as explaining :) | |
| 15:48:32 | artom | mdbooth, what's not clear to me, and it may be ignorance of Python's internals on my part, is how your code makes sure there are two threads | |
| 15:48:37 | mdbooth | artom: Well your point is around whether the complexity is worth it, right? | |
| 15:48:56 | mdbooth | I'm confident it's a valid test. I could be persuaded that a simpler test might be better, though. | |
| 15:49:08 | mdbooth | There aren't 2 threads. | |
| 15:49:22 | mdbooth | There's only 1 thread, but it acts like 2 threads. | |
| 15:49:45 | mdbooth | We use mock to intercept a function call in the 'main' thread. | |
| 15:50:05 | mdbooth | When the main thread executes that function call, we interrupt it and do something else first, before continuing. | |
| 15:50:23 | mdbooth | Does that make more sense? | |
| 15:50:55 | mriedem | seems pretty paranoid for something that we've already established a pattern of in several other objects | |
| 15:51:06 | mriedem | or are you actually seeing this race happening with something like the cellsv1 job? | |
| 15:51:40 | artom | mdbooth, let me look at the code again | |
| 15:51:53 | artom | I still can't wrap my head around how purely sequential execution can test a race | |
| 15:51:58 | mdbooth | mriedem: It's more that I can see the bug and I fixed it, and it's not *that* complicated. | |
| 15:52:25 | mdbooth | mriedem: artom is trying to get his head round the test, and I've never yet managed to write a unit test for a race which was easy to read. | |
| 15:52:31 | artom | If it's just about calling _create_uuid twice, surely can do that without the whole flip/race thing | |
| 15:53:28 | mdbooth | artom: That's where I could be persuaded. Except that my test is 1 step up from that. | |
| 15:54:00 | mdbooth | My test asserts that if the race happens whilst reading the bdm object, it will work fine. | |
| 15:54:20 | mdbooth | Your test would be much simpler, and we could possibly agree it's sufficient. | |
| 15:54:24 | artom | But... the race can only happen when writing | |
| 15:54:55 | artom | I guess if you go up one stop from that, it two reads happen at the same time on the same uuid-less BDM, both will attempt to write a UUID | |
| 15:54:59 | artom | *if two | |
| 15:55:02 | mdbooth | No, this is weird. If we *read* a bdm object with no uuid we create one before returning it. | |