| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-12-14 | |||
| 15:32:37 | mriedem | we will be supporting turds for longer, and hamstrung by said turds potentially | |
| 15:32:47 | mriedem | i'm glad we said rocky is the target to drop nova-net and cellsv1 | |
| 15:33:58 | cdent | the bits with "support both ways of doing this thing" have to stay around longer too, which is icky | |
| 15:34:09 | cdent | but then again, apparently the style is something the ff-upgrade people don't like? | |
| 15:39:18 | dansmith | mriedem: so my goal on that reqspec fix is to try to converse with bauzas about it today to make sure he's okay with it and/or there's not something huge we're missing on it | |
| 15:39:47 | mriedem | ack | |
| 15:39:49 | dansmith | mriedem: and then hand over the patch and backports to artom, who unfortunately is the recipient of many such turdish patches of mine right before I leave on vacay :P | |
| 15:40:12 | artom | Seriously, I'm like dansmith's crap scapegaot | |
| 15:40:17 | artom | Crapgoat | |
| 15:40:19 | mriedem | dansmith: i'm personally not comfortable with that one going to newton right before we eol | |
| 15:40:24 | dansmith | artom: as you will henceforth be known. | |
| 15:40:26 | mriedem | i think the column alter is fine | |
| 15:40:27 | mriedem | for newton | |
| 15:40:42 | mriedem | i'm worried too much about reqspec/group side effects | |
| 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 | |