| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-12-14 | |||
| 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 | |
| 16:25:46 | artom | Which... is it the case? | |
| 16:25:46 | dansmith | jaypipes: text isn't big enough | |
| 16:26:02 | artom | Surely some people are running Galera | |
| 16:26:05 | dansmith | artom: I think everyone would be running innodb in prod, AFAIK | |
| 16:26:07 | jaypipes | dansmith: TEXT > MEDIUMTEXT | |
| 16:26:21 | dansmith | jaypipes: um I don't think so | |
| 16:26:33 | jaypipes | sorry, yeah... | |
| 16:26:48 | dansmith | https://stackoverflow.com/questions/13932750/tinytext-text-mediumtext-and-longtext-maximum-storage-sizes | |
| 16:26:53 | jaypipes | was thinking LONGTEXT | |
| 16:27:00 | bauzas | yeah LONGTEXT no ? | |
| 16:27:05 | artom | Oh, Galera uses InnoDB | |
| 16:27:08 | bauzas | TEXT to LONGTEXT I mean | |
| 16:28:07 | jaypipes | dansmith, artom: what is the specific field we are talking about here? | |
| 16:28:10 | mriedem | dansmith: jaypipes: cool - thanks | |
| 16:28:13 | jaypipes | dansmith: is this metadata value? | |
| 16:28:24 | dansmith | jaypipes: request_specs.spec | |
| 16:28:29 | jaypipes | hmm. | |
| 16:28:40 | dansmith | jaypipes: it's a text we serialize the object into, and because we store instance_group.members, which can be huge, | |