Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2017-12-14
16:01:41 mriedem it's request spec and groups and scheduler stuff,
16:01:43 artom dansmith, you know objects better than I do, other than _from_db_object we don't generate it in any other way?
16:01:46 mriedem there is totally a way to fuck this up
16:01:48 mriedem w/o knowing it
16:02:04 artom mriedem, oh I agree
16:02:10 artom Which is why I'm thinking about it :)
16:02:13 bauzas mriedem: dansmith: FWIW, just trying to look at earlier code
16:02:20 dansmith mriedem: nobody asserted otherwise I don't think :)
16:02:23 openstackgerrit Stephen Finucane proposed openstack/nova master: SchedulerReportClient._get_providers_in_aggregates https://review.openstack.org/521097
16:02:24 openstackgerrit Stephen Finucane proposed openstack/nova master: Aggregate ops on ProviderTree https://review.openstack.org/526216
16:02:24 openstackgerrit Stephen Finucane proposed openstack/nova master: Traits ops on ProviderTree https://review.openstack.org/521605
16:02:25 openstackgerrit Stephen Finucane proposed openstack/nova master: Track provider traits in report client https://review.openstack.org/521686
16:02:25 openstackgerrit Stephen Finucane proposed openstack/nova master: Move aggregates from report client to ProviderTree https://review.openstack.org/521685
16:02:26 openstackgerrit Stephen Finucane proposed openstack/nova master: Raise on API errors getting aggregates/traits https://review.openstack.org/526540
16:02:26 openstackgerrit Stephen Finucane proposed openstack/nova master: Track associated sharing RPs in report client https://review.openstack.org/526539
16:02:27 openstackgerrit Stephen Finucane proposed openstack/nova master: Track tree-associated providers in report client https://review.openstack.org/526541
16:02:27 openstackgerrit Stephen Finucane proposed openstack/nova master: ProviderTree.populate_from_iterable https://review.openstack.org/520756
16:02:28 openstackgerrit Stephen Finucane proposed openstack/nova master: WIP: ComputeDriver.update_provider_tree() https://review.openstack.org/521187
16:02:28 openstackgerrit Stephen Finucane proposed openstack/nova master: WIP: Scheduler[Report]Client.get_provider_tree https://review.openstack.org/521098
16:02:29 openstackgerrit Stephen Finucane proposed openstack/nova master: WIP: Use update_provider_tree from resource tracker https://review.openstack.org/520246
16:02:41 mriedem dansmith: maybe i misinterpreted "i think it basically boils down to (1) foundatoin doesn't want to pay or plan 2 ptgs, (2) people don't want to upgrade, or feel the pressure to upgrade, every 6 months and (3) people that are now going to multiple events (kubecon+) don't want to feel stretched"
16:02:44 mriedem oops
16:02:50 mriedem "artom: I suppose there's no way for a request_spec to end up with an empty group in code that expects something there"
16:03:07 artom mriedem, yeah, "suppose" being the key word there ;)
16:03:20 artom So, what if we move the loading by uuid into instance group?
16:03:31 artom That way there's definitely no way to end up with an empty one
16:03:32 dansmith mriedem: I think that just meant group and members is not always present, can be null, nothing is depending on that being set to something specific that we might mess up
16:03:34 dansmith which is true I think
16:03:50 bauzas dansmith: oh shit, 'members' is not lazy-loadable, 'hosts' is
16:03:51 artom dansmith, yep, you understood that
16:04:00 bauzas hence my confusion
16:04:22 bauzas dansmith: and since we persist the group object in the DB, yeah I understand the issue
16:04:40 dansmith artom: well, not sure what that matters, since you didn't snip it out before we wrote it into reqspec anyway
16:04:41 dansmith but yeah
16:04:43 dansmith er
16:04:47 dansmith that was for bauzas:
16:05:09 openstackgerrit Stephen Finucane proposed openstack/nova master: ProviderTree.populate_from_iterable https://review.openstack.org/520756
16:05:09 openstackgerrit Stephen Finucane proposed openstack/nova master: Raise on API errors getting aggregates/traits https://review.openstack.org/526540
16:05:10 openstackgerrit Stephen Finucane proposed openstack/nova master: WIP: Scheduler[Report]Client.get_provider_tree https://review.openstack.org/521098
16:05:10 openstackgerrit Stephen Finucane proposed openstack/nova master: Track tree-associated providers in report client https://review.openstack.org/526541
16:05:11 openstackgerrit Stephen Finucane proposed openstack/nova master: WIP: Use update_provider_tree from resource tracker https://review.openstack.org/520246
16:05:11 openstackgerrit Stephen Finucane proposed openstack/nova master: WIP: ComputeDriver.update_provider_tree() https://review.openstack.org/521187
16:06:20 artom dansmith, so what if we move the loading by uuid down to instance_group? That way we make sure we can never be in a situation with empty members in code that expects them...
16:06:23 artom Right?
16:06:46 dansmith artom: meaning make it lazy-loadable?
16:07:20 artom dansmith, would it make it lazy-loadable? Not necessarily...
16:07:30 artom Actually
16:07:32 artom Yeah it would
16:08:04 dansmith artom: so that won't work if we end up lazy-loading on the compute, because it'll be in a cell and can't get that info later
16:08:10 dansmith artom: we need to pre-load it
16:08:36 dansmith which is kinda the point of reqspec: to pass along everything the stuff downstream needs in order to move the instance
16:09:07 artom Oh right, cells
16:10:05 cfriesen dansmith: would your proposed change to update the instance_group when reading the request spec from DB cover the live-migration case as well? We had a local bugfix to cover that case as part of various affinity/antiaffinity race fixes that haven't made it upstream.
16:10:50 dansmith cfriesen: if I know what you're asking, then it should yeah
16:11:14 dansmith cfriesen: when we load the reqspec in the api before kicking off the live migration, it will have a fresh and accurate set of group members passed down to anything that gets the reqspec in the process
16:12:38 cfriesen dansmith: I'm looking at _find_destination() in live_migrate.py. It calls request_spec = self._get_request_spec_for_select_destinations()
16:15:53 cfriesen dansmith: okay, so looks like that request spec is normally pulled from the DB in compute.api.API.live_migrate(), so we should be good.
16:16:25 dansmith cfriesen: it kinda has to be since it's in the api db
16:17:42 bauzas dansmith: +2 on the fix
16:18:31 artom Semi-related thought
16:18:54 artom Will the code actually fix existing cases?
16:19:09 dansmith artom: it will, because:
16:19:12 artom Ah, yeah, it sets members to None when saving
16:19:12 bauzas artom: you mean upstream bugs :p
16:19:23 dansmith artom: if you are under the limit now, then you are good.. if you're over, this will make you fit now
16:19:35 artom bauzas, no, I meant our downstream bug
16:19:36 bauzas exactly
16:19:41 dansmith artom: and, the api will immediately stop reading the stale group after this fix, for reqspecs with stale data from before the fix
16:19:46 bauzas artom: I do understand but we're upstream :)
16:20:16 artom bauzas, well, yes, but it's still relevant to talk about whether we're actually fixing the existing problem, or just preventing it in the future
16:20:24 bauzas it'll pull fresh data indeed
16:20:39 dansmith that's why I did it that way
16:20:46 jaypipes cfriesen, artom: TEXT/MEDIUMTEXT/VARCHAR fields are only pre-allocated at full size when the field is used in a GROUP BY or ORDER BY expression. Otherwise, only a pointer to the data in the data file is used during the projection phase of the SELECT expression (when the rows are actually sent to the client)
16:20:51 dansmith because we have to account for all the broken data in the db now
16:21:00 jaypipes cfriesen, artom: this is for MySQL, not PostgreSQL
16:21:04 bauzas dansmith: and you did it the right way, thanks :)
16:21:17 dansmith jaypipes: we're talking about allocation on disk, not memory
16:21:39 dansmith jaypipes: like whether 1 byte in a MEDIUMTEXT takes up 16mb on disk
16:22:04 jaypipes dansmith: there's almost no difference between a TEXT, MEDIUMTEXT or VARCHAR(8000) w.r.t. on-disk storage requirements or layout.
16:22:11 jaypipes dansmith: for InnoDB that is.
16:22:12 bauzas dansmith: FWIW, I fixed the problem way in the past for hosts https://github.com/openstack/nova/commit/c48c1098cbcd7a9c7980d5fbe4668c38b16767f5
16:22:31 dansmith jaypipes: okay
16:22:44 bauzas dansmith: but I didn't thought it would be a huge problem for members, given not a lot of operators use a long list of them
16:23:04 jaypipes dansmith: no, for all those field types, InnoDB stores a small amount of data in the data page itself and makes room (when needed) in separate extents by providing a pointer to that extent/page from the main data page.
16:23:43 jaypipes dansmith: the only thing that can be performance-wise a bad thing is if the size of those fields changes often (which isn't the case for these fields in Nova's DB)
16:23:44 artom So for MySQL/InnoDB a migration to mediumtext would actually make sense...
16:23:55 jaypipes artom: for InnoDB, yes. for NDB, no...
16:24:07 jaypipes artom: but that's a different problem entirely :)
16:24:24 artom Do we support NDB?
16:24:40 jaypipes artom: what is the current column type for this field you're talking about?
16:24:48 bauzas mriedem: jaypipes: https://review.openstack.org/#/c/527799/ looks legit to me
16:24:52 dansmith jaypipes: and what is the cost of resizing to medium from text?
16:25:01 jaypipes dansmith: virtually zero.
16:25:04 artom jaypipes, dansmith answered that for me
16:25:12 dansmith jaypipes: awesome, then we should do that too
16:25:19 dansmith artom, mriedem ^
16:25:33 dansmith still need the real fix, but sounds like a backport is too good to pass up
16:25:36 dansmith of the migration I mean
16:25:37 artom dansmith, that assumes we only support InnoDB
16:25:38 jaypipes dansmith: I don't recommend mediumtext anyway. just set it to TEXT

Earlier   Later