-
Notifications
You must be signed in to change notification settings - Fork 31
Skip read lock to improve concurrency #100
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
Conversation
I think this works great for |
True. There is still room for improvement for That being said, the improvement on |
I think we'd better accelerate the fix. I have been tripped by this issue myself several times when executing Azure CLI in parallels. |
e36b3b5
to
4dce139
Compare
4dce139
to
e04f8ae
Compare
Avoid the unnecessary sleep after the last retry
e04f8ae
to
2c9690e
Compare
I think there is still a very small chance that Azure/azure-cli#20273 can be triggered: It is when the token expires. If multiple instances refreshes at the same time, the permission issue may still happen since there is no retry logic: microsoft-authentication-extensions-for-python/msal_extensions/token_cache.py Lines 65 to 73 in 2c9690e
This issue has been bugging Azure CLI for years (Azure/azure-cli#9427). |
We consider "multiple app instances encountered token expiration at the same time" would happen less frequently than "multiple app instances run Please continue to advocate customers to adopt a pattern of "run one
Retry was considered unnecessary for
True. All these "unable to obtain a write lock" issues are designed to prevent a token cache file corruption issue (Azure/azure-cli#9427). In that sense, we are already in a better place than token cache file corruption. |
An attempt to bypass the locking behavior when reading token cache. Most of the time, token cache is only serving read operation, so conceptually we will only need to lock once per hour. This would likely improve the concurrency a lot.
@jiasli