From 0fe2b70d495563892bfd6459588bd6d0ac694ee7 Mon Sep 17 00:00:00 2001
From: Morgan Fainberg <morgan.fainberg@gmail.com>
Date: Fri, 17 Jul 2015 08:28:30 -0700
Subject: [PATCH] Do not remove expired revocation events on "get"

Revocation event pruning should only occur on new revocations. This
will limit the volume of churn to the DB/KVS store for revocation
events (and impact to performance).

Change-Id: I1ad7491c54023915c74610287a5095589f12d5c1
Closes-Bug: #1287757
(cherry picked from commit d7e529911c496c71effa1d51e1ecf2399ef359f1)
---
 keystone/contrib/revoke/backends/kvs.py | 31 +++++++++++++------------
 keystone/contrib/revoke/backends/sql.py |  2 +-
 2 files changed, 17 insertions(+), 16 deletions(-)

diff --git a/keystone/contrib/revoke/backends/kvs.py b/keystone/contrib/revoke/backends/kvs.py
index cc41fbeec9..8ec1291df7 100644
--- a/keystone/contrib/revoke/backends/kvs.py
+++ b/keystone/contrib/revoke/backends/kvs.py
@@ -45,29 +45,30 @@ class Revoke(revoke.Driver):
         except exception.NotFound:
             return []
 
-    def _prune_expired_events_and_get(self, last_fetch=None, new_event=None):
-        pruned = []
+    def list_events(self, last_fetch=None):
         results = []
+
+        with self._store.get_lock(_EVENT_KEY):
+            events = self._list_events()
+
+        for event in events:
+            revoked_at = event.revoked_at
+            if last_fetch is None or revoked_at > last_fetch:
+                results.append(event)
+        return results
+
+    def revoke(self, event):
+        pruned = []
         expire_delta = datetime.timedelta(seconds=CONF.token.expiration)
         oldest = timeutils.utcnow() - expire_delta
-        # TODO(ayoung): Store the time of the oldest event so that the
-        # prune process can be skipped if none of the events have timed out.
+
         with self._store.get_lock(_EVENT_KEY) as lock:
             events = self._list_events()
-            if new_event is not None:
-                events.append(new_event)
+            if event:
+                events.append(event)
 
             for event in events:
                 revoked_at = event.revoked_at
                 if revoked_at > oldest:
                     pruned.append(event)
-                    if last_fetch is None or revoked_at > last_fetch:
-                        results.append(event)
             self._store.set(_EVENT_KEY, pruned, lock)
-        return results
-
-    def list_events(self, last_fetch=None):
-        return self._prune_expired_events_and_get(last_fetch=last_fetch)
-
-    def revoke(self, event):
-        self._prune_expired_events_and_get(new_event=event)
diff --git a/keystone/contrib/revoke/backends/sql.py b/keystone/contrib/revoke/backends/sql.py
index fb285dc803..dd7fdd191b 100644
--- a/keystone/contrib/revoke/backends/sql.py
+++ b/keystone/contrib/revoke/backends/sql.py
@@ -81,7 +81,6 @@ class Revoke(revoke.Driver):
         session.flush()
 
     def list_events(self, last_fetch=None):
-        self._prune_expired_events()
         session = sql.get_session()
         query = session.query(RevocationEvent).order_by(
             RevocationEvent.revoked_at)
@@ -102,3 +101,4 @@ class Revoke(revoke.Driver):
         session = sql.get_session()
         with session.begin():
             session.add(record)
+        self._prune_expired_events()