Add Quota.TimeSinceOriginEvicted UMA histogram.

storage/browser/quota/quota_manager.cc

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "storage/browser/quota/quota_manager.h"
 
 #include <algorithm>
 #include <functional>
 
 #include "base/bind.h"
@@ skipping 55 matching lines @@
 // Preserve kMinimumPreserveForSystem disk space for system book-keeping
 // when returning the quota to unlimited apps/extensions.
 // TODO(kinuko): This should be like 10% of the actual disk space.
 // For now we simply use a constant as getting the disk size needs
 // platform-dependent code. (http://crbug.com/178976)
 int64 QuotaManager::kMinimumPreserveForSystem = 1024 * kMBytes;
 
 const int QuotaManager::kEvictionIntervalInMilliSeconds =
     30 * kMinutesInMilliSeconds;
 
+const char QuotaManager::kTimeBetweenOriginEvictionsHistogram[] =
+    "Quota.TimeBetweenOriginEvictions";
+
 // Heuristics: assuming average cloud server allows a few Gigs storage
 // on the server side and the storage needs to be shared for user data
 // and by multiple apps.
 int64 QuotaManager::kSyncableStorageDefaultHostQuota = 500 * kMBytes;
 
 namespace {
 
 void CountOriginType(const std::set<GURL>& origins,
                      SpecialStoragePolicy* policy,
                      size_t* protected_origins,
@@ skipping 59 matching lines @@
                             SpecialStoragePolicy* policy,
                             GURL* url,
                             QuotaDatabase* database) {
   DCHECK(database);
   database->GetLRUOrigin(type, *exceptions, policy, url);
   return true;
 }
 
 bool DeleteOriginInfoOnDBThread(const GURL& origin,
                                 StorageType type,
+                                bool is_eviction,
                                 QuotaDatabase* database) {
   DCHECK(database);
-  return database->DeleteOriginInfo(origin, type);
+  if (!database->DeleteOriginInfo(origin, type))
+    return false;
+
+  // If the deletion is not due to an eviction, delete the entry in the eviction
+  // table as well due to privacy concerns.
+  if (!is_eviction)
+    return database->DeleteOriginLastEvictionTime(origin, type);
+
+  base::Time last_eviction_time;
+  if (!database->GetOriginLastEvictionTime(origin, type, &last_eviction_time))

[michaeln, 2015/10/15 21:25:23]

In looking at this closely now, I see there are two other values stored in the
db that might be interesting when it comes to evaluating how good/bad the victim
picking logic is? I think the 'age' of the victim is very interesting. I'm not
sure all of these are good stats to have, but am wondering?

These values are stored in the OriginInfo table:
- last_access_time
- used_count

Interesting uma stats maybe:
- AgeOfEvictedOrigin = (now - last_access_time)
- UsageCountOfEvictedOrigin = used_count

Also, a stat for the delta between the victim's age and the oldest in the db's
age might be interesting, I think that could tell us how often karma picks a
different victim than the current lru algo would have.

- RelativeAgeOfEvictedOrigin = (last_access_time - oldest_last_access_time)

[calamity, 2015/10/16 00:53:04]

Yeah, I agree. I think these are all great things to log. I'll make a follow-up
CL. I'd like to split out this change that involves actual table creation and
stuff from just adding a few extra queries.

+    return false;
+
+  base::Time now = base::Time::Now();
+  if (last_eviction_time != base::Time()) {
+    UMA_HISTOGRAM_LONG_TIMES(QuotaManager::kTimeBetweenOriginEvictionsHistogram,

[michaeln, 2015/10/15 21:25:23]

To compute a failure rate (where failure is we repeatly evicted something too
quickly) do we also need to keep track of total eviction events?

There's no uma sample for a first time eviction of an origin so thats not
directly computable. Do you want to have add a sample for that case too, or
maybe a separate bool stat that keeps track of the first time vs repeat
evictions?

Idk the answer, just asking the question.

[calamity, 2015/10/16 00:53:04]

Good point.

I think we have the total eviction count already logged as the sum of
Quota.NumberOfEvictedOriginsPerRound.

SUM(Quota.NumberOfEvictedOriginsPerRound) -
COUNT(Quota.TimeBetweenRepeatedOriginEvictions) should be what we're looking for
here.

+                             now - last_eviction_time);
+  }
+
+  if (!database->SetOriginLastEvictionTime(origin, type, now))
+    return false;
+
+  return true;
 }
 
 bool InitializeTemporaryOriginsInfoOnDBThread(const std::set<GURL>* origins,
                                               QuotaDatabase* database) {
   DCHECK(database);
   if (database->IsOriginDatabaseBootstrapped())
     return true;
 
   // Register existing origins with 0 last time access.
   if (database->RegisterInitialOriginInfo(*origins, kStorageTypeTemporary)) {
@@ skipping 359 matching lines @@
 
   DISALLOW_COPY_AND_ASSIGN(GetUsageInfoTask);
 };
 
 class QuotaManager::OriginDataDeleter : public QuotaTask {
  public:
   OriginDataDeleter(QuotaManager* manager,
                     const GURL& origin,
                     StorageType type,
                     int quota_client_mask,
+                    bool is_eviction,
                     const StatusCallback& callback)
       : QuotaTask(manager),
         origin_(origin),
         type_(type),
         quota_client_mask_(quota_client_mask),
         error_count_(0),
         remaining_clients_(-1),
         skipped_clients_(0),
+        is_eviction_(is_eviction),
         callback_(callback),
         weak_factory_(this) {}
 
  protected:
   void Run() override {
     error_count_ = 0;
     remaining_clients_ = manager()->clients_.size();
     for (QuotaClientList::iterator iter = manager()->clients_.begin();
          iter != manager()->clients_.end(); ++iter) {
       if (quota_client_mask_ & (*iter)->id()) {
         (*iter)->DeleteOriginData(
             origin_, type_,
             base::Bind(&OriginDataDeleter::DidDeleteOriginData,
                        weak_factory_.GetWeakPtr()));
       } else {
         ++skipped_clients_;
         if (--remaining_clients_ == 0)
           CallCompleted();
       }
     }
   }
 
   void Completed() override {
     if (error_count_ == 0) {
       // Only remove the entire origin if we didn't skip any client types.
       if (skipped_clients_ == 0)
-        manager()->DeleteOriginFromDatabase(origin_, type_);
+        manager()->DeleteOriginFromDatabase(origin_, type_, is_eviction_);
       callback_.Run(kQuotaStatusOk);
     } else {
       callback_.Run(kQuotaErrorInvalidModification);
     }
     DeleteSoon();
   }
 
   void Aborted() override {
     callback_.Run(kQuotaErrorAbort);
     DeleteSoon();
@@ skipping 13 matching lines @@
   QuotaManager* manager() const {
     return static_cast<QuotaManager*>(observer());
   }
 
   GURL origin_;
   StorageType type_;
   int quota_client_mask_;
   int error_count_;
   int remaining_clients_;
   int skipped_clients_;
+  bool is_eviction_;
   StatusCallback callback_;
 
   base::WeakPtrFactory<OriginDataDeleter> weak_factory_;
   DISALLOW_COPY_AND_ASSIGN(OriginDataDeleter);
 };
 
 class QuotaManager::HostDataDeleter : public QuotaTask {
  public:
   HostDataDeleter(QuotaManager* manager,
                   const std::string& host,
@@ skipping 49 matching lines @@
       else
         CallCompleted();
     }
   }
 
   void ScheduleOriginsDeletion() {
     remaining_deleters_ = origins_.size();
     for (std::set<GURL>::const_iterator p = origins_.begin();
          p != origins_.end();
          ++p) {
-      OriginDataDeleter* deleter =
-          new OriginDataDeleter(
-              manager(), *p, type_, quota_client_mask_,
-              base::Bind(&HostDataDeleter::DidDeleteOriginData,
-                         weak_factory_.GetWeakPtr()));
+      OriginDataDeleter* deleter = new OriginDataDeleter(
+          manager(), *p, type_, quota_client_mask_, false,
+          base::Bind(&HostDataDeleter::DidDeleteOriginData,
+                     weak_factory_.GetWeakPtr()));
       deleter->Start();
     }
   }
 
   void DidDeleteOriginData(QuotaStatusCode status) {
     DCHECK_GT(remaining_deleters_, 0);
 
     if (status != kQuotaStatusOk)
       ++error_count_;
 
@@ skipping 229 matching lines @@
 
 void QuotaManager::SetUsageCacheEnabled(QuotaClient::ID client_id,
                                         const GURL& origin,
                                         StorageType type,
                                         bool enabled) {
   LazyInitialize();
   DCHECK(GetUsageTracker(type));
   GetUsageTracker(type)->SetUsageCacheEnabled(client_id, origin, enabled);
 }
 
-void QuotaManager::DeleteOriginData(
-    const GURL& origin, StorageType type, int quota_client_mask,
-    const StatusCallback& callback) {
-  LazyInitialize();
-
-  if (origin.is_empty() || clients_.empty()) {
-    callback.Run(kQuotaStatusOk);
-    return;
-  }
-
-  DCHECK(origin == origin.GetOrigin());
-  OriginDataDeleter* deleter =
-      new OriginDataDeleter(this, origin, type, quota_client_mask, callback);
-  deleter->Start();
+void QuotaManager::DeleteOriginData(const GURL& origin,
+                                    StorageType type,
+                                    int quota_client_mask,
+                                    const StatusCallback& callback) {
+  DeleteOriginDataInternal(origin, type, quota_client_mask, false, callback);
 }
 
 void QuotaManager::DeleteHostData(const std::string& host,
                                   StorageType type,
                                   int quota_client_mask,
                                   const StatusCallback& callback) {
   LazyInitialize();
 
   if (host.empty() || clients_.empty()) {
     callback.Run(kQuotaStatusOk);
@@ skipping 395 matching lines @@
 void QuotaManager::StartEviction() {
   DCHECK(!temporary_storage_evictor_.get());
   temporary_storage_evictor_.reset(new QuotaTemporaryStorageEvictor(
       this, kEvictionIntervalInMilliSeconds));
   if (desired_available_space_ >= 0)
     temporary_storage_evictor_->set_min_available_disk_space_to_start_eviction(
         desired_available_space_);
   temporary_storage_evictor_->Start();
 }
 
-void QuotaManager::DeleteOriginFromDatabase(
-    const GURL& origin, StorageType type) {
+void QuotaManager::DeleteOriginFromDatabase(const GURL& origin,
+                                            StorageType type,
+                                            bool is_eviction) {
   LazyInitialize();
   if (db_disabled_)
     return;
 
   PostTaskAndReplyWithResultForDBThread(
       FROM_HERE,
-      base::Bind(&DeleteOriginInfoOnDBThread, origin, type),
-      base::Bind(&QuotaManager::DidDatabaseWork,
-                 weak_factory_.GetWeakPtr()));
+      base::Bind(&DeleteOriginInfoOnDBThread, origin, type, is_eviction),
+      base::Bind(&QuotaManager::DidDatabaseWork, weak_factory_.GetWeakPtr()));
 }
 
 void QuotaManager::DidOriginDataEvicted(QuotaStatusCode status) {
   DCHECK(io_thread_->BelongsToCurrentThread());
 
   // We only try evict origins that are not in use, so basically
   // deletion attempt for eviction should not fail.  Let's record
   // the origin if we get error and exclude it from future eviction
   // if the error happens consistently (> kThresholdOfErrorsToBeBlacklisted).
   if (status != kQuotaStatusOk)
     origins_in_error_[eviction_context_.evicted_origin]++;
 
   eviction_context_.evict_origin_data_callback.Run(status);
   eviction_context_.evict_origin_data_callback.Reset();
 }
 
+void QuotaManager::DeleteOriginDataInternal(const GURL& origin,
+                                            StorageType type,
+                                            int quota_client_mask,
+                                            bool is_eviction,
+                                            const StatusCallback& callback) {
+  LazyInitialize();
+
+  if (origin.is_empty() || clients_.empty()) {
+    callback.Run(kQuotaStatusOk);
+    return;
+  }
+
+  DCHECK(origin == origin.GetOrigin());
+  OriginDataDeleter* deleter = new OriginDataDeleter(
+      this, origin, type, quota_client_mask, is_eviction, callback);
+  deleter->Start();
+}
+
 void QuotaManager::ReportHistogram() {
   GetGlobalUsage(kStorageTypeTemporary,
                  base::Bind(
                      &QuotaManager::DidGetTemporaryGlobalUsageForHistogram,
                      weak_factory_.GetWeakPtr()));
   GetGlobalUsage(kStorageTypePersistent,
                  base::Bind(
                      &QuotaManager::DidGetPersistentGlobalUsageForHistogram,
                      weak_factory_.GetWeakPtr()));
 }
@@ skipping 54 matching lines @@
 void QuotaManager::EvictOriginData(const GURL& origin,
                                    StorageType type,
                                    const EvictOriginDataCallback& callback) {
   DCHECK(io_thread_->BelongsToCurrentThread());
   DCHECK_EQ(type, kStorageTypeTemporary);
 
   eviction_context_.evicted_origin = origin;
   eviction_context_.evicted_type = type;
   eviction_context_.evict_origin_data_callback = callback;
 
-  DeleteOriginData(origin, type, QuotaClient::kAllClientsMask,
-                   base::Bind(&QuotaManager::DidOriginDataEvicted,
-                              weak_factory_.GetWeakPtr()));
+  DeleteOriginDataInternal(origin, type, QuotaClient::kAllClientsMask, true,
+                           base::Bind(&QuotaManager::DidOriginDataEvicted,
+                                      weak_factory_.GetWeakPtr()));
 }
 
 void QuotaManager::GetUsageAndQuotaForEviction(
     const UsageAndQuotaCallback& callback) {
   DCHECK(io_thread_->BelongsToCurrentThread());
   LazyInitialize();
 
   UsageAndQuotaCallbackDispatcher* dispatcher =
       new UsageAndQuotaCallbackDispatcher(this);
   GetUsageTracker(kStorageTypeTemporary)
@@ skipping 154 matching lines @@
   // |database_|, therefore we can be sure that database_ is alive when this
   // task runs.
   base::PostTaskAndReplyWithResult(
       db_thread_.get(),
       from_here,
       base::Bind(task, base::Unretained(database_.get())),
       reply);
 }
 
 }  // namespace storage