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

High CPU usage for basic auth #1294

Closed
adamcfraser opened this issue Nov 14, 2015 · 2 comments
Closed

High CPU usage for basic auth #1294

adamcfraser opened this issue Nov 14, 2015 · 2 comments

Comments

@adamcfraser
Copy link
Collaborator

adamcfraser commented Nov 14, 2015

While doing performance analysis for the distributed index, compareHashAndPassword was high on the list until user ramp-up completed. This is somewhat working as intended - the bcrypt call is intended to be slow/expensive, to make online attacks expensive.

To address this, we maintain a cache of recent password hashes + SHA1 digest (10000 entries). As users connect and authenticate for the first time, they will gradually be filling up this queue.

However, when this queue fills up, we're dropping it completely and starting again from an empty cache. At that point all active users would trigger the bcrypt call on their next basic auth API call. Under high load, we'd expect a large CPU spike at that point.

If there are more than 10000 concurrent users, we'd be repeatedly dropping and recreating the cache.

At minimum, users anticipating high load should be using session-based auth, not basic auth.

Would like to review whether there's a more efficient way to manage this cache (or if I'm missing a subtlety in the implementation). We might not want the overhead of a full LRU cache, but should find a way to avoid CPU spikes when it fills up.

@adamcfraser
Copy link
Collaborator Author

One additional note - we're not specifying a capacity for the cache map - we should probably do so to avoid the GC work when the map gets internally resized.

@djpongh djpongh added the icebox label Dec 6, 2016
adamcfraser added a commit that referenced this issue Jan 25, 2017
Support more concurrent users/node without hitting the cache limit.  Temporary fix, until #1294 is implemented
@djpongh djpongh added this to the 2.2.0 milestone Jan 31, 2018
@djpongh djpongh modified the milestones: Iridium, Cobalt Nov 19, 2018
@djpongh djpongh modified the milestones: Cobalt, Mercury Dec 13, 2018
@adamcfraser
Copy link
Collaborator Author

Moved to CBG-715

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants