search

waleedkadous / ansari-backend

Ansari is a helper for you to become a better Muslim
65 stars 12 forks source link

Implement refresh token caching to prevent race conditions #40

Closed abdullah-alnahas closed 1 month ago

abdullah-alnahas commented 1 month ago

Could you please review the pull request, especially the caching logic?

abdullah-alnahas commented 1 month ago

I appreciate your thoughts on this. I wanted to clarify my reasoning for exploring a caching solution like diskcache.

The primary goal is to manage concurrent refresh token requests effectively:

  1. Block concurrent refresh token requests from the same user, while still being able to handle concurrent requests from other users.
  2. Invalidate the old refresh token, but allow a grace period for concurrent requests.

While our existing database abstractions are robust, they might not provide the level of atomicity and concurrency control I'm looking for in this specific case. Caching solutions are designed to handle such scenarios more efficiently.

That being said, I understand the value of reusing the tools and systems we already have in place. If diskcache ends up adding complexity or issues to our Heroku hosting environment, I'm open to considering other options. Ideally, I'd like a solution that covers both aspects (1 & 2) out of the box, without the need for me to do any additional implementation.

In my search, I found dogpile.cache, which seems to be a good fit as it can use PostgreSQL as a backend. The transition to it shouldn't be too time-consuming, as the core logic is in place and I'd just be swapping out APIs.

What are your thoughts on this? Should I stick with diskcache or make the move to dogpile.cache?

waleedkadous commented 1 month ago

Let's ship it as is and then if there are issues we can switch to dogpile

On Fri, May 24, 2024 at 6:09 PM Abdullah Al Nahas @.***> wrote:

I appreciate your thoughts on this. I wanted to clarify my reasoning for exploring a caching solution like diskcache.

The primary goal is to manage concurrent refresh token requests effectively:

  1. Block concurrent refresh token requests from the same user, while still being able to handle concurrent requests from other users.
  2. Invalidate the old refresh token, but allow a grace period for concurrent requests.

While our existing database abstractions are robust, they might not provide the level of atomicity and concurrency control I'm looking for in this specific case. Caching solutions are designed to handle such scenarios more efficiently.

That being said, I understand the value of reusing the tools and systems we already have in place. If diskcache ends up adding complexity or issues to our Heroku hosting environment, I'm open to considering other options. Ideally, I'd like a solution that covers both aspects (1 & 2) out of the box, without the need for me to do any additional implementation.

In my search, I found dogpile.cache https://github.com/sqlalchemy/dogpile.cache, which seems to be a good fit as it can use PostgreSQL as a backend. The transition to it shouldn't be too time-consuming, as the core logic is in place and I'd just be swapping out APIs.

What are your thoughts on this? Should I stick with diskcache or make the move to dogpile.cache?

— Reply to this email directly, view it on GitHub https://github.com/waleedkadous/ansari-backend/pull/40#issuecomment-2130635077, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAUXGUHB6URJUXT3IM35N2DZD7QE5AVCNFSM6AAAAABIG3MNEKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCMZQGYZTKMBXG4 . You are receiving this because you modified the open/close state.Message ID: @.***>