| *** takashin has joined #openstack-placement | 01:02 | |
| *** amodi has quit IRC | 04:22 | |
| *** dansmith has quit IRC | 05:04 | |
| *** dansmith has joined #openstack-placement | 05:22 | |
| *** artom has joined #openstack-placement | 06:16 | |
| *** artom is now known as artom|gmtplus3 | 06:19 | |
| *** belmoreira has joined #openstack-placement | 06:24 | |
| openstackgerrit | Merged openstack/placement master: Fix up some inaccuracies in perfload comments and logs https://review.opendev.org/668449 | 06:36 |
|---|---|---|
| *** cdent has joined #openstack-placement | 06:55 | |
| *** tssurya has joined #openstack-placement | 07:13 | |
| *** helenafm has joined #openstack-placement | 07:19 | |
| *** belmoreira has quit IRC | 07:37 | |
| *** evrardjp is now known as evrardjp_on_holi | 07:40 | |
| *** ttsiouts has joined #openstack-placement | 07:40 | |
| *** evrardjp_on_holi is now known as evrardjp_away | 07:40 | |
| *** e0ne has joined #openstack-placement | 07:49 | |
| *** ttsiouts has quit IRC | 07:52 | |
| *** ttsiouts has joined #openstack-placement | 07:53 | |
| *** belmoreira has joined #openstack-placement | 07:56 | |
| *** ttsiouts has quit IRC | 07:57 | |
| *** takashin has left #openstack-placement | 08:00 | |
| *** ttsiouts has joined #openstack-placement | 08:19 | |
| *** belmoreira has quit IRC | 08:52 | |
| *** tetsuro has joined #openstack-placement | 09:04 | |
| *** belmoreira has joined #openstack-placement | 09:20 | |
| *** tetsuro has quit IRC | 10:04 | |
| *** tetsuro has joined #openstack-placement | 10:10 | |
| openstackgerrit | Tetsuro Nakamura proposed openstack/placement master: Support `same_subtree` queryparam https://review.opendev.org/668376 | 12:42 |
| *** jaypipes has joined #openstack-placement | 12:46 | |
| *** tetsuro has quit IRC | 12:47 | |
| *** mriedem has joined #openstack-placement | 13:14 | |
| *** helenafm has quit IRC | 13:56 | |
| efried | cdent: -nova has been pretty noisy this morning; did you see my response about retries on set_inventory_for_provider? | 14:07 |
| *** helenafm has joined #openstack-placement | 14:14 | |
| openstackgerrit | Eric Fried proposed openstack/placement stable/stein: [Stein only] Correct Ubuntu package name https://review.opendev.org/668680 | 14:14 |
| openstackgerrit | Eric Fried proposed openstack/placement stable/stein: [Stein only] Correct Ubuntu package name https://review.opendev.org/668680 | 14:16 |
| mriedem | i left some comments on https://review.opendev.org/#/c/668680/ | 14:36 |
| mriedem | i'm not sure about that one | 14:37 |
| mriedem | the user might not have had the stein UCA enabled? | 14:37 |
| efried | mriedem: thanks for that. We've been pestering people to look at those distro install guides and they just haven't; maybe this tack will work. | 14:39 |
| efried | (I had very little expectation that I actually got it right as is) | 14:39 |
| mriedem | coreycb is the ubuntu person i always reach out to | 14:39 |
| efried | gtk | 14:39 |
| cdent | efried: I didn't until now. Doing a retry of any sort, I get. What I'm not clear about is if/how the report client's provider tree needs to be reset between retries | 14:44 |
| efried | cdent: We only update the report client's provider tree on successful API calls. | 14:44 |
| cdent | efried: the issue is this: the generation is wrong when making the first set_... call. What will make sure the generation goes correct? | 14:45 |
| efried | cdent: Right, the strategy (for this and all generation-conflict retries) is: | 14:47 |
| efried | @retry | 14:47 |
| efried | GET (which may have happened at some earlier point, if I'm caching, but even if I do it right now I could still be racing | 14:47 |
| efried | set_inventory_for_provider | 14:47 |
| efried | sorry, that was too general in some ways and too specific in others. The point is that the retry has to wrap GET and PUT both. | 14:48 |
| efried | In the case of _update_to_placement, the GET happens in get_provider_tree_and_ensure_root | 14:49 |
| cdent | right, and I was hoping/thinking I could force something like this: | 14:49 |
| cdent | @retry ; try: iset_inventory... ; except Conflict: self._clear_provider_cache_for_tree(rp_uuid) | 14:50 |
| cdent | (sorry, that's proably insufficiently verbose) | 14:50 |
| cdent | but I guess I cannot do that because there would still need to be a GET? | 14:51 |
| efried | in the _update_to_placement flow, the cache clearing is done by update_from_provider_tree. | 14:51 |
| efried | so yeah, it's all pretty spread out | 14:51 |
| efried | one sec and I'll answer your last question... | 14:51 |
| efried | yes, you definitely still need a GET if the failure was 409 generation conflict, because otherwise how would you get the latest generation? | 14:52 |
| efried | (and the latest inventory to make your changes to - but assuming you just want to overwrite the inventory regardless of whatever's in there, you still need the latest generation) | 14:53 |
| cdent | I had this perhaps unfortunate belief that invalidating the cache would cause it to refill itself the next time you queried for data, but this is apparently not the case | 14:54 |
| efried | the next time you queried what? set_inventory_for_provider receives an inventory dict. | 14:55 |
| cdent | set_inventor_for_provider looks in the provider tree for data.generation | 14:55 |
| efried | I guess it could refill when it does the has_inventory_changed check | 14:55 |
| efried | o | 14:56 |
| efried | so yeah, I don't think we would want to have it clear and refresh | 14:56 |
| cdent | that's fine that it doesn't | 14:56 |
| efried | You got information at some point, and hopefully if everybody is behaving well, whatever GET you did populated the cache, so the generation that's in there matches your assumption of what you started with before you changed inventory. | 14:57 |
| cdent | but I need to determine the right call (and lightest) to make a refresh of the provider generation... | 14:57 |
| cdent | yeah, this is not a behaving welll situation | 14:57 |
| efried | if a conflict cleared and repopulated the cache, and you didn't redrive whatever that get was, you'd be operating on a stale inventory. | 14:57 |
| cdent | this is a dirty hack within a virt driver | 14:58 |
| efried | sorry, I mean code organized behaving well, not problems on the server or races or whatever. | 14:58 |
| efried | got it. mriedem is doing something similar, with this same function, somewhere or other. | 14:58 |
| *** evrardjp_away has quit IRC | 14:58 | |
| cdent | which used to work until _update_inventory was removed and someone (definitely not me) came along and replaced it with set_inventory_for_provider without knowing how it worked | 14:58 |
| *** evrardjp has joined #openstack-placement | 14:59 | |
| efried | hm, I'm not seeing whatever you're looking at. In master the only non-test use of set_inventory_for_provider is from update_from_provider_tree. | 15:00 |
| cdent | [t 8Dsw] | 15:01 |
| purplerbot | <cdent> this is a dirty hack within a virt driver [2019-07-02 14:58:03.414443] [n 8Dsw] | 15:01 |
| cdent | as in, this is not upstream | 15:01 |
| cdent | as far as I can tell the hack would be: | 15:02 |
| cdent | @retry_ensure_provider | 15:02 |
| cdent | nix that | 15:03 |
| cdent | one mo | 15:03 |
| *** edmondsw_ has joined #openstack-placement | 15:04 | |
| cdent | efried: something like this http://paste.openstack.org/show/753765/ | 15:05 |
| cdent | assuming we intend to clobber (not merge) inventory | 15:05 |
| efried | cdent: since set_inventory_for_provider does not itself invalidate the cache, you'll have to do that too. | 15:06 |
| cdent | ah, okay, that's the missing piece | 15:07 |
| efried | I would be supportive of a change to incorporate that. | 15:07 |
| efried | making it redundant for that one sub-path in update_from_provider_tree | 15:07 |
| efried | but I'm fine with that. | 15:07 |
| efried | so it be like: | 15:08 |
| efried | @retry | 15:08 |
| efried | _ensure | 15:08 |
| efried | try: | 15:08 |
| efried | set_inventory_for_provider | 15:08 |
| efried | except Conflict: | 15:08 |
| efried | # clear cache for @retry | 15:08 |
| efried | scrub cache | 15:08 |
| efried | raise | 15:08 |
| * cdent nods | 15:09 | |
| efried | or something | 15:09 |
| efried | this involves accessing private things, though | 15:09 |
| efried | which is a dirty hack | 15:09 |
| efried | makes one question whether you should actually be trying to reuse report client methods or rolling your own. | 15:10 |
| cdent | on making a change to incorporate something like for set_inventory... that seems a slippery slope because then you'd want it for set_traits and set_aggregates too and people just use the provider tree for real | 15:10 |
| cdent | this stuff I have in front of me right now is _way_ down in the mud | 15:10 |
| efried | agree there are tradeoffs, but yes if we wanted to make cache clearing automatic for one, we would want same for all. | 15:10 |
| cdent | and is acknowledged as such. it even has TODOs which says things like "we should do this properly" | 15:10 |
| efried | today the methods are all over the place in terms of scope. | 15:10 |
| efried | Some just broker raw REST and do little more than convert args to json | 15:11 |
| cdent | anyway, I think I have enough to go on for this little thing, and enough to think about for making the future brighter. thanks for your help | 15:11 |
| efried | some interpret returns and do things | 15:11 |
| cdent | LET'S REWRITE IT ALL | 15:11 |
| efried | sometimes things involve the cache, sometimes not | 15:11 |
| efried | yeah, let's. | 15:11 |
| efried | starting with murdering @safe_connect. | 15:11 |
| cdent | in jerusalem | 15:12 |
| cdent | speaking of: i have a POC for mappings on AllocationRequest but it's stalled while I'm called away by this hack | 15:12 |
| efried | okay. Does it look like it has legs? | 15:13 |
| efried | Whether it does or not, I'm not sure if there's value in waiting to merge the existing fix. I would think we could put such optimizations/refactors on top. | 15:14 |
| cdent | a) I think it probably has legs, b) it's not an optimization, it's a correction | 15:16 |
| cdent | c) it should be done pretty soon | 15:16 |
| cdent | plural suffixes is just all kinds of wrong | 15:16 |
| efried | um | 15:16 |
| efried | we need plural suffixes | 15:16 |
| efried | without question | 15:16 |
| efried | per AllocationRequest | 15:16 |
| cdent | i mean plural suffixes on ARR | 15:16 |
| efried | mm, again disagree, under the exact circumstance of the bug. | 15:17 |
| cdent | having multiple suffixes on AR is good and just | 15:17 |
| efried | combining due to group_policy=none | 15:17 |
| efried | one ARR represents amount for one rp+rc | 15:17 |
| cdent | no | 15:17 |
| efried | which can be the accumulation of multiple RGs when group_policy=none | 15:17 |
| cdent | one ARR represents one rp+rc+suffix | 15:17 |
| efried | then it doesn't map to the output | 15:18 |
| efried | which (I thought) was one of its main purposes | 15:18 |
| efried | we have to combine at some point | 15:18 |
| efried | you're just suggesting (I assume) pushing that combining to the output processing | 15:18 |
| efried | instead of _consolidate_... | 15:18 |
| cdent | i think we've got hung up in my implementation. | 15:19 |
| cdent | no | 15:19 |
| cdent | let me do this thing and then I'll finish the POC and you can see | 15:19 |
| efried | suresure | 15:19 |
| * efried bbiab | 15:19 | |
| cdent | whatever rush you are imaging doesn't exist | 15:19 |
| *** cdent has quit IRC | 15:24 | |
| *** e0ne has quit IRC | 15:27 | |
| *** ttsiouts has quit IRC | 15:31 | |
| *** tssurya has quit IRC | 15:40 | |
| *** helenafm has quit IRC | 15:52 | |
| *** cdent has joined #openstack-placement | 15:54 | |
| *** artom|gmtplus3 has quit IRC | 15:56 | |
| cdent | my internet goes weird at the strangest times | 15:57 |
| efried | It's not a rush cdent, just refactoring where .suffix(es) lives is a separate thing from fixing the bug. | 16:31 |
| efried | also may or may not have an impact on same_subtree+resourceless | 16:32 |
| cdent | which is why I'd like to get it right before subtree gets too far along | 16:32 |
| cdent | both your fix and tetsuro's work override the ARR in a way that tickles my maintenance problems coming soon radar | 16:32 |
| efried | look, I'm not saying that that having suffixes on AR is wrong. I'm just saying neither is having them on ARR. | 16:50 |
| efried | If we move them to AR, we have to keep track of mapping data as well - which may wind up being the form the information has to take on the AR (there wouldn't be much point in having .suffixes and then having .mappings separately). | 16:50 |
| efried | Maybe we need a new Mappings class that lives on AR and gets built up as we go. | 16:50 |
| cdent | what I've done is made a mappings attribute on the AR | 16:50 |
| cdent | which makes sense because that's what we need | 16:50 |
| cdent | 'suffix' (singular) made sense on the ARR but as you discovered doesn't actually work | 16:51 |
| cdent | my initial mistake was to use suffix on the ARR at all. it's entirely correct | 16:52 |
| cdent | and now I'm trying to fix it | 16:52 |
| cdent | it's _not_ entirely correct | 16:52 |
| * cdent sighs | 16:52 | |
| *** belmoreira has quit IRC | 17:09 | |
| *** amodi has joined #openstack-placement | 17:55 | |
| openstackgerrit | Chris Dent proposed openstack/placement master: Manage mappings on AllocationRequest object https://review.opendev.org/668724 | 18:05 |
| cdent | efried: there it is ^. Probably not quite perfect yet, but it quells my concerns and seems to work okay | 18:05 |
| efried | cool | 18:06 |
| cdent | damn, I was so close to having 0 extant patches | 18:15 |
| * cdent quits while he's nearly ahead | 18:17 | |
| cdent | goodnight | 18:17 |
| *** cdent has quit IRC | 18:17 | |
| *** tssurya has joined #openstack-placement | 20:37 | |
| *** evrardjp has quit IRC | 20:51 | |
| *** evrardjp has joined #openstack-placement | 20:52 | |
| *** ttsiouts has joined #openstack-placement | 20:55 | |
| *** ttsiouts has quit IRC | 21:37 | |
| openstackgerrit | Chris Dent proposed openstack/placement master: Manage mappings on AllocationRequest object https://review.opendev.org/668724 | 21:49 |
| *** tssurya has quit IRC | 21:50 | |
| *** ttsiouts has joined #openstack-placement | 22:16 | |
| *** ttsiouts has quit IRC | 22:21 | |
| efried | cdent: I was focusing hard on specs today, so I probably won't have had a chance to look at that mappings-in-AR fixup by the time you sign back on. It's high on my list though. | 23:10 |
| *** mriedem has quit IRC | 23:20 | |
Generated by irclog2html.py 2.15.3 by Marius Gedminas - find it at mg.pov.lt!