CODEWITHSUNDEEP
javaapilocking
flash
flash

Reputation: 1229

Lock waiting in an API should it be exposed

I am providing an api to do some crud operations, which inturn uses a existing internal implementation, what I have implemented additionally is a lock mechanism , which waits/retries definitely for a application level lock to do these operations, if it cannot acquire in some retries , it throws a custom Exception with the error text . During code review I was told that this waiting should not be done, instead the user of the api , should handle this.

should I remove this lock waiting from the Implementation?

        lockManager = new LockManager();
        aquiredLock = lockManager.aquireLock();
        final int numberOfRetries = 3;
        final int sleepTime = 1000;
        int retryAttempt = 0;
        while (!aquiredLock.isLockAquired() && retryAttempt < numberOfRetries) {
            retryAttempt++;
            try {
                Thread.sleep(sleepTime);
            } catch (InterruptedException e) {
                throw new CannotAcquireLockException(e.getMessage(), e);
            }
            aquiredLock = lockManager.aquireLock();
        }

Regards, Goutham

Upvotes: 0

Views: 200

Answers (2)

jolivier
jolivier

Reputation: 7635

Its your choice, depending on what API you whant to provide, for me there is this alternative

  • You leave the lock waiting mechanism in your method. Benefits: easier for the user since it does not have to care about this lock problem. Drawback: the API user cannot act to timeout or retry himself.

  • If you just try to acquire the lock and throw if you could not. Benefits: the user can decide itself for a retry/timeout strategy.

In both case you have to document your choice in your API. What is important in the review process is that it can tell you if this choice is consistent with other parts of the API (other methods regarding the same locks), and if this lock is really needed.

Side note: in our application (database engine), we provide the API to acquire/release the locks and tests in API calls that the locks are indeed held.

Upvotes: 1

Marko Topolnik
Marko Topolnik

Reputation: 200148

This kind of thing cannot be decided in general. Your code reviewers probably know a lot about your problem domain, so their advice may well be justified.

From a general perspective I can only say that the kind of code you are showing would definitely not be the default/obvious way to do it and would be called for only when it is clear from the circumstances that this approach is indeed necessary.

Upvotes: 3

Related Questions