| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-10-03 | |||
| 17:21:27 | melwitt | I'd think we'd need a deprecation cycle for changing nova-manage syntax. is that clock already in motion? | |
| 17:22:41 | mriedem | sdague: cdent: this needs some more thought https://review.openstack.org/#/c/507762/ | |
| 17:22:45 | mriedem | plus, it came up in newton | |
| 17:22:51 | mriedem | and there are some issues to overcome | |
| 17:22:52 | clarkb | as a drive by comment, the use of stevedore (via cliff aiui) in osc is what makes it so painfully slow to use for commands | |
| 17:23:07 | clarkb | so please don't use entrypoints for cli commands | |
| 17:23:21 | melwitt | good to know | |
| 17:23:21 | mriedem | dansmith: you too on https://review.openstack.org/#/c/507762/ because it would require paging instance actions across cells... | |
| 17:23:23 | mriedem | which we know is fun | |
| 17:23:43 | mriedem | melwitt: there were some deprecations made in pike | |
| 17:23:48 | mriedem | to prep for this in queens | |
| 17:23:49 | cdent | ah, fart, forgot about cross cell biz | |
| 17:24:05 | mriedem | it's not only that - it's that we literally don't update the updated_at column for intsance actions | |
| 17:24:11 | mriedem | so filtering on changes-since for that is kind of dumb | |
| 17:24:19 | dansmith | mriedem: ack will look | |
| 17:24:22 | sdague | clarkb: it's not going to be entry points | |
| 17:24:24 | mriedem | like i said, it's come up before | |
| 17:24:33 | cdent | mriedem, why is that not a bug? (as in, to fix) | |
| 17:24:44 | sdague | clarkb: it's just going to be non custom cli parsing | |
| 17:24:52 | sdague | clarkb: there is nothing about stevedore here | |
| 17:24:59 | mriedem | Kevin_Zheng is on holiday this week but i'll also bug him on the wechat-o-sphere | |
| 17:25:40 | dansmith | mriedem: instance action list is per instance, right? so no paging across cells? | |
| 17:26:03 | clarkb | sdague: I thought cliff had some baked in ideas of registering commands via stevedore as part of its command parsing | |
| 17:26:10 | clarkb | sdague: but maybe its optional | |
| 17:27:18 | cdent | mriedem: “ it's that we literally don't update the updated_at column for intsance actions” <- Is there a reason why for that? Isn’t that what updated_at means? | |
| 17:28:13 | melwitt | I didn't think we updated individual instance action records. aren't they just written once? | |
| 17:28:50 | melwitt | like, "instance created" "instance rebooted" "instance rebuilt" it's not like you go back and edit those | |
| 17:30:03 | dansmith | updated_at is built into oslo.db | |
| 17:30:14 | dansmith | so if it's never set on those, it's because we never did a save, AFAIK | |
| 17:30:35 | dansmith | there are other things missing in that spec, | |
| 17:30:44 | dansmith | like it doesn't show them actually using the marker they say they're going to, | |
| 17:31:06 | dansmith | but I assume they really mean start_time, which is its own field | |
| 17:31:11 | dansmith | separate from the usual three default ones | |
| 17:31:31 | dansmith | there is a finish_time on that object too | |
| 17:31:37 | melwitt | yeah, I was trying to say I don't think instance action rows are ever saved, they're created/written once and that's it | |
| 17:31:48 | sdague | melwitt: how is end time set then? | |
| 17:32:08 | sdague | or, only if it's set all at once | |
| 17:32:25 | dansmith | we can update them actually | |
| 17:32:34 | dansmith | action_event_finish() | |
| 17:32:47 | dansmith | sets the finish time and calls action.update() | |
| 17:32:54 | melwitt | oh, weird | |
| 17:33:15 | dansmith | I'd bet we don't finish all the actions we start though.. like everything else:P | |
| 17:38:56 | mriedem | i'd have to read back through the specs, but we create the action record and then we just deal with the events | |
| 17:38:59 | mriedem | which are different records | |
| 17:39:15 | mriedem | so there is event_start and event_finish | |
| 17:40:35 | mriedem | and i don't see us ever updating the action record when the event is finished | |
| 17:41:31 | dansmith | mriedem: https://github.com/openstack/nova/blob/master/nova/db/sqlalchemy/api.py#L6224 | |
| 17:41:48 | dansmith | ohh, the action and event I'm getting confused I guess | |
| 17:42:07 | dansmith | we actually just return the action from the action_finish() thing | |
| 17:42:17 | dansmith | not sure why or how that makes sense | |
| 17:42:25 | dansmith | but I guess that means we're never updating the _action_ | |
| 17:42:40 | mriedem | correct | |
| 17:42:46 | dansmith | it doesn't matter though, because changes-since should key on the start_time I would think | |
| 17:43:25 | dansmith | or it could be on max(start_time, finish_time) in case we ever set finish_time | |
| 17:43:30 | dansmith | but not updated_at I wouldn't think | |
| 17:43:37 | melwitt | I think the question was, why isn't updated_at updated and I was saying because it's never updated | |
| 17:44:13 | cdent | tautology alert | |
| 17:44:19 | cdent | but: yes | |
| 17:44:24 | melwitt | haha yeah | |
| 17:44:26 | dansmith | melwitt: right, I get that, and it's because we're not updating it from action_finish | |
| 17:44:43 | dansmith | melwitt: I had found action_event_finish() when looking for action_finish() which _does_ | |
| 17:44:51 | melwitt | yeah | |
| 17:44:58 | dansmith | I dunno _why_ we're not updating it during the finish, but.. | |
| 17:45:22 | dansmith | regardless, I would expect changes-since on this kind of thing to use the inbuilt fields | |
| 17:45:22 | melwitt | well, because each action is just "instance create started" "instance create finished" and they're separate right? | |
| 17:45:27 | dansmith | not the default one | |
| 17:45:33 | melwitt | there's nothing to update about it, it's just there | |
| 17:45:48 | dansmith | melwitt: except we have a finish method that doesn't finish it, and a finish_time field we never set, apparently | |
| 17:45:57 | mriedem | dansmith: i don't think anything actually calls action_event_finish() | |
| 17:46:06 | mriedem | i remember talking to laski about that at one point | |
| 17:46:14 | melwitt | oh | |
| 17:46:15 | mriedem | and it was like, something something tasks...we never used it | |
| 17:46:22 | dansmith | fine, extend my argument to why we have those in addition to why they do nothing :) | |
| 17:46:35 | mriedem | the EventReporter context manager calls objects.InstanceActionEvent.event_finish_with_failure | |
| 17:46:38 | dansmith | it's beside the point though, IMHO | |
| 17:47:27 | melwitt | I think I don't know what instance action events are. I'm just thinking of the "instance actions" which are a basic log of things that have happened to the instance | |
| 17:47:56 | mriedem | the events are tied to the action | |
| 17:48:06 | dansmith | right, an action is a high level thing, | |
| 17:48:07 | mriedem | see the EventReporter class | |
| 17:48:09 | dansmith | events are small steps | |
| 17:48:21 | mriedem | ever notice the @wrap_instance_event decorator on the compute manager methods? | |
| 17:48:33 | mriedem | that creates the event start on entry, and event finish on exit | |
| 17:48:55 | mriedem | the action record is created in the API before casting to compute where the real business happens, and that real bidness is recorded with events | |
| 17:48:57 | mriedem | on the action | |
| 17:50:06 | melwitt | bidness | |
| 17:51:04 | melwitt | okay, so I guess someone forgot to ever finish_time instance actions after creating the data model | |
| 17:51:28 | mriedem | jesus i thought this was merged in pike https://review.openstack.org/#/c/480746/ | |
| 17:51:44 | mriedem | so ^ gives an example of how a user can use these things for polling when an operation is complete | |
| 17:52:24 | melwitt | that's leet | |
| 17:53:48 | mriedem | leet? | |
| 17:55:06 | mriedem | also, note that if we ever rename methods in the compute manager which have the @wrap_instance_event decorator, we break that api | |
| 17:55:14 | dansmith | which we have, I think | |
| 17:55:17 | melwitt | like elite use of that API | |
| 17:55:19 | dansmith | hasn't gibi complained? | |
| 17:55:26 | mriedem | don't remember | |
| 17:55:41 | dansmith | I think when we moved things from compute to conductor there was a thing about that | |
| 17:56:03 | dansmith | I don't recall if there was an override to fix it or just "meh, this is an internal api" | |
| 17:56:05 | mriedem | for events or notifications? | |
| 17:56:22 | dansmith | oh yeah I guess I'm thinking of notifications | |
| 17:56:30 | dansmith | but similar deal | |