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

[COST-4624] do not show progress update during initial data gather #296

Merged
merged 5 commits into from
Feb 28, 2024

Conversation

maskarb
Copy link
Member

@maskarb maskarb commented Feb 20, 2024

Updating the status during initial data gather is causing the operator to generate invalid reports.

Here is the scenario:

  1. a cluster was created in the afternoon on 2-19.
  2. the operator was installed on 2-20.
  3. During initial data gathering, we start gathering data from 14 days ago. As we progress thru the days with no data, we set the status with the last successful query time and the message that no data was collected. Each of these status updates causes another reconciliation.
  4. While we progress thru the first reconciliation, we do not gather the data on the 19th because it is incomplete data.
  5. We gather all of the 2-20 data and ship the payload. This data file remains on the PVC because we have not collected all of 2-20 data yet.
  6. The next reconciliation starts (one of the ones that was triggered by the progress update). Let's suppose that during this reconciliation, the status was set to:
"last_hour_queried":"2024-02-18 00:00:00 - 2024-02-18 00:59:59","data_collection_message":"No data to report for the hour queried."

Because of this logic, we restart gathering data from 2-18 01:00:00. This causes us to insert the 2-19 data into the report file. So now we have 2-19 data after the 2-20 data in the report files.

The simplest solution for this is to not do the status update until the full reconciliation is done.

Copy link

codecov bot commented Feb 20, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (9f05c65) 85.12% compared to head (06dd3ba) 85.11%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #296      +/-   ##
==========================================
- Coverage   85.12%   85.11%   -0.02%     
==========================================
  Files          13       13              
  Lines        2730     2728       -2     
==========================================
- Hits         2324     2322       -2     
  Misses        325      325              
  Partials       81       81              
Flag Coverage Δ
unittests 85.11% <ø> (-0.02%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
...nternal/controller/kokumetricsconfig_controller.go 85.94% <ø> (-0.05%) ⬇️

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9f05c65...06dd3ba. Read the comment docs.

@maskarb maskarb changed the title do not show progress update during initial data gather [COST-4624] do not show progress update during initial data gather Feb 20, 2024
Copy link
Contributor

@djnakabaale djnakabaale left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm 👍🏾

@maskarb maskarb merged commit 47fe1f2 into main Feb 28, 2024
10 checks passed
@maskarb maskarb deleted the dont-show-progress branch February 28, 2024 15:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants