Update events that are used to determine VM session starts and stops #732
Conversation
| "instance" | ||
| "instance", | ||
| "start_time", | ||
| "end_time" |
There was a problem hiding this comment.
I.e. why were both start and end added? Surely the start_time is sufficient to generate uniqueness. If you include the end_time does this have a problem when the data are aggregated tomorrow (since the end time of an instance may have changed - but not the start time).
There was a problem hiding this comment.
I note that the cloud transient action that populates this table uses "resource_id ASC, instance_id ASC, start_time ASC" as the order by clause.
There was a problem hiding this comment.
After thinking about it, I think that just using start time would be enough but as it is I don't think there will be an issue when you run aggregation the next day. The cloud_events_transient table is truncated every time aggregation is run so if you have a different end time for a session than the day before you will only get one row for that session with the new end time not two rows with the same start time but different end times.
I have no real strong feelings about this either way since I think both will work fine. If someone wants me to change it to just start_time I can do that.
There was a problem hiding this comment.
I thought the plan was to update the code to not truncate the tables every day. If you add the end_time in then it will just have to be removed again soon.
There was a problem hiding this comment.
I know there was talk of making it so that you don't have to re-aggregate all the cloud data every time you run aggregation which would mean that the cloudfact tables would no longer be truncated everyday. I'm not sure what affect that would have on the cloud_events_transient table. If it's the case that it would no longer be truncated every time aggregation is run then the end_time would need to be removed.
There was a problem hiding this comment.
Of course the cloud_events_transient table would have to not be truncated - this table stores the facts. What design did you have in mind where the cloud_events_transient table could be truncated every day?
There was a problem hiding this comment.
I don't have a design in mind. I don't think I'm the one that is going to make this change so I haven't really thought about it which is why I said I wasn't sure how any changes to how the aggregation is done would affect the cloud_events_transient table. Since it seems that we are going to change how the aggregation happens and that means the cloud_events_transient will no longer be truncated then I can remove the end_time.
…-start-stop-events
…d taking account new mappings for cloud events
| EOT | ||
| ); | ||
| $runaggregation = $console->promptBool( | ||
| 'Do you want to run cloud aggregation now?', |
There was a problem hiding this comment.
We should do some basic timing to allow them to know how long this might take.
This also runs ingestion.
We should check if the cloud is enabled before we bother prompting (already talked about that this ability is in another PR, so this can get updated at that point for all the things...)
| $cloudLogDirectoryEntryAttempts++; | ||
| } | ||
|
|
||
| $cloudResourceName = $console->prompt( |
There was a problem hiding this comment.
Does this handle multiple cloud types?
Generic and openStack?
…-start-stop-events
…-start-stop-events
…nstead of trying to do it for them
| @@ -353,7 +353,7 @@ public function aggregate( | |||
| * | |||
| * @param string $realm The realm you are checking to see if exists | |||
| */ | |||
| private function realmEnabled($realm) | |||
| public function realmEnabled($realm) | |||
There was a problem hiding this comment.
nitpick future thing...
I want to rename this isRealmEnabled
reasoning:
isRealmEnabled to me means it is going to tell you true or false (I like to use the prefixes is and has)
vs realmEnabled makes me think of things like session_name which can be used to set things.
eiffel777
added
the
enhancement
There are more events that are VM sessions start and stop events than we previously realized. This PR adds 6 new events, 3 start events and 2 end events. The start events added are unsuspend, unpause and power_on. The end events added are power_off and pause. Resume events in openstack were are also mapped to the RESUME event.
During testing a bug was also found in the aggregation where if an instance had more that one session then the details of the first session was added to the cloud_events_transient table and any other sessions just updated the first row instead of adding a new one because of the keys on the cloud_events_transient table. To fix this I am adding the start_time and end_time field to primary key on that table.
Motivation and Context
Adding the new stop and start events gives us more accuracy on how VM's are being used
Tests performed
Updated regression tests and ran them. Also, manually checked to make sure the new aggregation was correct.
Types of changes
Checklist: