| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2020-04-28 | |||
| 16:01:48 | zzzeek | stephenfin: both contains() / startswith() are based on LIKE which is going to need a table scan | |
| 16:02:00 | artom | zzzeek, and more generally, if we're stuck doing text filtering on a column without an index, what would be the lest sucky way? | |
| 16:02:06 | artom | *least | |
| 16:02:18 | bauzas | gmann: I briefly looked at the note | |
| 16:02:25 | zzzeek | I don't see the db/model files here that we're talking about I just see objects files, is there a SQL query in those ? | |
| 16:02:42 | zzzeek | and is the issue that you don't wnat to add a new column with the indexed information you need ? | |
| 16:02:43 | bauzas | gmann: and unless I'm wrong, I don't see a technical problem for providing a nova-status check command verifying this | |
| 16:02:53 | stephenfin | zzzeek: sec, lemme drag up the model | |
| 16:03:04 | zzzeek | oh i see it | |
| 16:03:12 | zzzeek | .filter(~models.ComputeNode.numa_topology.startswith( | |
| 16:03:12 | zzzeek | compute_nodes = (context.session.query(models.ComputeNode) | |
| 16:03:12 | zzzeek | '{"nova_object.name"')) | |
| 16:03:12 | zzzeek | .filter(models.ComputeNode.numa_topology != null()) | |
| 16:03:16 | zzzeek | that ? yeah that's not great :) | |
| 16:03:22 | zzzeek | depends on number of rows | |
| 16:03:25 | zzzeek | < 1000, no problem | |
| 16:03:27 | bauzas | gmann: ie. look at the file, and if you find both old-style and new-style, yell it | |
| 16:03:30 | artom | zzzeek, well, all instances | |
| 16:03:44 | artom | So multiple thousands, maybe into the millions for large deployments | |
| 16:03:46 | zzzeek | including old ones that are offline ? | |
| 16:03:53 | zzzeek | artom: yeah that's not going to be fast | |
| 16:03:54 | stephenfin | artom: not deleted instances | |
| 16:04:06 | stephenfin | tbc | |
| 16:04:10 | zzzeek | you want to get the # of rows as low as possible before applying a like on it | |
| 16:04:14 | artom | stephenfin, does that filtering happen before or after the string filter? | |
| 16:04:31 | gmann | bauzas: ok, let me add in nova-status also | |
| 16:04:36 | bauzas | gmann: if you wanna some examples, go over there https://github.com/openstack/nova/blob/master/nova/cmd/status.py#L364 | |
| 16:04:38 | zzzeek | there are other ways to do this, mysql/mariadb might ahve a MATCH operator but that requires changes | |
| 16:04:51 | stephenfin | I actually don't know /o\ Does filter ordering matter? | |
| 16:05:16 | artom | zzzeek, yeah, IIUC MATCH needs full text index, no? | |
| 16:05:42 | zzzeek | but ideally you'd have the information you need captured in some dedicated column, or if for example the "numa_topology" were a forieng key to some collection table, you could limit the rows on that side to a list of posible foreign key ids, that kind of thing | |
| 16:05:56 | zzzeek | artom: yes | |
| 16:06:24 | gmann | bauzas: sure, thanks | |
| 16:06:24 | bauzas | anyway, /me calls it a day | |
| 16:07:08 | stephenfin | zzzeek: Unfortunately this is a JSON blob rather than an actual table. No foreign keys here | |
| 16:07:19 | zzzeek | so as far as LIKE '%foo%' vs. LIKE 'foo%', I don't know the details of what mysql/mariadb would do, we would imagine the second is "faster" but i dont know that it can use indexes for that | |
| 16:07:22 | stephenfin | and we're trying to move from one format of JSON blob to another | |
| 16:07:27 | artom | zzzeek, we can't play around with the schema for this, we need to work with the columns/indeces that we have | |
| 16:07:43 | artom | Or don't have :( | |
| 16:08:03 | zzzeek | stephenfin: Mysql /mariadb have JSON operators now, do those apply here? | |
| 16:08:37 | stephenfin | do those apply if the column type is just TEXT? | |
| 16:08:37 | zzzeek | e.g. you can extract values from a json structure | |
| 16:08:57 | zzzeek | stephenfin: yes there is no JSON type on both backends, there's ...some kind of keyword on one of htem | |
| 16:09:22 | dansmith | zzzeek: is that really faster than a string comparison? I would expect that would require reading the whole text, more cpu to parse, and then run the comparison | |
| 16:09:30 | zzzeek | mysql has a native JSON mariasdb does not but the json operators should work on either | |
| 16:09:41 | zzzeek | dansmith: I dunno, would need to read some docs | |
| 16:09:49 | dansmith | I would be absolutely blown away if so :) | |
| 16:10:04 | zzzeek | https://mariadb.com/kb/en/json_value/ | |
| 16:10:14 | artom | stephenfin, I don't support looking at updated_date is enough? | |
| 16:10:14 | zzzeek | dansmith: Postgresql JSON type probably builds some index | |
| 16:10:18 | artom | *suppose | |
| 16:10:19 | zzzeek | Mysqls' might as well | |
| 16:10:20 | artom | Or can help in any way? | |
| 16:10:22 | zzzeek | mariadb, not too sure | |
| 16:10:24 | dansmith | zzzeek: sure, if the column type is json then maybe | |
| 16:10:40 | stephenfin | artom: I considered that, but I don't know when someone upgraded | |
| 16:11:49 | stephenfin | so if ~in theory~ someone was on Juno for ages - say, until 2016, and eventually decided to upgrade, and are now upgrading that same cloud to Victoria | |
| 16:11:59 | artom | stephenfin, actually - what about a two step approach? 1. the update on read thing we talked about earlier 2. let it sit for a cycle or two, then do a query with udpated_at earlier than time spent sitting | |
| 16:12:07 | zzzeek | dansmith: oh here's an intersting approahc, uisng a generated column to index parts of the json document when they are inserted | |
| 16:12:11 | zzzeek | https://www.compose.com/articles/mysql-for-json-generated-columns-and-indexing/ | |
| 16:12:23 | dansmith | zzzeek: that doesn't help this case | |
| 16:12:25 | stephenfin | and are moving the instance for the first time since before that upgrade off of Juno :) | |
| 16:12:33 | zzzeek | dansmith: brcause....table cannot be changed, right? | |
| 16:12:44 | dansmith | zzzeek: we're specifically looking at old data yeahg | |
| 16:12:58 | zzzeek | dansmith: old data can be copied into...new tables and columns? | |
| 16:13:11 | dansmith | zzzeek: no, we might as well convert it if we're going to read it | |
| 16:13:37 | zzzeek | dansmith: yup. if you have a big old JSON blob and you want to pull things out of it and performance is an issue then you would need to put this data into some indexable format somewhere else | |
| 16:14:01 | zzzeek | stephenfin: ^^^ | |
| 16:14:58 | dansmith | artom: updated_at is on the instance_extra row, right? so it doesn't really mean we've updated any one or all columns necessarily | |
| 16:15:05 | stephenfin | zzzeek: okay, thanks for the input :) | |
| 16:16:01 | artom | dansmith, hrmm, yeah - I guess I was assuming "update on read" would trigger for any read of any column of that row | |
| 16:16:19 | artom | Not just instance_extra.numa_topology reads | |
| 16:16:39 | dansmith | instance_extra was really supposed to be individually queried and updated, although I dunno how much that really happens.. it was mostly for lazy-loadable blobby things originally | |
| 16:17:26 | artom | Is this is the first time we're hitting this problem? | |
| 16:17:59 | artom | Apparently so - at least using .contains() | |
| 16:29:50 | zigo | If you guys think we don't have enough time before the final release (which I can agree on), why not just delay this for 21.0.1 or something? | |
| 16:30:25 | zigo | My biggest concern is not being able to actually "see" what's set by what we have in defaults. | |
| 16:30:34 | zigo | Yes, it's documented, but it's not the same. | |
| 16:30:43 | zigo | I'll try using policy.yaml though ... :P | |
| 16:30:56 | dansmith | zigo: delay "this" meaning the policy stuff in general? | |
| 16:31:03 | zigo | Yeah. | |
| 16:31:37 | dansmith | zigo: it was a large number of patches.. reverting it now would be .. huge. | |
| 16:32:31 | zigo | dansmith: I thought it'd be just a simple patch to the oslo-policy-sample-generator ... :P | |
| 16:32:36 | zigo | Maybe I'm being naive. | |
| 16:35:00 | dansmith | zigo: the change in question was on the nova side, not oslo.. we'd need new code on the oslo side to allow generating the policy file with the now-deprecated original defaults | |
| 16:37:01 | zigo | Now I see what you guys were talking about for the yaml thing: it's by default generated with everything commented out ... | |
| 16:37:11 | zigo | (I just tried...) | |
| 16:37:38 | zigo | That's probably nicer indeed, and probably good enough for operators, and also maybe more easy to have a policy.d | |
| 16:37:50 | zigo | Though the last time I tried, it wouldn't load ... :/ | |
| 16:41:16 | zigo | Looks like it works as one would expect... :) | |
| 16:41:45 | jsuchome | dansmith: ok, so it seems it's up to me, so if you could reopen and reassign it ... thanks | |
| 16:42:04 | dansmith | jsuchome: all you need to do is propose the changes yourself | |
| 16:43:00 | jsuchome | so should I just cherry pick to my branch? | |
| 16:44:21 | zigo | All this would be great if the Debian infrastructure wasn't completely down today ... :/ | |
| 16:44:45 | dansmith | jsuchome: just grab the latest version of that patch into your tree, make changes, git review. | |
| 17:33:28 | artom | dansmith, answers provided: https://review.opendev.org/#/c/672595/73 | |
| 17:33:34 | artom | (Hopefully) | |
| 17:33:43 | gmann | zigo: nice. +1 thanks for checking. | |
| 17:43:57 | artom | \o/ | |
| 17:44:14 | artom | I have a 1:1 in 15 minutes, otherwise it'd be beer time | |