Fixes #10439 - Made trends:reduce task idempotent

[ShimShtein]

Made sure that the task can run multiple times without damaging the system, and it knows how to deal with data points that were added after the original data has already migrated.

[domcleal]

Am I missing something here? I only see tests, no change.

[ShimShtein]

@domcleal That's because my code is awesome :) I have added the tests, but they didn't fail. That means that I have already covered these use cases.
I have left the tests, because they are describing the feature better.

[domcleal]

Ok, so it's just adding tests and not actually making it idempotent.

How is this intended to be used? It's unclear to me from the original ticket.

[unorthodoxgeek]

@ShimShtein first, be careful, you are starting to sound like me. Next up you'll be using four letter words liberally, and I don't want to be a bad influence 😛
second, if you wrote tests and they passed, you are doing something wrong, and should either reconsider calling your code "awesome" or change the order you do things - write the tests first, only then make them pass.

[ShimShtein]

@domcleal Now in a more serious tone:
The task is the migration process that converts trends data from points format to intervals format.
I have put it in the task, so as a user you will be able to run it multiple times in order to be able not to stop reporting procedure while the task is migrating your data.

[domcleal]

Who runs the task? It isn't configured as a cronjob in our packages, called from a DB migration to my knowledge, or documented. Should be called regularly, or only during upgrade from 1.8 to 1.9?

[ohadlevy]

@domcleal see https://github.com/theforeman/foreman/blob/develop/db/migrate/20150202094307_add_range_to_trend_counters.rb#L6

[ohadlevy]

i thought we wanted to get rid of the clean task? is it needed anymore?

[ShimShtein]

I'll put it in a different PR. Without this line the tests will fail.

[domcleal]

@domcleal see https://github.com/theforeman/foreman/blob/develop/db/migrate/20150202094307_add_range_to_trend_counters.rb#L6

Okay, I missed that. The issue description says it should be moved to migrations - should it?

[ohadlevy]

I think it make sense to keep it, in the scenario you still collect new trends counters while the migration is working, one can rerun the task to clear up any new data points.

also useful in case you have multiple foreman running, and you upgrade them one by one or other non standard usage cases..

[ShimShtein]

For simple scenarios it should run only once in the migration. But PR #2365 pointed out a use case when there is more than one writer to a single DB.
The task itself does not maintain a single transaction for all trends, so if the task stops in the middle for some reason, it will be able to resume without corrupting the data.

[domcleal]

Merged as c27e0f6, thanks @ShimShtein.