Reduce revocation events for performance improvement

[Jorge Munoz]

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)

[Dolph Mathews (dolph)]

http://www.mattfischer.com/blog/?p=672

[Steve Martinelli (stevemar)]

Automatically unassigning due to inactivity.

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

Related fix proposed to branch: master
Review: https://review.openstack.org/393399

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

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

[Lance Bragstad (lbragstad)]

Automatically unassigning due to inactivity.

[Lance Bragstad (lbragstad)]

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

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

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