Old revocation events must be purged

Bug #1456797 reported by Dolph Mathews on 2015-05-19
16
This bug affects 3 people
Affects Status Importance Assigned to Milestone
OpenStack Identity (keystone)
Medium
Morgan Fainberg

Bug Description

Similar to token_flush, we need an operation to purge old revocation events from the database, else they'll eventually consume excessive disk space. The operation should be very similar in implementation and usage to token_flush.

Filing this as Medium priority because it does impact all supported releases (Icehouse+), but operators can get around the issue by manually running DELETE queries.

Changed in keystone:
assignee: nobody → Deepti Ramakrishna (dramakri)
Deepti Ramakrishna (dramakri) wrote :

Dolph, old revocation events already seem to be deleted as part of the list_events operation. See:

1) the call to _prune_expired_events() from list_events() in keystone/contrib/revoke/backends/sql.py
2) the call to _prune_expired_events_and_get() from list_events() in keystone/contrib/revoke/backends/kvs.py

It seems that list_events() is a high-traffic call (via the "GET /OS-REVOKE/events" API) that would therefore ensure that the event count in the database is not too large. Should we still augment the keystone-manage command line tool with the "revocation_event_flush" command (similar to "token_flush" command)? If so, what would be the reasons?

Dolph Mathews (dolph) on 2015-06-01
Changed in keystone:
importance: Medium → Low
Dolph Mathews (dolph) wrote :

Good to hear! Thanks for investigating. Downgraded the priority of this as a result. I actually reported this as the outcome of a summit conversation with Adam Young- neither of us were aware that they were already being purged.

I guess that presents another potential issue, though. If DELETEs are being emitted on every list_events(), does that cause a negative performance impact or unnecessary load on the database? The benefit of the current implementation, I assume, is that the controller is far less likely to emit expired events. Events are intended to be relatively rare, though, so incredibly frequent DELETEs should be unnecessary.

Changed in keystone:
status: Triaged → Incomplete
Deepti Ramakrishna (dramakri) wrote :

My understanding is that delete-token operation is very rare (must be done explicitly by a user/admin) while list-events operation is very frequent (every OpenStack service invokes this for every API request to check if an unexpired token is revoked). So, doing a purge on every list-event seems too frequent as you say, although probably not very stressing on the database since there would be nothing to delete most of the time (do empty deletes stress the database?)

I don't have enough knowledge to be sure about some of the above assumptions. If it is not db-stressing, should we leave the code as it is and close the bug? If not, should we move purge functionality exclusively to the keystone-manage tool?

Dolph Mathews (dolph) wrote :

I agree with your analysis & share your questions. I'm hoping Adam Young can provide further insight.

Adam Young (ayoung) wrote :

I was sasuming that most of the list_event calls would be "no-ops" for purging, and only the first one would be expensive. We an rewrite the mechanism based on real data.

Dolph Mathews (dolph) wrote :

I don't have real data at this point (the impact of emitting frequent DELETE calls with 0 rows affected), so I'm leaving this as Incomplete unless someone else pitches in.

Deepti Ramakrishna (dramakri) wrote :

Thanks Dolph and Adam. I plan to leave it as is for now and wait to see if others have any input.

Deepti Ramakrishna (dramakri) wrote :

Dolph/Adam, I had sent an email to the openstack-operators mailing list (http://lists.openstack.org/pipermail/openstack-operators/2015-June/007210.html) asking for opinions about this issue based on their actual deployment experience but didn't get much feedback. I got exactly one reply mentioning that they see revoke tokens being used rarely and hence not an issue of concern to them. Since there is no clear forward path, I am unassigning myself from this bug.

Changed in keystone:
assignee: Deepti Ramakrishna (dramakri) → nobody
Dolph Mathews (dolph) wrote :

Thanks, Deepti! I'm going to leave this as incomplete for now. If this issue becomes concerning, we can pick it back up later.

Dolph Mathews (dolph) wrote :

Restored this bug and increased the priority based on concern for this behavior from Matt Fischer.

Changed in keystone:
status: Incomplete → Triaged
importance: Low → Medium
Adam Young (ayoung) wrote :
Changed in keystone:
assignee: nobody → Morgan Fainberg (mdrnstm)
status: Triaged → Fix Committed
Morgan Fainberg (mdrnstm) wrote :

Im going to mark this as a duplicate of 1287757

To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers