| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-04-06 | |||
| 20:44:26 | dansmith | right | |
| 20:44:51 | mriedem | ok i also need to update https://review.openstack.org/#/c/453916/ since i didn't realize the compute manager and host manager were doing the exact same thing in the init | |
| 20:44:55 | dansmith | claims in the scheduler will let us do affinity without all that stuff | |
| 20:45:17 | cdent | mriedem: I haven't caught up on backlog, but are you looking at the stuff that does affinity based on live host/instance info? | |
| 20:45:28 | mriedem | so if you're using caching scheduler or not using these filters/weighers, and not paying attention to default options, you could have all of this extra rpc traffic and db lookups for nothing | |
| 20:45:47 | mriedem | cdent: it's the instance info stuff in the host manager and scheduler filter yeah | |
| 20:46:13 | dansmith | this mess is the source of my tweet from last week :) | |
| 20:46:17 | mriedem | dansmith: like https://github.com/openstack/nova/blob/master/nova/scheduler/host_manager.py#L731 | |
| 20:46:31 | mriedem | if we knew you weren't using those filters/weighers, that could just be a noop | |
| 20:46:37 | mriedem | but, those are defaults so i guess... | |
| 20:46:47 | cdent | those are the things that make me say that we'll never have a fully single transaction placement engine | |
| 20:46:48 | dansmith | mriedem: well, also, | |
| 20:46:55 | dansmith | mriedem: compute sends instance info when an instance boots, | |
| 20:47:03 | dansmith | mriedem: but host manager just uses that as a trigger to re-query the db | |
| 20:47:05 | dansmith | it's madness | |
| 20:47:20 | mriedem | to refresh it's cache right? | |
| 20:47:36 | dansmith | yeah | |
| 20:47:37 | dansmith | but it sends the whole instance, | |
| 20:47:40 | dansmith | and we just ignore it and re-query | |
| 20:47:43 | dansmith | unless it sends all of them, | |
| 20:47:45 | dansmith | and then we assume it's right | |
| 20:48:07 | mriedem | cripes | |
| 20:48:09 | dansmith | it's very complicated and wasteful | |
| 20:49:26 | mriedem | "ServerGroupAntiAffinityFilter", | |
| 20:49:27 | mriedem | "ServerGroupAffinityFilter", are default filters | |
| 20:49:35 | mriedem | and filter scheduler is default | |
| 20:49:39 | mriedem | and track_instance_changes is on by default | |
| 20:50:34 | mriedem | oh, and | |
| 20:50:51 | mriedem | all computes have a periodic task that run every 2 minutes by default to pull all instances from that host and send them to the scheduler | |
| 20:51:07 | cdent | huzzah | |
| 20:51:18 | dansmith | which nobody can have enabled at that frequency with a reasonable number of computes I'm sure | |
| 20:51:18 | mriedem | i wonder if that filters out deleted instances... | |
| 20:51:56 | mriedem | oh plus that does the big default joins | |
| 20:52:48 | dansmith | of course | |
| 20:52:52 | dansmith | go big or go home | |
| 20:52:56 | mriedem | well maybe we don't | |
| 20:53:00 | mriedem | we pass expected_attrs=[] | |
| 20:53:04 | mriedem | to try and trick that i think | |
| 20:53:14 | dansmith | okay yeah that won't do the joins then | |
| 20:53:16 | dansmith | BUT | |
| 20:53:18 | dansmith | still stupid :) | |
| 20:53:20 | mriedem | but i wouldn't be surprised if there is an "if not expected_attrs: expected_attrs = DEFAULT_BIG_ASS_LIST" | |
| 20:53:35 | dansmith | no it checks =None | |
| 20:53:37 | dansmith | =None means default =[] means nothing | |
| 20:53:55 | mriedem | i was thinking the db api | |
| 20:54:05 | mriedem | so, | |
| 20:54:07 | mriedem | instance_get_all_by_host in the db api, | |
| 20:54:11 | mriedem | does not filter out deleted instances | |
| 20:54:17 | dansmith | WAT | |
| 20:54:30 | mriedem | oh wait | |
| 20:54:36 | mriedem | model_query filters deleted right? | |
| 20:54:40 | dansmith | depends on context | |
| 20:54:59 | openstackgerrit | Sihan Wang proposed openstack/nova master: Update checking vm state in vmware driver https://review.openstack.org/454328 | |
| 20:56:10 | mriedem | def _instance_get_all_query(context, project_only=False, joins=None): | |
| 20:56:10 | mriedem | if joins is None: | |
| 20:56:10 | mriedem | joins = ['info_cache', 'security_groups'] | |
| 20:56:20 | mriedem | so we are at least joining on those mofos | |
| 20:56:34 | dansmith | but should be not =None there if we're passing =[] | |
| 20:56:39 | mriedem | not there | |
| 20:57:00 | mriedem | https://github.com/openstack/nova/blob/master/nova/db/sqlalchemy/api.py#L2530 | |
| 20:57:09 | mriedem | we don't pass columns_to_join to _instance_get_all_query | |
| 20:57:10 | dansmith | you said we were passing expected_attrs=[] right? | |
| 20:57:11 | mriedem | so it's None | |
| 20:57:22 | mriedem | we pass [] to _instances_fill_metadata | |
| 20:57:28 | mriedem | but not _instance_get_all_query | |
| 20:57:35 | dansmith | ah | |
| 20:57:39 | mriedem | https://github.com/openstack/nova/blob/master/nova/db/sqlalchemy/api.py#L2513 | |
| 20:57:39 | dansmith | so the funny thing is, | |
| 20:57:53 | dansmith | if we just sent an empty call to that update method on the scheduler, | |
| 20:58:02 | dansmith | it would do it itself, direct from the db, no big rpc involved | |
| 20:58:11 | dansmith | because remember this goes compute->conductor->compute->scheduler over rpc | |
| 20:58:25 | dansmith | but if you just call that method with an empty list, | |
| 20:58:33 | dansmith | scheduler will just pull it itself without any rpc | |
| 20:58:39 | cdent | ha! | |
| 20:58:40 | mriedem | that's _update_scheduler_instance_info ? | |
| 20:58:40 | dansmith | or maybe something else to fool it | |
| 20:58:47 | dansmith | whatever that scheduler rpc call is | |
| 20:58:48 | cdent | that's awesome | |
| 20:59:06 | dansmith | cdent: we should figure out more places to route that through | |
| 20:59:20 | dansmith | compute->conductor->compute->twitter->facebook->compute->conductor->super-conductor->scheduler | |
| 20:59:38 | cdent | i think you left out the pigeon | |
| 20:59:53 | dansmith | heh | |
| 21:00:27 | mriedem | dansmith: you're talking about this right? https://github.com/openstack/nova/blob/master/nova/scheduler/host_manager.py#L767 | |
| 21:00:30 | mriedem | where it does the full pull | |
| 21:00:43 | dansmith | mriedem: yeah | |
| 21:00:51 | mriedem | but that's only if it doesn't already have a list for the host | |
| 21:01:02 | mriedem | https://github.com/openstack/nova/blob/master/nova/scheduler/host_manager.py#L752 | |
| 21:01:09 | mriedem | that gets created when the compute starts up | |
| 21:01:19 | mriedem | which is what we were talking about yesterday | |
| 21:01:35 | dansmith | sure, but we're still sending it | |
| 21:01:36 | dansmith | for no reason | |
| 21:01:42 | mriedem | yes that's dumb too | |
| 21:01:43 | dansmith | we'd have to tweak the logic there just a bit, agreed | |
| 21:01:54 | mriedem | i'm just thinking https://github.com/openstack/nova/blob/master/nova/scheduler/host_manager.py#L767 is a complete fallback | |
| 21:02:05 | mriedem | if the compute host didn't send the instances on startup | |
| 21:02:09 | mriedem | to register itself with the scheduler | |
| 21:02:15 | mriedem | err host manager | |
| 21:02:32 | mriedem | https://github.com/openstack/nova/blob/master/nova/scheduler/host_manager.py#L761 is the compute start case | |
| 21:02:41 | mriedem | well, unless it's a new compute with no hosts | |
| 21:02:42 | dansmith | we only get here in the rpc call right? | |