[ML] Remove out-of-phase buckets feature #318
Conversation
This feature was never fully completed and in fact we no longer need it as the multibucket feature covers the benefits from supporting out-of-phase buckets.
There was a problem hiding this comment.
This looks great Dimitris! I've been through once and don't see any problems. Just a couple of minor suggestions. I'd like to digest this a bit more and 1) see if there is anything else redundant I can think of, 2) convince myself a bit more there isn't anything that has been accidentally broken.
The failing unit test could well be a perturbation effect due to changing memory used by, for example, removing queues. If the change is small it'll be fine to just update the threshold as the control is approximate.
include/api/CAnomalyJob.h
Outdated
| //! \p processingTimer is the processing time can be written to the bucket | ||
| //! \p sumPastProcessingTime is the total time previously spent processing | ||
| //! but resulted in no bucket being outputted. | ||
| //! \p processingTime is the processing time can be written to the bucket |
There was a problem hiding this comment.
is the processing time can be written to the bucket |-> is the processing time of the bucket
lib/model/CDataGatherer.cc
Outdated
| boost::cref(gatherer_), _1)); | ||
| } | ||
| const std::string& tag = m_BucketGatherer->persistenceTag(); | ||
| if (tag == CBucketGatherer::EVENTRATE_BUCKET_GATHERER_TAG) { |
There was a problem hiding this comment.
This probably doesn't need to cast and use separate code paths since bind behaves polymorphically, i.e. I think
inserter.insertLevel(m_BucketGatherer->persistenceTag(),
boost::bind(&CBucketGatherer::acceptPersistInserter, m_BucketGatherer.get(), _1));
works. Persist and restore unit tests will fail if it doesn't so you can try it out safely.
This feature was never fully completed and in fact we no longer need it as the multibucket feature covers the benefits from supporting out-of-phase buckets.
This feature was never fully completed and in fact
we no longer need it as the multibucket feature
covers the benefits from supporting out-of-phase buckets.
closes #302