Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2017-12-14
15:40:50 dansmith mriedem: well, I'd rather just not backport any of it to newton myself,
15:41:04 dansmith mriedem: and don't really want to backport the migration at all, but that's fine
15:41:14 dansmith we will, of course, but newton needs to go at some point
15:41:24 artom I agree that the migration might have more side effects than the load by UUID patch
15:41:25 mriedem i know but i love it so
15:41:27 artom Er, less
15:41:27 dansmith and this is something that has been this way since the release and which we've only _just_ heard about
15:41:44 artom So if we're going to backport something, it might actually be safer to do just the migration
15:41:44 mriedem yeah, because people are just now upgraded
15:41:46 mriedem *upgrading
15:42:02 mriedem well, people with big ass server groups apparently :)
15:42:14 artom Also good point - there's like a 1-2 year lag between upstream and what's actually running
15:42:36 dansmith well, there are definitely people already on newton and beyond, but I understand that reasoning
15:43:02 dansmith mriedem: fwiw, there were only four such instances in a database of tens of thousands that were affected here
15:43:15 dansmith mriedem: and it seemed like it was a very specific case, which might have been testing and/or a script run amok
15:43:26 dansmith just for the data point
15:43:43 mriedem ok
15:44:10 dansmith the fact that we're crystallizing known-bad server groups in the database at creation time is almost more concerning to me than the overrun case
15:44:48 artom Known-bad?
15:46:18 dansmith artom: yeah, so we're capturing the server group (and specifically its members) at the time in which we create/write the reqspec for an instance
15:46:44 mriedem i'm fully on board with fixing that
15:46:48 dansmith so lets say you do this: nova boot --group foo; nova boot --group foo
15:46:49 artom Ah, and some of those can get deleted, so we're left with a group with non existing members
15:46:49 dansmith the first one has an InstanceGroup with .members=[justme]
15:46:54 dansmith the second one is complete
15:47:06 dansmith there is no reason we should be persisting that and then ever using it ever, but we are
15:47:25 dansmith artom: either stale members, or incomplete, depending on when you snapshotted it
15:47:55 artom I'm trying to think if anyone could be relying of this behaviour for whatever reason
15:48:41 artom This is used in stuff like the group (anti)affinity filters, right?
15:48:43 cfriesen dansmith: mriedem: for what it's worth, for reliability you have to update the server group members before rescheduling anyways, otherwise you're working off stale data
15:48:57 gibi mriedem: could you check back to https://review.openstack.org/#/c/527444/ I start seeing the related bug appearing more and more on the gate
15:48:59 dansmith cfriesen: right
15:49:16 mriedem gibi: yeah
15:49:32 gibi mriedem: thanks
15:51:12 bauzas dansmith: hola, what's the point about the request spec ?
15:51:19 artom So are we coming to the consensus that we fix the stale group membership in master, but to play it safe only backport the db migration?
15:51:49 dansmith bauzas: we're persisting InstanceGroup in RequestSpec with stale members for some reason.. why is that?
15:51:55 artom Though I suppose it's conceivable the db migration might not fix stuff either, if mediumtext is too small still
15:52:08 dansmith bauzas: this came because someone with a very large number of members in that group (at the time of that snapshot) can overrun a TEXT field
15:52:17 dansmith artom: mediumtext is very large I think
15:52:20 dansmith like 16mb or something
15:52:34 artom Seriously?
15:52:40 dansmith yup
15:52:50 artom Wait, so if we migrate a DB with like, thousands of rows, we're adding how much space requirement?
15:52:57 artom And how long will such a thing take?
15:53:12 dansmith artom: hence me not wanting to do it unnecessarily :)
15:53:16 dansmith we rewrite that whole table
15:53:21 bauzas dansmith: mmmmm
15:53:36 bauzas dansmith: AFAIK, we lazy-load the members group
15:53:44 bauzas but I could be wrong
15:53:46 cfriesen postgres has unlimited text fields.
15:53:50 dansmith bauzas: no, we write them all into requestspec
15:53:53 artom Yeha, so my SQL knowledge is fuzzy, but if we increase it to 16 MB we're essentially pre-allocating 16 MB for every row, correct?
15:53:57 dansmith bauzas: and explode the database
15:54:02 dansmith artom: yeah
15:54:15 dansmith artom: which is why I'd rather fix the problem, because 64kb should be enough I think
15:54:16 bauzas seriously? :(
15:54:27 dansmith bauzas: https://review.openstack.org/#/c/527799/3
15:54:54 bauzas looking
15:56:46 cfriesen dansmith: I'm seeing docs saying that text colums are stored separately from the row, so would it really preallocate 16MB for each row?
15:56:56 cfriesen (for mysql)
15:57:23 dansmith cfriesen: oh? I didn't think that's how it worked
15:57:56 cfriesen I'm *not* a mysql guy, so do your own research
15:58:36 artom Where's Monty when you need him
15:58:46 dansmith it doesn't matter that much anyway, as I think fixing the problem will fix the problem
15:58:55 dansmith artom: jaypipes knows a thing or two :)
15:59:08 dansmith it only adds fuel to the discussion though, we need to fix the real problem
15:59:36 artom I think we're just afraid fixing the problem might create another one
15:59:45 artom Like sending rebuild to the scheduler :)
15:59:46 cfriesen dansmith: agreed...I don't see any reason to persist the group members at time of booting
16:00:22 dansmith artom: well, that's why I want bauzas to look at it closely, but also, I think my fix is pretty straightforward
16:00:28 artom dansmith, yeah
16:00:42 cfriesen artom: you need to check the group members right before scheduling.
16:00:45 dansmith it didn't even require functional test change
16:01:06 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:01:23 mriedem artom: umm
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: Traits ops on ProviderTree https://review.openstack.org/521605
16:02:24 openstackgerrit Stephen Finucane proposed openstack/nova master: Aggregate ops on ProviderTree https://review.openstack.org/526216
16:02:25 openstackgerrit Stephen Finucane proposed openstack/nova master: Move aggregates from report client to ProviderTree https://review.openstack.org/521685
16:02:25 openstackgerrit Stephen Finucane proposed openstack/nova master: Track provider traits in report client https://review.openstack.org/521686
16:02:26 openstackgerrit Stephen Finucane proposed openstack/nova master: Track associated sharing RPs in report client https://review.openstack.org/526539
16:02:26 openstackgerrit Stephen Finucane proposed openstack/nova master: Raise on API errors getting aggregates/traits https://review.openstack.org/526540
16:02:27 openstackgerrit Stephen Finucane proposed openstack/nova master: ProviderTree.populate_from_iterable https://review.openstack.org/520756
16:02:27 openstackgerrit Stephen Finucane proposed openstack/nova master: Track tree-associated providers in report client https://review.openstack.org/526541
16:02:28 openstackgerrit Stephen Finucane proposed openstack/nova master: WIP: Scheduler[Report]Client.get_provider_tree https://review.openstack.org/521098
16:02:28 openstackgerrit Stephen Finucane proposed openstack/nova master: WIP: ComputeDriver.update_provider_tree() https://review.openstack.org/521187
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 ;)

Earlier   Later