✨feat: Added new method TryGetAcquireLocks and override for ReleaseAppLock method to work with lists. #13
Conversation
…pLock method to work with lists.
✨feat: Added new method TryGetAcquireLocks and override for ReleaseAppLock method to work with lists.
| return allReleased; | ||
| } | ||
|
|
||
| public static async Task<bool> TryGetAcquireLocks(this SqlConnection connection, IEnumerable<int> keys, int retryTimeout = 100, int numberOfRetries = 3, IDbTransaction transaction = null, TimeSpan? lockTimeout = null) |
There was a problem hiding this comment.
To keep the name aligned with existing GetAppLockAsync, this should be TryGetAppLockAsync
|
|
||
| public static async Task<bool> TryGetAcquireLocks(this SqlConnection connection, IEnumerable<int> keys, int retryTimeout = 100, int numberOfRetries = 3, IDbTransaction transaction = null, TimeSpan? lockTimeout = null) | ||
| { | ||
| if (keys == null) |
There was a problem hiding this comment.
There should also be a check if there is 0 keys in the IEnumerable.
|
|
||
| foreach (var key in keys) | ||
| { | ||
| var lockName = $"Sql_lock_{key}"; |
There was a problem hiding this comment.
Don't modify the name, the caller should pass in unique names
| { | ||
| var lockName = $"Sql_lock_{key}"; | ||
| var locked = await connection.GetAppLockAsync(lockName, transaction, lockTimeout); | ||
| if (locked) |
There was a problem hiding this comment.
I'd write this differently so it's more clear what the expected flow is.
if (!locked)
{
allLocked = false;
break;
}
acquiredLocks.Add(lockName);
There was a problem hiding this comment.
We can achieve the intent of the code without using a list of acquiredLocks. Looking at a scenario where in vast majority of cases locks will go through on the first go creating and discarding a list of names of acquired locks is a bit of waste.
You only need to track the index of the last created lock and then to ReleaseAppLockAsync you can pass something like this
connection.ReleaseAppLockAsync(keys.Take(index+1), transaction);
This way you take only one int in most scenarios, and in those scenarios where you do need to release them you then spend some extra computing, but that should be rare.
| } | ||
| } | ||
|
|
||
| if (!allLocked) |
There was a problem hiding this comment.
I'd also rewrite this
if (allLocked)
break;
await connection.ReleaseAppLockAsync(....);
await Task.Delay(retryTimeout);
| } | ||
| } | ||
|
|
||
| return false; |
2 participants
Added new method TryGetAcquireLocks and override for ReleaseAppLock method to work with lists.