Earlier  
Posted Nick Remark
#openstack-nova - 2018-05-17
19:39:33 efried mriedem: I caught up with Gerald. He cited a customer scenario with 500 volumes on an instance. So 1024 seems like a safe and relatively future-proof number.
19:52:07 dansmith efried: um, I had already provided a customer scenario with 600, although no other detail, so clearly 1024 makes sense in a real-world situation
19:52:49 efried dansmith: Okay, I didn't follow that conversation to its end. Just got an answer so thought I would post it.
19:53:50 dansmith efried: how is "some customer wants to use 500" a reasonable argument? I guess my overt sarcasm wasn't overt enough
19:54:50 efried now you've lost me.
19:55:26 dansmith efried: I want to hear the *reason* for wanting to use 500 volumes on a single guest, not just that someone thinks it's a good idea
19:55:27 efried You mean we should require said customers to give justification that we agree with, as opposed to letting us tell them how many volumes they should be using instead?
19:55:39 efried yeah, got it.
19:56:18 dansmith the bug/bp being filed seems to indicate that someone somewhere wants to do it
19:56:20 efried In this case, the customer is using IBMi and has 16 FC adapters and is doing huge striping to get desired performance.
19:56:48 dansmith hah, I was going to say raid0ing of 1GB volumes
20:07:09 openstackgerrit Merged openstack/nova master: Deduplicate config/policy reference docs from main index https://review.openstack.org/564846
20:09:58 openstackgerrit Matt Riedemann proposed openstack/nova master: WIP: Trim the fat on HostState.instances https://review.openstack.org/569247
20:10:11 mriedem i thorougly enjoy what you have to say for this new config option if you enable it
20:10:15 mriedem ^
20:10:25 cdent striping across 500 volumes sounds kind of fun. Like: whatever they are doing, they must think it is pretty serious. Thus: fun.
20:11:21 dansmith more fun for the operator that gave a tenant 500 volume quota and realized they attached them all to a single instance to hack around performance limits
20:14:16 efried mriedem: Fast-approved
20:14:36 mriedem ha
20:17:09 tssurya mriedem: sorry was afk, but thanks for fixing the nits in both the patches
20:19:40 dansmith mriedem: you really want to provide a workaround knob for that?
20:20:36 mriedem dansmith: as in just let it break?
20:20:41 dansmith yeah
20:20:50 dansmith I mean, I appreciate the care, but..
20:21:00 mriedem i didn't see any HostInstance.instances usage in the huawei product code but...
20:21:17 dansmith also,
20:21:53 dansmith if we change how we do that query, we could populate the id field, which would make it actually lazy-load the things they want
20:21:54 dansmith right now it will fail with
20:21:57 dansmith "orphaned object" errors
20:22:14 dansmith or make instance lazy-load by uuid if it doesn'talready
20:22:39 artom Or just outright own the full thing and change the object to be a list of instance UUIDs?
20:22:52 artom Not sure how that would work for backwards compatibility tho
20:23:02 mriedem so first fellas, WIP clearly
20:23:04 dansmith oh it does load by uuid, so make the context right and it will lazy-load
20:23:16 dansmith mriedem: I know, I'm just asking :)
20:23:23 mriedem second, yeah long term i want to do away with the dict and just make HostState.instances a list of uuids
20:23:34 artom mriedem, too bad, you gave me an excuse to procrastinate real work. Fell the full wrath of my nitpicking.
20:23:42 mriedem dansmith: so {uuid: objects.Instance(cctxt, uuid=uuid)}?
20:23:49 dansmith mriedem: yup
20:23:51 mriedem i fell it real good
20:24:10 dansmith mriedem: then you can just not do the conf option, it'll be slow if they touch it, and we can remove it later
20:24:11 mriedem dansmith: yeah that's agood point
20:24:19 mriedem damn good point
20:24:53 dansmith mriedem: snarky comments still required though
20:25:05 dansmith # NOTE(mriedem): Set the timer on this bomb and walk out casually
20:31:35 mriedem dansmith: setting the context on the instance object won't lazy-load all fields
20:31:40 mriedem just those in INSTANCE_OPTIONAL_ATTRS
20:31:54 dansmith should call _load_generic, which will do anything
20:32:03 mriedem if attrname not in INSTANCE_OPTIONAL_ATTRS:
20:32:08 mriedem raise exception.ObjectActionError(
20:32:17 dansmith oh, heh,
20:32:31 dansmith I dunno why we did that
20:32:33 mriedem so now my filter on instance.default_ephemeral_device no longer works
20:32:34 dansmith because load_generic will do it
20:33:30 dansmith it's from 2013, nuke it
20:34:33 dansmith it looks like that was just trying to make it not do an empty load for something it wasn't going to be able to satisfy,
20:34:44 dansmith since it was trying to figure out what to put in extra_attrs
20:35:36 dansmith and that was before we had load_generic
20:35:52 dansmith so when we added the generic thing we probably just didn't remove the thing at the top to let you use it for anything
20:36:05 dansmith we should also have that fill in any field it gets that isn't already set,
20:36:15 dansmith so you don't load the instance twice for two regular fields
20:36:20 dansmith I can make those changes in one patch if you want
20:36:40 mriedem "we should also have that fill in any field it gets that isn't already set," - i believe it will do that for all non-joined fields anyway
20:36:49 mriedem because it just does a normal get and load from db
20:36:56 dansmith load_generic won't,
20:37:02 dansmith it will only copy over the one thing you asked for
20:37:29 mriedem self[attrname] = instance[attrname]
20:37:29 mriedem if instance.obj_attr_is_set(attrname):
20:37:29 mriedem oh i see
20:37:33 dansmith yeah
20:37:33 openstackgerrit Merged openstack/nova master: Cleanup ugly stub in TestLocalDeleteAllocations https://review.openstack.org/561997
20:37:35 dansmith I mean,
20:37:41 dansmith it nicely punishes them hard,
20:37:45 dansmith but probably not the best thing to do :P
20:38:03 mriedem i just sent to the ML btw
20:38:34 mriedem efried: http://lists.openstack.org/pipermail/openstack-dev/2018-May/130624.html
20:38:47 efried mriedem: ack
20:39:25 mriedem also related, we probably no longer need to default join on security_groups since that would only ever be populated for nova-network
20:40:43 mriedem dansmith: if you want, what i do need right now is to handle the NOTE in InstanceList.v
20:40:44 mriedem *get_uuids_by_host
20:41:28 dansmith hah
20:41:33 dansmith yeah I can do that too
20:41:54 dansmith there has to be some animated gif that applies well to that
20:42:15 mriedem i've been meaning to do that since https://review.openstack.org/#/c/563234/1/nova/api/openstack/compute/services.py@222
20:42:42 dansmith ooh, yessss, I made unit tests fail with infinite recursion
20:47:10 dansmith heh, instance will trigger a double lazy load on itself apparently, which I uncovered by removing this check
20:47:13 dansmith that's nice
20:48:02 mriedem you also have a note in that exact code
20:48:05 mriedem about recursion
20:48:20 edmondsw any stable cores around to review https://review.openstack.org/#/c/567599 ?
20:48:33 dansmith I know
20:49:11 edmondsw been out there a while, and it's an easy review
20:56:26 openstackgerrit Merged openstack/nova master: Remove mox in test_xenapi.py (3) https://review.openstack.org/564645
21:11:53 dansmith mriedem: funny thing happened on the way to the forum: https://github.com/openstack/nova/blob/master/nova/db/sqlalchemy/api.py#L2596-L2607
21:12:40 mriedem edmondsw: easy for me to -1
21:14:06 mriedem ha
21:14:13 edmondsw esberglu ^
21:14:27 mriedem dansmith: one difference with that,
21:14:33 mriedem the project_only=False kwarg to model_query
21:14:42 dansmith aww

Earlier   Later