Skip to content
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

Sample pull request with very basic support for acquiring locks on multiple resources in one call #49

Closed
wants to merge 2 commits into from

Conversation

jsbattig
Copy link

This pull requests allows the user to acquire locks on multiple resources on one call.
The code ain't pretty. I tried not to modify the existing code but rather there's a new set of files that operate with an Enumerable for the lockNames and there's a new set of interfaces and classes to handle this.

I think it could be generalized to keep the code more compact.

I didn't want to delve into Semaphores either as the code is significantly more complicated on the T-SQL side.

And finally, the code has no support for the async methods.

This PR is not intended to be merged as-is but rather to take the idea if desired and implement in the core library and likely extend to other classes.

@madelson
Copy link
Owner

@jsbattig thanks for your interest in the library!

As a starting point, I'd like to better understand your use-case; what kind of pattern requires taking multiple locks in one pass? How many locks are being taken? What is the observed performance difference? Do you foresee this being a need for multiple consumers?

It would be great to create a github issue describing some of the above and linking this PR; we can iterate there.

Also, I'll note that I'm currently working on the 2.0 release of the library; you can see the latest code on the release-2.0 branch.

@jsbattig
Copy link
Author

jsbattig commented Sep 15, 2020 via email

@madelson
Copy link
Owner

Thanks for the detail, @jsbattig , this is really helpful.

Before we go further down this road, how are you constructing your distributed lock instances today when you are acquiring 50 locks in a loop? Are you sharing a single transaction or connection between them (by passing a transaction/connection to the constructor)? If not, can you try sharing a connection/transaction and see how that changes performance?

Obviously batched is going to be faster than 50 round trips, but if the library is opening 50 concurrent SQL connections under the hood then that could be very slow as well.

@jsbattig
Copy link
Author

jsbattig commented Sep 15, 2020 via email

@madelson
Copy link
Owner

Thanks @jsbattig that context is helpful. I've filed an issue for this here: #50 with a proposed API. Mind taking a look and sharing your thoughts there? Is this something you have potential interest in contributing?

@samirbanjanovic
Copy link

What happens when an object in the collection is already locked by some other resource? Would you fail the entire batch or just that lock? Seems like you'd open up code to race conditions and potentially deadlocks. I've not fully read through the code, just my first instinct.

@madelson
Copy link
Owner

madelson commented Mar 9, 2022

@samirbanjanovic see the proposal issue for details.

@madelson
Copy link
Owner

madelson commented Jul 8, 2022

Closing since this is tracked under #50

@madelson madelson closed this Jul 8, 2022
madelson added a commit that referenced this pull request Jul 17, 2023
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.

None yet

3 participants