sarkaramrit1993 commented on code in PR #5161:
URL: https://github.com/apache/solr/pull/5161#discussion_r4356391362
##########
solr/core/src/java/org/apache/solr/cloud/ZkDistributedLockFactory.java:
##########
@@ -37,16 +39,15 @@ abstract class ZkDistributedLockFactory {
protected DistributedLock doCreateLock(
boolean isWriteLock, String lockPath, String lockIdToMirror) {
try {
- if (StrUtils.isBlank(lockIdToMirror)) {
- // TODO optimize by first attempting to create the ZkDistributedLock
without calling
- // makeLockPath() and only call it if the lock creation fails. This
will be less costly on
- // high contention (and slightly more on low contention)
- makeLockPath(lockPath);
+ for (int attempt = 1; attempt < MAX_CREATE_ATTEMPTS; attempt++) {
+ try {
+ return newLock(isWriteLock, lockPath, lockIdToMirror);
+ } catch (KeeperException.NoNodeException e) {
+ // Lock dirs are containers, so ZK drops them when empty. Recreate
and try again.
+ makeLockPath(lockPath);
+ }
}
-
- return isWriteLock
- ? new ZkDistributedLock.Write(zkClient, lockPath, lockIdToMirror)
- : new ZkDistributedLock.Read(zkClient, lockPath, lockIdToMirror);
+ return newLock(isWriteLock, lockPath, lockIdToMirror);
Review Comment:
Done in c935397fa19. The condition stays empty because the catch bounds it,
and `testLockCreationGivesUpAfterMaxAttempts` pins the attempt count.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
