Reduce revocation events for performance improvement

Bug #1524030 reported by Jorge Munoz on 2015-12-08
12
This bug affects 2 people
Affects Status Importance Assigned to Milestone
OpenStack Identity (keystone)
Medium
Lance Bragstad

Bug Description

Keystone performance reduces as revocation events are written to the backend. In an effort to reduce the number of revocation events written to the revocation_event table, keystone has to explicitly check if a domain or project associated to the token are enabled.

Patch: https://review.openstack.org/#/c/253273/

Follow up patches:
1. Remove revocation events for deleted domains or projects
2. Remove revocation events for deleted grants
3. Bug 1511775
4. Delete unused columns (revocation_table - project and domains)

Changed in keystone:
assignee: nobody → Jorge Munoz (jorge-munoz)
status: New → In Progress
description: updated
Dolph Mathews (dolph) wrote :
tags: added: performance
Changed in keystone:
importance: Undecided → Medium
tags: added: revoke
description: updated
Steve Martinelli (stevemar) wrote :

Automatically unassigning due to inactivity.

Changed in keystone:
assignee: Jorge Munoz (jorge-munoz) → nobody
status: In Progress → Triaged
Changed in keystone:
assignee: nobody → Steve Martinelli (stevemar)
status: Triaged → In Progress
Changed in keystone:
assignee: Steve Martinelli (stevemar) → nobody
status: In Progress → Triaged

Fix proposed to branch: master
Review: https://review.openstack.org/370252

Changed in keystone:
assignee: nobody → Richard (csravelar)
status: Triaged → In Progress

Change abandoned by Richard Avelar (<email address hidden>) on branch: master
Review: https://review.openstack.org/370252

Change abandoned by Richard Avelar (<email address hidden>) on branch: master
Review: https://review.openstack.org/371076

Changed in keystone:
assignee: Richard (csravelar) → Ron De Rose (ronald-de-rose)
Changed in keystone:
assignee: Ron De Rose (ronald-de-rose) → Steve Martinelli (stevemar)
Changed in keystone:
assignee: Steve Martinelli (stevemar) → Ron De Rose (ronald-de-rose)

Reviewed: https://review.openstack.org/376523
Committed: https://git.openstack.org/cgit/openstack/keystone/commit/?id=477189d0c51a3fd2f8709827fb635934f0e74200
Submitter: Jenkins
Branch: master

commit 477189d0c51a3fd2f8709827fb635934f0e74200
Author: Ronald De Rose <email address hidden>
Date: Mon Sep 26 13:59:46 2016 +0000

    Add revocation event indexes

    This patch adds indexes to the revocation_event table as part of an
    effort to improve performance during token validation.

    Partial-Bug: 1524030
    Change-Id: I73ff077bb6dc3ca8821f8cc14639bf986517d158

Reviewed: https://review.openstack.org/382107
Committed: https://git.openstack.org/cgit/openstack/keystone/commit/?id=9e84371461831880ce5736e9888c7d9648e3a77b
Submitter: Jenkins
Branch: master

commit 9e84371461831880ce5736e9888c7d9648e3a77b
Author: “Richard <email address hidden>
Date: Tue Oct 4 23:00:31 2016 +0000

    Improve check_token validation performance

    This patch improves check_token validation performance by only pulling
    revocation events based on the token issued_at value, taking advantage
    of the table index. In this way, only a subset of relevant events will
    be returned for validation.

    Benchmarks can be seen at [1], but included here as well:

    Time per Request for Old Method
    -------------------------------
    10 revokes at 7.908
    100 revokes at 18.224
    1,000 revokes at 110.155
    10,000 revokes at 1998.220

    Time per Request New Method
    ---------------------------
    10 revokes at 17.636ms,
    100 revokes at 17.279ms,
    1,000 revokes at 17.370,
    10,000 revokes w/all revokes issued before token at 17.263 (best case)
    10,000 revokes w/all revokes after token creation 44.934ms (worst case)

    [1] https://gist.github.com/csrichard1/4b7b8527ee5a6565a84956cff33cf29b

    Change-Id: I9c2f067d870d542ec5909eaf8b24ded07b75f433
    Partial-Bug: 1524030

Richard (csravelar) on 2016-11-01
Changed in keystone:
assignee: Ron De Rose (ronald-de-rose) → Richard (csravelar)

Change abandoned by Richard Avelar (<email address hidden>) on branch: master
Review: https://review.openstack.org/387548

Changed in keystone:
assignee: Richard (csravelar) → Steve Martinelli (stevemar)
Changed in keystone:
assignee: Steve Martinelli (stevemar) → Richard (csravelar)
Changed in keystone:
assignee: Richard (csravelar) → Steve Martinelli (stevemar)

Reviewed: https://review.openstack.org/393399
Committed: https://git.openstack.org/cgit/openstack/keystone/commit/?id=852a5186b8bcbd64fe274ed5b05d72f5809c254f
Submitter: Jenkins
Branch: master

commit 852a5186b8bcbd64fe274ed5b05d72f5809c254f
Author: Richard Avelar <email address hidden>
Date: Thu Nov 3 16:32:36 2016 +0000

    Remove unused statements in matches

    With recent changes [1], in list_events in the sql backend there is no
    longer a need to use repetitive statements. list_events already prunes
    out rows based off user_id, project_id, audit_id and issued_at so we
    can remove these subsequent statements in matches.
    In order to test this, the _assertToken methods needed to call the
    list_events method directly. The old implementation ignores
    list_events and instead makes a list where it can "add_event" and
    "remove_event" which bypasses testing the actual changes made in
    list_events since the tests assume list_events would have just sent
    back the entire list anyway rather than what it does now which is
    filter events based off the token

    [1] 9e84371461831880ce5736e9888c7d9648e3a77b
    Related-Bug: 1524030
    Change-Id: I8cb111df733f826df7aabf70359cc849a70f914b

Changed in keystone:
assignee: Steve Martinelli (stevemar) → Richard (csravelar)

Change abandoned by Richard Avelar (<email address hidden>) on branch: master
Review: https://review.openstack.org/388842

Lance Bragstad (lbragstad) wrote :

Automatically unassigning due to inactivity.

Changed in keystone:
assignee: Richard (csravelar) → nobody
status: In Progress → Triaged
Lance Bragstad (lbragstad) wrote :

Doing some investigation to see what is left to close this bug. There have been several patches that improve performance by adding indexes to the revocation table and taking a lot of the processing out of python. This is one patch left that makes it so revocation events are not persisted when a project/domain is disabled [0]. I'd like to get that reviewing passing and merged, then this can be closed given the improvements made.

[0] https://review.openstack.org/#/c/253273/37

Changed in keystone:
assignee: nobody → Lance Bragstad (lbragstad)
status: Triaged → In Progress

Change abandoned by Lance Bragstad (<email address hidden>) on branch: master
Review: https://review.openstack.org/378047
Reason: We have another patch in review that attempts to do the same thing [0]. I got the other patch passing tests locally before I saw this review. I've incorporated the code from this review in to the other patch.

Abandoning this one since its duplicate.

[0] https://review.openstack.org/#/c/253273

tags: added: office-hours

Reviewed: https://review.openstack.org/253273
Committed: https://git.openstack.org/cgit/openstack/keystone/commit/?id=8eb29c37d1a5163d4f485c559399a4b82969e21e
Submitter: Zuul
Branch: master

commit 8eb29c37d1a5163d4f485c559399a4b82969e21e
Author: Jorge Munoz <email address hidden>
Date: Fri Nov 24 22:59:32 2017 +0000

    Validate disabled domains and projects online

    Keystone's performance degrades as the `revocation_event` table grows
    in size. This patch reduces the total number of events written to the
    table by not persisting events when a domain or project is disabled.

    The main reason for persisting a revocation event when a project or
    domain is disabled is to make sure tokens associated to those targets
    are considered invalid. Instead of relying on revocation events, we
    can check if the project or domain is enabled when we validate the
    token. We take the same approach when we validate a user's role
    assignments instead of relying on an ever-growing database table.

    Co-Authored-By: Lance Bragstad <email address hidden>

    Closes-Bug: 1524030
    Change-Id: I76330567e0df2d9f2af88ef9b6b98b8c379e7406

Changed in keystone:
status: In Progress → Fix Released
Changed in keystone:
milestone: none → queens-2

This issue was fixed in the openstack/keystone 13.0.0.0b2 development milestone.

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

Other bug subscribers