awslabs / amazon-dynamodb-lock-client

The AmazonDynamoDBLockClient is a general purpose distributed locking library built on top of DynamoDB. It supports both coarse-grained and fine-grained locking.
Other
472 stars 85 forks source link

Potential Bug - Client should reset leaseDuration waiting time when recordVersionNumber noticed to change #94

Open basilmusa opened 10 months ago

basilmusa commented 10 months ago

In the below source code in acquireLock method (source code link)

It is noticed that the else condition notices that the UUID changed in recordVersionNumber but it never resets the waiting period in variable millisecondsToWait.

This could have negative impact since the acquireLock is now waiting on something that it knows it will never acquire (unless ofcourse isReleased in DDB is set to true). So I think a fix should be applied here to extend the duration the acquire lock time has to wait by increasing the millisecondsToWait to allow the method to wait for the actual leaseDuration again by resetting it.

p.s. I work at Amazon, you can contact me on my username basabbas to discuss.

                        if (lockTryingToBeAcquired.getRecordVersionNumber().equals(existingLock.get().getRecordVersionNumber())) {
                            /* If the version numbers match, then we can acquire the lock, assuming it has already expired */
                            if (lockTryingToBeAcquired.isExpired()) {
                                return upsertAndMonitorExpiredLock(options, key, sortKey, deleteLockOnRelease, sessionMonitor, existingLock, newLockData, item,
                                    recordVersionNumber);
                            }
                        } else {
                            /*
                             * If the version number changed since we last queried the lock, then we need to update
                             * lockTryingToBeAcquired as the lock has been refreshed since we last checked
                             */
                            lockTryingToBeAcquired = existingLock.get();
                        }