Skip to content

Store AAMVA tokens in Rails.cache instead of behind an application mutex#8330

Merged
mitchellhenke merged 1 commit intomainfrom
mitchellhenke/use-cache-instead-of-mutex-for-aamva-token
May 4, 2023
Merged

Store AAMVA tokens in Rails.cache instead of behind an application mutex#8330
mitchellhenke merged 1 commit intomainfrom
mitchellhenke/use-cache-instead-of-mutex-for-aamva-token

Conversation

@mitchellhenke
Copy link
Contributor

@mitchellhenke mitchellhenke commented May 3, 2023

🛠 Summary of changes

We are seeing some test flakiness due to the token being stored in a mutex that is not cleared between tests and it can create different states for different tests depending on the order they run. This PR moves the token storage to Rails.cache and relies on it to handle writing and expiration of the token.

The mutex looks to originate from the original implementation of the AAMVA client.

Slack discussion, CI Example Failure 1, CI Example Failure 2

The flaky test can be reproduced reliably with:

rspec spec/services/proofing/aamva/proofer_spec.rb spec/services/proofing/resolution/progessive_proofer_spec.rb --seed 933

@mitchellhenke mitchellhenke force-pushed the mitchellhenke/use-cache-instead-of-mutex-for-aamva-token branch from 029c79e to aa99969 Compare May 3, 2023 20:35
@mitchellhenke mitchellhenke force-pushed the mitchellhenke/use-cache-instead-of-mutex-for-aamva-token branch 3 times, most recently from 078537d to 7f97458 Compare May 4, 2023 01:44
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh i really like just having the cache handle the expiration

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's also a chance this reduces the number of requests to AAMVA for tokens since the mutex would have been a local cache for an individual web server process, but moving it to Rails.cache (backed by Redis) will allow all instances to share the token.

changelog: Internal, AAMVA Proofing, Store AAMVA tokens in Rails.cache instead of behind an application mutex

Co-authored-by: Zach Margolis <zachmargolis@users.noreply.github.com>
@mitchellhenke mitchellhenke force-pushed the mitchellhenke/use-cache-instead-of-mutex-for-aamva-token branch from 7f97458 to 3b1350a Compare May 4, 2023 14:09
@mitchellhenke mitchellhenke marked this pull request as ready for review May 4, 2023 14:19
Copy link
Contributor

@zachmargolis zachmargolis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@mitchellhenke mitchellhenke merged commit af7d9c3 into main May 4, 2023
@mitchellhenke mitchellhenke deleted the mitchellhenke/use-cache-instead-of-mutex-for-aamva-token branch May 4, 2023 18:30
@eileen-nava
Copy link
Contributor

Thank you for fixing this! 🙏🏻

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants