-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Reduce subscription lock usage to unsubscribe workflow only #17483
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like it is protecting DataLayer.unsubscribe_data_queue
which we append to in some places and iterate in others. At least. That could be addressed in other ways potentially.
Oh, I thought this was the PR where you wanted to remove a lock entirely. Nevermind, dismissing my review as I need to look more. |
This PR has been flagged as stale due to no activity for over 60 days. It will not be automatically closed, but it has been given a stale-pr label and should be manually reviewed by the relevant parties. |
close and reopen |
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
1 similar comment
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
This PR has been flagged as stale due to no activity for over 60 days. It will not be automatically closed, but it has been given a stale-pr label and should be manually reviewed by the relevant parties. |
Conflicts have been resolved. A maintainer will review the pull request shortly. |
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
This PR has been flagged as stale due to no activity for over 60 days. It will not be automatically closed, but it has been given a stale-pr label and should be manually reviewed by the relevant parties. |
Conflicts have been resolved. A maintainer will review the pull request shortly. |
I think we can remove most of these subscription locks and rely on the DB context reader/writer locks instead. We don't cache this information, so I don't think we need any of these for data consistency on subscriptions
The one place where we maintain the lock is during unsubscribe where it protects access to the
unsubscribe_data_queue