Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix extraneous cloud sessions #1504

Merged
merged 6 commits into from
Mar 19, 2021

Conversation

eiffel777
Copy link
Contributor

https://app.asana.com/0/808093868887967/1199187338120726

This fixes an issue where extra sessions may exist in the session_records table when ingesting data to fill a time gap where data was not ingested. An example of a gap in the data could be the file copying not running on a given day and then a catch-up run being done later. The CloudStateReconstructorTransformIngestor constructs all sessions for all instances that have an event greater than the last_modified_start_date specified when you run xdmod-ingestor or the relevant etl_overseer.php command and places the in the event_reconstructed table.

To make sure we have the correct sessions this change removes all the sessions for instances we are aggregating from the session_records table. This means that when the cloud-session-records action runs it will load all the sessions from the event_reconstructed table which has the correct sessions for each instance and none of the extra sessions.

It also removes rows from cloudfact_by_day_sessionlist that have session_ids tied to instances that are being aggregated. If this is not done it would result in rows in cloudfact_by_day_sessionlist that have session_ids that no longer exist.

Motivation and Context

Preventing sessions that are incorrect from existing in the session_records table

Tests performed

Tested in docker

Checklist:

  • The pull request description is suitable for a Changelog entry
  • The milestone is set correctly on the pull request
  • The appropriate labels have been added to the pull request

@eiffel777 eiffel777 added bug Bugfixes Category:Cloud Cloud Realm labels Mar 5, 2021
@eiffel777 eiffel777 added this to the 9.5.0 milestone Mar 5, 2021
@eiffel777 eiffel777 requested review from jpwhite4 and ryanrath March 5, 2021 17:48
@eiffel777 eiffel777 self-assigned this Mar 5, 2021
@jpwhite4
Copy link
Member

jpwhite4 commented Mar 5, 2021

When you re-aggregate a row it should automatically remove the corresponding rows from the session table. This is done in the deleteAggregationPeriodData() function in the aggregator class. So why add an extra manual step to do the delete twice?

@jpwhite4
Copy link
Member

jpwhite4 commented Mar 5, 2021

I think i have found the answer to my question. But the question is now why didn't you use the dedicated aggregation class that is designed for the purpose of handling aggregation with session|job tables?

@eiffel777 eiffel777 merged commit 371353b into ubccr:xdmod9.5 Mar 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Bugfixes Category:Cloud Cloud Realm
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants