| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-10-31 | |||
| 12:15:39 | jaypipes | efried: that leads *in* to that series. | |
| 12:16:11 | efried | jaypipes Sounds good to me. What do you have in mind? | |
| 12:16:33 | jaypipes | efried: I'm not a fan of the objectified approach in that series and would like to break that mega-method out into separate module-level functions that are appropriately named. | |
| 12:17:39 | efried | jaypipes As in, get rid of e.g. the ProviderSummary[List] and AggregatedProviderSummary[List] etc.? | |
| 12:17:49 | jaypipes | yes, exactly. | |
| 12:17:58 | jaypipes | I don't believe those objects are healthy. | |
| 12:18:06 | jaypipes | or necessary. | |
| 12:18:09 | efried | jaypipes Okay, so essentially supplanting the series with a different approach. | |
| 12:18:52 | jaypipes | efried: meh, in a way. more just laying a different foundation for the series to build from... | |
| 12:19:17 | efried | I'm less enthused about that, but not opposed to it. We need to be able to move forward here, and it's clear that the current monolith is untenable. But it's also clear we're having trouble constructing SQL that handles all the cases. | |
| 12:20:05 | efried | So on the one hand, having some jaypipes join-fu help would be great; but on the other hand, having something only jaypipes can understand... less than ideal. | |
| 12:20:15 | jaypipes | efried: and I've said in reviews on this series (and others before) that we don't need to try and do all the pieces in one giant SQL statement. | |
| 12:20:22 | efried | ++ | |
| 12:20:57 | efried | I'm not well versed enough to understand where we introduce huge inefficiencies by moving join-y stuff from the database to python. | |
| 12:21:05 | efried | But in principle, I love that idea. | |
| 12:21:06 | jaypipes | cdent: for the life of me, I cannot understand why I'm getting the failure (only on the gabbit in py35) in this particular patch: https://review.openstack.org/#/c/385693/. If you might take a looksie, that would be most appreciated... | |
| 12:21:22 | cdent | jaypipes: shore. grass still too wet to cut | |
| 12:22:38 | efried | jaypipes So like, would it be appropriate to have one method that pulls just inventory/allocation info for a single RP+RC? Or is that too small? | |
| 12:23:11 | jaypipes | efried: it's not that complicated actually. the basic idea is don't complicate the SQL unless there's a reason to do so. All lengthy and complex SQL statements, as you know, can be broken out into composite sub-statements and executed separately. Sometimes, actually, doing multiple queries that each are very efficient can be faster than trying to do a single SQL complex statement. | |
| 12:23:42 | jaypipes | efried: for the use_single_provider request groups, that is EXACTLY the right way to do things. | |
| 12:24:14 | efried | jaypipes Yeah, I've been thinking the SQL for the use_single_provider groups would be way simpler than what's happening now. | |
| 12:25:25 | efried | jaypipes Okay, so I'm behind the idea, but alex_xu and I ought to know what we should be doing with the existing series while you work that up. Probably best to quit messing with it, cause it'll either change drastically or be abandoned altogether? | |
| 12:25:39 | jaypipes | efried: think in terms of sets of records. there are multiple sets of records we want to retrieve based on the problem we're attempting to solve. in the case of shared providers, that's one set of records. the set of providers that provider ALL parts of a request of resources, that's another set. The set of providers that provides *some* but not all resources, that's yet another set of records. the trick is make efficient SQL statements to | |
| 12:25:39 | jaypipes | produce each of those sets in a way that can be reasoned about easily afterwards. | |
| 12:26:56 | efried | jaypipes And presumably to make sure the amount of data we're pulling from the database and then paring down from python isn't way too big. | |
| 12:27:04 | jaypipes | efried: right. | |
| 12:27:51 | priya | gibi:Could you please review https://review.openstack.org/#/c/508533/ | |
| 12:28:10 | efried | jaypipes Dunno if you remember in Austin the example I called out: they had a shell script that did "SELECT * FROM really_huge_database" | wc -l | |
| 12:28:27 | jaypipes | efried: who is "they"? | |
| 12:29:08 | jaypipes | priya: I *think* gibi may be on public holiday today... not entirely sure though. | |
| 12:29:10 | efried | jaypipes I don't remember. Could have been a cinder thing? To count up soft-deleted entries or something? And it wasn't production code, just a testing util. But it still grated. | |
| 12:29:22 | jaypipes | yeah... | |
| 12:30:22 | cdent | jaypipes: the gabbi problem is a combination of case sensitivity and dict ordering unpredictability. you didn’t introduce it, it just didn’t fail before in commit a9105b4904b that added the failing test. I’ll note the fix on the review, which we might want to do separately to yours and merge nowish | |
| 12:30:59 | jaypipes | cdent: hmm, intersting. is this a py35 vs py27 thing? or just a freak occaaion? | |
| 12:31:04 | jaypipes | occasion.. | |
| 12:31:19 | cdent | I suspect that py35’s dict ordering is a factor | |
| 12:31:32 | efried | jaypipes So let me know if I can help out here. Sounds to me like the intent is to write a bunch of separate independent functions and eventually refactor _get_by_filters to use 'em. So in that first stage, if you want to dole out "write me a function that does X" we can work in parallel. | |
| 12:31:35 | gibi | jaypipes, priya: I will be on public holiday tomorrow :) so I will look at it soon | |
| 12:31:41 | cdent | but there’s also freakery involved otherwise it would have failed on efried’s patch | |
| 12:31:56 | jaypipes | efried: zactly. | |
| 12:32:27 | jaypipes | efried: just like the current code currently first calls out to a separate function to get "root providers" and then gets shared providers, etc... | |
| 12:32:41 | jaypipes | efried: just needs to be cleaned up a bit. | |
| 12:32:45 | jaypipes | efried: and better doc'd. | |
| 12:40:16 | openstackgerrit | David Rabel proposed openstack/nova master: Fix format in live-migration-usage.rst https://review.openstack.org/516416 | |
| 12:45:35 | cdent | jaypipes: i’ll go ahead and fix that thing separately https://bugs.launchpad.net/nova/+bug/1728934 | |
| 12:45:37 | openstack | Launchpad bug 1728934 in OpenStack Compute (nova) "placement gabbi tests that manipulate microversion can intermittently fail" [Medium,Triaged] - Assigned to Chris Dent (cdent) | |
| 12:57:25 | openstackgerrit | Chris Dent proposed openstack/nova master: [placement] avoid case issues microversions in gabbits https://review.openstack.org/516662 | |
| 12:57:34 | efried | jaypipes Was about to respin the flavor-side granular series (https://review.openstack.org/#/c/515151/) - you got more to say there at the moment? | |
| 12:57:53 | cdent | jaypipes, efried, gibi ^^ gate flakiness bug fix | |
| 12:58:11 | efried | cdent Would that be officially remedied by making gabbi use case insensitive dicts (like requests does) for headers? | |
| 12:58:25 | cdent | efried: read the commit message for the answer to that | |
| 12:59:35 | cdent | efried: the idea is that gabbi needs to support the behaviors of http, not whatever clients happen to be available | |
| 12:59:40 | gibi | cdent: looking... | |
| 12:59:55 | efried | cdent I thought the http standard was for headers to be case-insensitive. But I'm no expert. | |
| 13:00:07 | efried | cdent Did you deliberately say "vagrancies" rather than "vagaries"? | |
| 13:00:37 | cdent | and because http header names are case insensitive it is possible to accidentally set the same header twice in some implementations | |
| 13:00:43 | cdent | no, but i’ll take it :) | |
| 13:01:22 | efried | I think even with case insensitivity you can set the same header multiple times. | |
| 13:01:45 | cdent | yes, that’s the “implementation” part of the commit message. | |
| 13:01:55 | cdent | it’s a yaml dict | |
| 13:02:29 | cdent | so _if_ we want to send a header twice, we have to use different case | |
| 13:03:01 | cdent | sloppy, but… | |
| 13:03:13 | efried | cdent Because yaml, or because gabbi? | |
| 13:03:43 | cdent | because of gabbi’s choice to use a dict for request headers | |
| 13:04:36 | cdent | efried: there’s a gabbi bug about this from a few months ago: https://github.com/cdent/gabbi/issues/213 | |
| 13:05:16 | cdent | efried: note that in that case the bug is talking about all the dicts in defaults, not just request headers | |
| 13:06:08 | cdent | the resulting conclusion was “don’t normalize as then what you see is not what you get” | |
| 13:07:03 | gibi | cdent: is there a reason choosing the non capitalized form of the header name? It seems that placement returns a capitalized from in the response here https://github.com/openstack/nova/blob/master/nova/tests/functional/api/openstack/placement/gabbits/microversion.yaml#L16 | |
| 13:08:23 | cdent | gibi: a) because nearly everything else you type into left hand side of gabbi is lowercase, b) uppercase is a pain to type. Yes, the response header is cased, and I noted that while making the changes and decided to still go with lower case. | |
| 13:09:21 | gibi | cdent: if it is intentional then I'm cool with it :) | |
| 13:10:06 | cdent | yeah, it’s intentional. I hate the shift key :) | |
| 13:14:38 | alex_xu | jaypipes: efried, hey I'm here | |
| 13:15:45 | efried | alex_xu Check out the backscroll between me & jaypipes and speak up if you have objections/questions/comments on what jaypipes is planning to do wrt refactoring stuff for GET /allocation_candidates | |
| 13:17:31 | alex_xu | efried: yea, I read those, the current series can be rewrite as a set of sub-method instead of the objects also. but yea, I'm not sure what is the plan of jaypipes | |
| 13:19:32 | alex_xu | jaypipes: efried, i'm not sure what I can do now, sounds like just abandoned the patch, or jaypipes do you want to share your idea on etherpad or anywhere. Then I and efried can help on that, since we already work on that series, and found few cases which easy to miss, that may help you? | |
| 13:25:21 | jaypipes | alex_xu: no, please don't abandon. lemme push some sample code and then let me know what you think about using it as the basis for your series. | |
| 13:26:09 | alex_xu | jaypipes: ok, got it, if you get the code up very soon, reading the code is more easy something | |
| 13:26:17 | jaypipes | efried: no, feel free to respin. I think you got most of my feedback on IRC on that one, yeah? | |
| 13:26:22 | alex_xu | s/something/sometime/ | |
| 13:26:26 | jaypipes | alex_xu: working it right now :) | |
| 13:26:39 | efried | jaypipes Hopefully. Respinning now... | |
| 13:28:51 | jaypipes | alex_xu: nighty night! :)( | |
| 13:29:03 | jaypipes | cdent: +W'd | |
| 13:39:33 | efried | jaypipes You said the order of subclauses in an AND isn't important... so the database doesn't short-circuit the clause on the first false condition? | |
| 13:40:27 | jaypipes | efried: no, it doesn't work that way. RDBMs will always prioritize the field(s) that are in indexes chosen by the query optimizer. | |
| 13:40:47 | efried | hm, neat. | |
| 13:41:23 | jaypipes | efried: so in that case, it makes no difference since the resource class and resource provider fields will always be evaluated before anything with total, max_unit, step_size, or min_unit | |
| 13:42:14 | efried | jaypipes But it's (slightly) cheaper to do the single-comparator math than the multiple-add-compare stuff. | |
| 13:42:38 | efried | multiply-add-compare, that is. | |
| 13:42:44 | efried | I'm sure it's negligible. | |
| 13:42:52 | jaypipes | efried: I don't think you'd see the difference there, no. | |
| 13:43:25 | jaypipes | efried: compared to the cost of loading all of the fields into memory after the join conditions are satisfied, the comparison of the data in those loaded fields is miniscule. | |
| 13:43:38 | efried | noted | |
| 13:44:28 | openstackgerrit | OpenStack Proposal Bot proposed openstack/os-vif master: Updated from global requirements https://review.openstack.org/511035 | |
| 13:48:56 | openstackgerrit | OpenStack Proposal Bot proposed openstack/os-vif master: Updated from global requirements https://review.openstack.org/511035 | |
| 13:59:11 | openstackgerrit | Eric Fried proposed openstack/nova master: Parse granular resources/traits from extra_specs https://review.openstack.org/515151 | |
| 13:59:11 | openstackgerrit | Eric Fried proposed openstack/nova master: Granularize resources_from_{flavor|request_spec} https://review.openstack.org/515223 | |
| 13:59:12 | openstackgerrit | Eric Fried proposed openstack/nova master: ResourceRequest.to_querystring() https://review.openstack.org/515811 | |
| 13:59:18 | efried | gibi jaypipes alex_xu ^ | |
| 13:59:26 | efried | and cdent too ^ | |