Fix statsd crash due to bad bucket index

Statsd crashes because predictAnomalyTimestamp requests past buckets
that occurred before time began. That's fine, but statsd needs to know
that the data before time began was 0 (instead of reading before the
beginning of the array and crashing).

AnomalyTracker's use of bucketNumbers is in general risky, and should be
auditted. But this cl will fix the current crashing.

Bug: 73825954
Test: Definitely necessary. Will write during audit.
Change-Id: I990ff134153f290d3089bfe3440d838f47996b63
This commit is contained in:
Bookatz
2018-03-08 11:16:48 -08:00
parent 983c1e54e1
commit 2fb5653b97
2 changed files with 9 additions and 5 deletions

View File

@@ -59,6 +59,12 @@ void AnomalyTracker::resetStorage() {
}
size_t AnomalyTracker::index(int64_t bucketNum) const {
if (bucketNum < 0) {
// To support this use-case, we can easily modify index to wrap around. But currently
// AnomalyTracker should never need this, so if it happens, it's a bug we should log.
// TODO: Audit this.
ALOGE("index() was passed a negative bucket number (%lld)!", (long long)bucketNum);
}
return bucketNum % mNumOfPastBuckets;
}
@@ -72,9 +78,7 @@ void AnomalyTracker::flushPastBuckets(const int64_t& latestPastBucketNum) {
// The past packets are ancient. Empty out old mPastBuckets[i] values and reset
// mSumOverPastBuckets.
if (latestPastBucketNum - mMostRecentBucketNum >= mNumOfPastBuckets) {
mPastBuckets.clear();
mPastBuckets.resize(mNumOfPastBuckets);
mSumOverPastBuckets.clear();
resetStorage();
} else {
for (int64_t i = std::max(0LL, (long long)(mMostRecentBucketNum - mNumOfPastBuckets + 1));
i <= latestPastBucketNum - mNumOfPastBuckets; i++) {
@@ -150,7 +154,7 @@ void AnomalyTracker::addBucketToSum(const shared_ptr<DimToValMap>& bucket) {
int64_t AnomalyTracker::getPastBucketValue(const MetricDimensionKey& key,
const int64_t& bucketNum) const {
if (mNumOfPastBuckets == 0) {
if (mNumOfPastBuckets == 0 || bucketNum < 0) {
return 0;
}

View File

@@ -110,7 +110,7 @@ protected:
// Number of past buckets. One less than the total number of buckets needed
// for the anomaly detection (since the current bucket is not in the past).
int mNumOfPastBuckets;
const int mNumOfPastBuckets;
// The existing bucket list.
std::vector<shared_ptr<DimToValMap>> mPastBuckets;