Memoize cols_on_metric_rollup in chargeback #17927
Conversation
|
Checked commits lpichler/manageiq@bf2b318~...131cf41 with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0 |
| chargeable_cols.map! { |x| VIRTUAL_COL_USES[x] || x }.sort | ||
| @chargeable_cols_on_metric_rollup ||= begin | ||
| existing_cols = MetricRollup.attribute_names | ||
| chargeable_cols = pluck(:metric) & existing_cols |
There was a problem hiding this comment.
Could new records be created with new data in the metric column that would never be seen?
There was a problem hiding this comment.
Yes but not during running application. It could be added only by adding record (this happens very rarely) to chargeable_field.yml and based on the yaml, table chargeable_field table is filled up in the seed process.
thanks
…ollup Revert "Merge pull request #17927 from lpichler/memoize_cols_on_metri…
| chargeable_cols.map! { |x| VIRTUAL_COL_USES[x] || x }.sort | ||
| @chargeable_cols_on_metric_rollup ||= begin | ||
| existing_cols = MetricRollup.attribute_names | ||
| chargeable_cols = pluck(:metric) & existing_cols |
There was a problem hiding this comment.
@libor, I think this line may be causing the sporadic test failures. If this code is called before ChargeableField is seeded it will memoize an empty array. The other tests will use the memoized value and potentially get the nil error. I found that consumption_with_rollups_spec.rb calls this code in a bunch of places without seeding ChargeableField. The fix may be to just make sure the class is seeded first.
There was a problem hiding this comment.
yes! and finally I probably found a culprit. ./spec/models/chargeback/consumption_with_rollups_spec.rb but I am not sure it if it is the only culprit.
thanks
Pulled out from #17591 done during work on this #17592
Links
*#17592
@miq-bot add_label performance, chargeback
@miq-bot assign @gtanzillo