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

Add independent lock #3247

Open
liran2000 opened this issue Mar 25, 2024 · 5 comments
Open

Add independent lock #3247

liran2000 opened this issue Mar 25, 2024 · 5 comments

Comments

@liran2000
Copy link

Description

Add a lock class, not depends on leader election.

Usage high level guideline:

try {
  lock.lock(timeout); // blocking
  doSomeAction();
} finally {
  lock.unlock();
}

Use cases

Actions which require distributed lock.

Solution suggestion

implementation details - high level

  • lock:
    • try to creates a lease.
    • if already exists, wait and try again until created, or timeout reached.
  • unlock:
    • delete the lease.

Note that I have this implementation ready, so if confirmed, I can try opening a PR for it at extended module.
Would appreciate feedback on proceeding with it.

@brendandburns
Copy link
Contributor

What is the purpose of this class? We have several implementations of Lock already:

https://github.com/kubernetes-client/java/blob/master/extended/src/main/java/io/kubernetes/client/extended/leaderelection/resourcelock/LeaseLock.java

@liran2000
Copy link
Author

Actions which require a distributed lock. How can lock/unlock usage be achieved with existing lock ?
Keep in mind this functionality commonly needed without leader election.

try {
  lock.lock(timeout); // blocking
  doSomeAction();
} finally {
  lock.unlock();
}

@brendandburns
Copy link
Contributor

Is it that you want to be able to unlock()? If that is the case, I'd prefer that you added that to the existing leader election implementation vs adding a completely new implementation of locking/leader election.

There's not much difference between locking and leader election (except perhaps the unlock part) so I'd prefer to extend the existing implementations.

@liran2000
Copy link
Author

Existing LeaseLock implements io.kubernetes.client.extended.leaderelection.Lock, which is "strictly for use by the leaderelection code". Adding to it can feel like abuse.
I am thinking of implementing java.util.concurrent.locks.Lock, as encountered a need for it, for common usage.
What do you think ?

Lock lock = ...;
 if (lock.tryLock()) {
   try {
     // manipulate protected state
   } finally {
     lock.unlock();
   }
 } else {
   // perform alternative actions
 }

@brendandburns
Copy link
Contributor

I think it's definitely fine to implement java.util.concurrent.locks.Lock but I believe that you can implement that using the LeaderElector class, you don't have to implement the lock acquire/retain/release code yourself.

That code is tricky to get right and we're better off focusing on a single implementation.

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

No branches or pull requests

2 participants