| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-09-28 | |||
| 20:12:52 | mriedem | and we allow those | |
| 20:13:12 | dansmith | an out-of-tree driver that uses our filters? | |
| 20:13:16 | mriedem | sure | |
| 20:13:18 | mriedem | like, | |
| 20:13:24 | mriedem | maybe i extend CachingScheduler | |
| 20:13:32 | mriedem | because i like to have fun | |
| 20:13:44 | dansmith | out of tree filters and weighers I can see, but.. whole drivers? | |
| 20:13:59 | mriedem | it's a thing i guess, and we broke it in ocata, | |
| 20:14:05 | mriedem | and had to fix that in pike and backport | |
| 20:14:11 | mriedem | since we never deprecated that ability formally | |
| 20:14:15 | dansmith | with an out of tree driver you're going to end up with fubar'd placement and such | |
| 20:14:35 | mriedem | do we default to use placement or not... | |
| 20:14:46 | mriedem | USES_ALLOCATION_CANDIDATES = True | |
| 20:14:49 | mriedem | we default to use placement | |
| 20:15:28 | mriedem | btw, | |
| 20:15:31 | dansmith | I guess I'm not sure where the seam is, are you saying that we do the claim late enough that it's run for every driver? | |
| 20:15:40 | mriedem | it seems a bit nutty that we join on system_metadata when listing all instances with details | |
| 20:16:01 | dansmith | we used to have to have that join for flavor info | |
| 20:16:06 | mriedem | yeah, i figured, | |
| 20:16:08 | mriedem | but that's long gone | |
| 20:16:19 | mriedem | do you still have your perf box env setup? | |
| 20:16:31 | dansmith | I think it will come back up ready, lemme see | |
| 20:16:47 | dansmith | I was also thinking of another thing I could do: | |
| 20:16:52 | mriedem | for the scheduling thing, if the driver says USES_ALLOCATION_CANDIDATES=False, we don't ask placement for anything | |
| 20:16:57 | dansmith | put duplicate cell entries in for the same cell to cause us to list across more cells for free | |
| 20:17:09 | mriedem | and we don't attempt to claim in the scheduler | |
| 20:17:28 | dansmith | we could make those filters refuse to load if driver is set to the filter scheduler, just flip the logic | |
| 20:17:44 | dansmith | I mean log deprecation now, and fail in rocky | |
| 20:17:57 | mriedem | that seems ok | |
| 20:18:37 | dansmith | we really need to be removing the honoring of the limits provided by those filters from compute anyway I think | |
| 20:18:50 | dansmith | we've not really done any culling of stuff that is now handled by placement from compute/rt | |
| 20:20:20 | mriedem | speaking of culling | |
| 20:20:22 | mriedem | _get_all_instance_metadata | |
| 20:20:24 | mriedem | in compute api | |
| 20:20:31 | mriedem | apparently the only things that use that, aren't used by anything else | |
| 20:22:20 | mriedem | i'm going through https://review.openstack.org/#/c/505418/ btw | |
| 20:22:25 | mriedem | hence asking random questions | |
| 20:23:19 | dansmith | thank you | |
| 20:23:53 | dansmith | my devstack setup came back so I'll poke at sysmeta | |
| 20:27:09 | mriedem | ok comments inline | |
| 20:27:23 | dansmith | mriedem: is that one of the tests I pulled out to the cells class in an earlier patch? | |
| 20:27:27 | mriedem | nope | |
| 20:27:28 | mriedem | just looked | |
| 20:27:40 | dansmith | okay | |
| 20:27:59 | mriedem | checking to see if anything else covers that | |
| 20:28:05 | mriedem | we tend to duplicate a lot of our unit tests | |
| 20:30:16 | mriedem | _get_all_instance_metadata is only used by methods that were for the ec2 api | |
| 20:31:03 | mriedem | ec2api repo doesn't call them though | |
| 20:34:07 | dansmith | hmm, got worried for a sec | |
| 20:34:24 | dansmith | baseline was taking 10s instead of 6s from yesterday | |
| 20:34:47 | dansmith | but after the reboot the devstack@dstat.service was consuming two cores for some reason | |
| 20:34:50 | dansmith | hopefully that's why | |
| 20:39:05 | mriedem | ok we return metadata during GET /servers/{server_id} which makes sense, so still need to join on that | |
| 20:39:11 | mriedem | and flavor for flavor, and info_cache for IPs, | |
| 20:39:19 | mriedem | but system_metadata should be able to be nuked from the join in the API | |
| 20:41:07 | mriedem | oh | |
| 20:41:09 | mriedem | geez | |
| 20:41:14 | mriedem | we join on security_groups... | |
| 20:41:25 | mriedem | for no good reason | |
| 20:41:42 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Remove unused get_all_instance_*metadata methods https://review.openstack.org/508299 | |
| 20:42:43 | mriedem | gmann: Kevin_Zheng: shouldn't JOINED_TABLE_QUERY_PARAMS_SERVERS include 'tags'? | |
| 20:43:32 | dansmith | hrm, still running slow | |
| 20:44:01 | mriedem | if we removed system_metadata from the default join list in the API, if it was used somewhere, we'd see the "lazy-loading ..." message in the API logs though right? | |
| 20:44:58 | dansmith | yes | |
| 20:50:16 | mriedem | gmann: Kevin_Zheng: oh nvm it can't because "tags" is an actual query parameter | |
| 20:50:33 | dansmith | for a single list operation via curl I really shouldn't be hitting keystone more than once right? | |
| 20:50:53 | mriedem | hmmm | |
| 20:51:05 | mriedem | going to neutron? | |
| 20:51:22 | mriedem | we pass the token to neutron and it has to auth? | |
| 20:51:23 | dansmith | for a list? | |
| 20:51:42 | mriedem | we proxy the security group information during list to neutron | |
| 20:51:53 | dansmith | I'm just trying to figure out why this is taking double what it was yesterday | |
| 20:51:56 | mriedem | i think anyway, this has come up before b/c we don't cache security groups | |
| 20:57:36 | mriedem | nova meeting in 3 minutes | |
| 20:58:49 | mriedem | jaypipes: even if the instance_security_groups table is empty, i'm assuming that listing 1000 instances and joining on that table is not insignificant? | |
| 20:59:38 | mriedem | sorry the security_group_instance_association table | |
| 20:59:57 | jaypipes | mriedem: well, if i_s_g is empty, it's an insignificant thing. | |
| 21:00:28 | jaypipes | mriedem: if there's no records, the join is optimized out by the DB. that said, as soon as it starts to get many records in it, boom. | |
| 21:00:41 | mriedem | if using neutron it shouldn't ever have records in it | |
| 21:00:46 | mriedem | system_metadata totally will though | |
| 21:01:05 | mriedem | anyway, meeting time | |
| 21:01:32 | dansmith | mriedem: I'm restacking to make sure I'm clean and measuring what I expect, because things are taking twice what they should be | |
| 21:02:09 | mriedem | ok | |
| 21:12:37 | jaypipes | efried: care to update https://review.openstack.org/#/c/497713/ to say required= and let's ship it? | |
| 21:12:50 | tonyb | dansmith: I should've added a comment but I'd only just +wd the pike version so I was waiting for that to merge | |
| 21:13:08 | dansmith | tonyb: okay, well I hit it anyway | |
| 21:13:12 | efried | jaypipes Sure, I can do that. | |
| 21:13:26 | jaypipes | efried: ty | |
| 21:16:16 | openstackgerrit | Eric Fried proposed openstack/nova-specs master: Add trait support in the allocation candidates API https://review.openstack.org/497713 | |
| 21:16:22 | efried | jaypipes hecho ^ | |
| 21:16:42 | jaypipes | efried: danke | |
| 21:18:42 | cdent | i guess I better actually read that one | |
| 21:19:05 | openstackgerrit | Dan Smith proposed openstack/nova master: Fix CellDatabases fixture swallowing exceptions https://review.openstack.org/506312 | |
| 21:19:05 | openstackgerrit | Dan Smith proposed openstack/nova master: Use improved instance_list module in compute API https://review.openstack.org/505418 | |
| 21:19:06 | openstackgerrit | Dan Smith proposed openstack/nova master: Move cell marker tests to Cellsv1DeprecatedTestMixIn https://review.openstack.org/508314 | |
| 21:19:06 | openstackgerrit | Dan Smith proposed openstack/nova master: Fix minor input items from previous patches https://review.openstack.org/506416 | |
| 21:25:13 | efried | jaypipes Oh, did you mean https://review.openstack.org/#/c/468797/ ? I can update that one too... | |
| 21:25:30 | jaypipes | efried: that would be great, too. | |
| 21:25:37 | jaypipes | efried: that's the flavor changes, right? | |
| 21:25:51 | efried | They're different, mind you: one's in flavor and the other's in API. But with the current proposals I don't see any reason they shouldn't both be required= | |