Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

UaaTokenStore: Don't fall over when failing to expire oauth codes #772

Closed

Conversation

mook-as
Copy link
Contributor

@mook-as mook-as commented Feb 9, 2018

MySQL does row-level locking, which means when we delete expired oauth codes in parallel we can sometimes hit a harmless deadlock (where the two delete updates execute across the rows in different order simultaneously). This is not an issue as the winning update will expire tokens as appropriate, plus the tokens are checked for expiry on use.

This was encountered when repeatedly running multiple instances of cf ssh in parallel on a Cloud Foundry deployment using a single non-HA UAA instance (backed by a single non-HA MySQL database).

Thanks for building UAA!

@cfdreddbot
Copy link

Hey mook-as!

Thanks for submitting this pull request! I'm here to inform the recipients of the pull request that you and the commit authors have already signed the CLA.

@cf-gitbot
Copy link

We have created an issue in Pivotal Tracker to manage this:

https://www.pivotaltracker.com/story/show/155091700

The labels on this github issue will be updated when the story is started.

mook-as added a commit to mook-as/uaa that referenced this pull request Feb 9, 2018
MySQL does row-level locking, which means when we delete expired oauth
codes in parallel we can sometimes hit a harmless deadlock (where the
two delete updates execute across the rows in different order
simultaneously).  This is not an issue as the winning update will expire
tokens as appropriate, plus the tokens are checked for expiry on use.

This commit is cherry-picked from 0144d13
( cloudfoundry#772 )
mook-as added a commit to mook-as/uaa-release that referenced this pull request Feb 9, 2018
See cloudfoundry/uaa#772
Cherry picked from PR commit 0144d1310434b94cb18742e975f4aed0bfe8e1bd
@coveralls
Copy link

coveralls commented Feb 9, 2018

Coverage Status

Coverage increased (+0.006%) to 86.454% when pulling 332d119 on mook-as:oauth-token-expiry-deadlock into a4132cb on cloudfoundry:master.

MySQL does row-level locking, which means when we delete expired oauth
codes in parallel we can sometimes hit a harmless deadlock (where the
two delete updates execute across the rows in different order
simultaneously).  This is not an issue as the winning update will expire
tokens as appropriate, plus the tokens are checked for expiry on use.
@fhanik
Copy link
Contributor

fhanik commented Mar 5, 2018

Closed via: ebdecc4

@fhanik fhanik closed this Mar 5, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accepted Accepted the issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants