-
Notifications
You must be signed in to change notification settings - Fork 38
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
Is this thread safe? #13
Comments
Hey @patrykk21, distribute_reads doesn't try to resolve any issues with thread-safety. However, I haven't seen thread-safety issues with Makara and Sidekiq. Are you seeing issues with your app? |
Not me personally but we'd rather not use the gem in production environment since the issue I linked is still open. Threading issues are really hard to replicate. But I guess you answered me, thank you |
I commented on the issue to try and get more info, but hesitant to add a warning without understanding more. |
Thank you very much. You can either close this or keep it open. I'll be following the thread since your gem seems interesting. It would be our primary choice if not for that issue |
Going to close. I certainly think it’s possible there are code paths that aren’t thread safe in Makara, but the discussion has staled and I haven’t seen issues with very heavy usage of Makara. |
Hey @ankane, I believe we are starting to see issues with this. We havent met, but I am in your old stomping ground, ^. There is talk of trying go move off makara. Which seems...a big task for us. Just thought I'd let you know as I'm investigating over here. |
Nice to meet you @philosoralphter, let me know what you find. This gem was created as an abstraction so you should be able to swap out the underlying database library without touching much application code. |
Hey @ankane, is it advisable to use to_a with lazy loading in the context of distribute_reads? |
Reading through makara (which this gem is dependent on), they have an open and still to be resolved issue at: instacart/makara#151
It basically says it is not thread safe, therefore won't properly work if puma/sidekiq are configured for multiple threads.
Did you fix the issue in your gem? If not I think it should be a really important information to highlight in the
README.md
.The text was updated successfully, but these errors were encountered: