| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-07-28 | |||
| 13:42:51 | leakypipes | for the record, guys, I should be done with my patch shortly. | |
| 13:43:00 | bauzas | mriedem: yup, saw the pike-3 change this morning | |
| 13:43:02 | leakypipes | just working on unit tests now | |
| 13:43:15 | bauzas | it was already merged | |
| 13:43:19 | dansmith | mriedem: insert some artificial rpc delay into the periodic and see if #1 happens | |
| 13:43:41 | mriedem | i can do that | |
| 13:44:30 | mriedem | cdent: bauzas: also https://bugs.launchpad.net/nova/+bug/1707071 if you haven't seen that yet | |
| 13:44:31 | openstack | Launchpad bug 1707071 in OpenStack Compute (nova) ocata "Compute nodes will fight over allocations during migration" [Medium,Confirmed] | |
| 13:44:40 | bauzas | not yet indeed | |
| 13:44:57 | bauzas | mriedem: dansmith: leakypipes: I thought about something this night | |
| 13:45:04 | bauzas | cdent: ^ | |
| 13:45:13 | leakypipes | dansmith: you need to change your clothes. | |
| 13:45:31 | bauzas | what if we should just get the current allocations before deleting them by the scheduler, so in case we know we have a problem, we could put them again ? | |
| 13:45:32 | dansmith | heh | |
| 13:45:42 | bauzas | oh snap, FF | |
| 13:46:07 | cdent | thanks mriedem | |
| 13:46:48 | superdan | bauwser: get + check is just as safe as get + delete + re-put, except you never have to delete | |
| 13:46:49 | superdan | not sure why we would do the latter | |
| 13:47:06 | bauwser | mriedem: superdan: leakypipes: oh, and me and cdent just discussed about https://review.openstack.org/#/c/427200/ : it could be important for operators | |
| 13:47:50 | bauwser | superdan: sure, was just wondering if it was simplier than just trying to get two allocations for both source and target when moving | |
| 13:48:00 | bauwser | I know it's the consensus | |
| 13:48:05 | superdan | I don't see how it helps | |
| 13:48:06 | bauwser | and I totally agree with it | |
| 13:48:06 | mriedem | bauwser: yes there is an odd scenario in there which i commented on | |
| 13:48:14 | superdan | without two allocations we're not accounting for the resources used by a moving instance | |
| 13:48:19 | bauwser | but I do wonder if we could simplify the problem | |
| 13:48:33 | bauwser | at least for the races we know of | |
| 13:48:48 | superdan | well, the double allocation. there's still only one allocation for the instance | |
| 13:49:08 | bauwser | the problem here is that we have 3 different services looking at placement when moving : #1 scheduler, #2 source compute, #3 target compute | |
| 13:49:21 | superdan | that's kindof the point of placement right? | |
| 13:49:40 | bauwser | yeah, I know, but that means we have like shared information | |
| 13:49:56 | bauwser | but I think we discussed that yesterday | |
| 13:49:56 | mriedem | that's the point | |
| 13:49:57 | mriedem | the global view | |
| 13:50:03 | superdan | yeah, that's the whole thing | |
| 13:50:11 | bauwser | anyway, I don't want to nitpick | |
| 13:50:26 | mriedem | but yes that means old code that assumes it's all local and owns everything at any given time has to change | |
| 13:50:32 | superdan | it's not the shared state/view/responsibility that concerns me, it's that the current RT was designed for a different model | |
| 13:50:38 | bauwser | I'm just trying to see how to help with the problems we know about multiple services taking the same allocations | |
| 13:51:43 | leakypipes | hopefully my new patch's code comments explain the situation well enough. | |
| 13:51:47 | bauwser | ideally, eventually, I'm not sure we need resouretrackers for computes | |
| 13:52:01 | superdan | bauwser: we don't in the long run | |
| 13:52:01 | bauwser | if allocations would only be done by schedulers | |
| 13:52:15 | superdan | we still need some of that code on the computes, | |
| 13:52:19 | superdan | but not the full RT | |
| 13:52:23 | bauwser | I'd see RTs just *deleting* allocations if something goes mad | |
| 13:52:26 | superdan | that's what I was saying yesterday | |
| 13:52:28 | bauwser | superdan: yeah, I know | |
| 13:52:46 | bauwser | I'm just thinking out loud to try to identify how we could simplify | |
| 13:52:48 | superdan | so.... :) | |
| 13:52:57 | superdan | okay :) | |
| 13:53:01 | bauwser | but I agree with you, using the existing RT means tech deby | |
| 13:53:22 | superdan | Tech Deby.. that's like the host of a kids show about computers? | |
| 13:54:25 | bauwser | :) | |
| 13:54:43 | bauwser | my keyboard is AZERYT :p | |
| 13:55:37 | superdan | T and Y are together on both | |
| 13:55:49 | superdan | I thought it was AZERTY? | |
| 13:56:59 | bauwser | rather, yting | |
| 13:59:59 | cdent | can someone merge this https://review.openstack.org/#/c/488363/ looking at that warning is getting tiresome | |
| 14:05:28 | kashyap | superdan: Thanks for the review on this: https://review.openstack.org/#/c/485752/ | |
| 14:05:38 | kashyap | superdan: And thanks for the little snark, too :P | |
| 14:06:10 | superdan | heh | |
| 14:06:56 | kashyap | Can anyone +W it? I can also backport it to the revlevant upstream branches | |
| 14:09:28 | superdan | kashyap: sdague loves +Wing patches like that | |
| 14:10:01 | kashyap | :-) I thought of pinging him explicitly, but refrained in the spirit of being a good citizen as I shouldn't specifically nag people | |
| 14:10:15 | kashyap | And just ask the generic "ether", that is the channel :-) | |
| 14:10:31 | superdan | kashyap: sdague owes us all a beer after yesterday, so I think today it's uniquely okay to ask him directly :P | |
| 14:11:08 | kashyap | sdague: If you are listening in, it's a straight-forward perf issue fixed by calling a simple "please set cache mode" method. And the reporter has even confirmed the fix works -- https://review.openstack.org/#/c/485752/ | |
| 14:11:25 | superdan | cdent: fix mriedem's comment and I'll fast approve | |
| 14:11:36 | kashyap | superdan: What happened yesterday that bestows this windfall on the rest? | |
| 14:11:53 | superdan | kashyap: oh nothing.. just a good-faith-gone-awry sort of deal | |
| 14:11:59 | kashyap | :D | |
| 14:13:44 | openstackgerrit | Chris Dent proposed openstack/nova master: [placement] quash unicode warning with shared provider https://review.openstack.org/488363 | |
| 14:14:04 | cdent | superdan: done ^. tvm | |
| 14:14:26 | superdan | cdent: doneski | |
| 14:14:40 | cdent | rawkin or something | |
| 14:17:32 | bauwser | mriedem: cdent: FYI https://review.openstack.org/#/c/488363/1/nova/objects/resource_provider.py@741 | |
| 14:17:57 | bauwser | I'm not a SQLA expert, but I know that you need all your strings to be unicode (and UTF-8) if you want to use SQLA | |
| 14:18:36 | cdent | bauwser: I think the relevant point here is that os_traits is _not_ which might be a surprise to some (it was to me) | |
| 14:18:47 | bauwser | cdent: ostraits is a python lib | |
| 14:18:59 | bauwser | cdent: so it really just gives us what python gives us | |
| 14:19:11 | bauwser | there is no contract about what it returns | |
| 14:19:18 | cdent | yes, I know that, but no, it has a choice of how it constructs the values | |
| 14:19:20 | bauwser | and a shit ton of 3rd-party libs behave the same | |
| 14:19:21 | cdent | it could have don them as unicdoe | |
| 14:19:54 | bauwser | the point is just, SQLA by default expects unicode, so in general you explicitely convert | |
| 14:20:18 | bauwser | cdent: that reminds me | |
| 14:20:19 | cdent | yes, and? | |
| 14:20:44 | bauwser | cdent: I'm not sure that if we create a trait like CUSTOM_mé | |
| 14:20:51 | bauwser | it will correctly work | |
| 14:20:59 | bauwser | because we won't define the charset | |
| 14:22:10 | cdent | "pattern": "^CUSTOM\_[A-Z0-9_]+$", | |
| 14:22:25 | cdent | so it is moot on that front | |
| 14:22:45 | cdent | none of this changes that matt’s right that a comment is helpful | |
| 14:22:49 | cdent | it’s been done, it’s merged. | |
| 14:25:01 | bauwser | cdent: oh correct, we only accept ASCII | |
| 14:25:04 | bauwser | even less | |
| 14:25:11 | bauwser | so that's not a big deal | |
| 14:31:00 | kashyap | mriedem: Thanks for the merge! | |
| 14:31:48 | kashyap | (For this https://review.openstack.org/#/c/485752/) | |