Introduce lock acquisition timeout #8062
Conversation
MishaDemianenko
force-pushed
the
3.1-terminate-long-locks-acquisitions
branch
from
39ed26f
to
09c65da
Compare
|
@lutovich any news? |
There was a problem hiding this comment.
@MishaDemianenko sorry, I kind of forgot about this PR.
Changes look good, added one question/suggestion.
Also couple more general points:
- please make newly introduced fields in
LockManagerImpl,RWLock,ForsetiClientandForsetiLockManagerfinal - would be really nice to have ITs for lock acquisition timeout involving community/forseti, shared/exclusive and master/slave. I understand, they might be flaky or long running because of need to rely on time but still it is better than nothing.
| * Used in lock clients for cases when we unable to acquire a lock for a time that exceed configured | ||
| * timeout, if any. | ||
| * | ||
| * @see Locks.Client |
There was a problem hiding this comment.
Maybe also @see GraphDatabaseSettings#lock_acquisition_timeout
| wait(); | ||
| if ( lockAcquisitionTimeoutMillis > 0 ) | ||
| { | ||
| wait( Math.abs( lockAcquisitionTimeBoundary - clock.millis() ) ); |
There was a problem hiding this comment.
If clock.millis() >>> lockAcquisitionTimeBoundary due to thread scheduling or smth, thread will still wait for a while and then throw exception. Would it make sense to make previous condition
if (lockAcquisitionTimeoutMillis > 0 && lockAcquisitionTimeBoundary > currentMillis)
and then just wait(lockAcquisitionTimeBoundary-currentMillis) ?
MishaDemianenko
force-pushed
the
3.1-terminate-long-locks-acquisitions
branch
from
09c65da
to
f708f7d
Compare
Introduce property that defines timeout for shared and exclusive lock acquisition in lock clients. In case if user configure timeout any acquisitions that take more then configured time interval will be terminated and LockAcquisitionTimeoutException will be raised. Add support of new property to community and forseti lock managers. Unify clocks usage during database construction/creation: now all components will use exactly the same instance of clock that provided by PlatformModule.
MishaDemianenko
force-pushed
the
3.1-terminate-long-locks-acquisitions
branch
from
b572d73
to
c0e43ad
Compare
… shared locks for community and forseti lock managers.
|
@lutovich additional tests introduced, can you please take a look now! |
| public void tearDown() | ||
| { | ||
| secondTransactionExecutor.close(); | ||
| clockExecutor.close(); |
There was a problem hiding this comment.
Should database.shutdown() be here as well?
There was a problem hiding this comment.
@lutovich absolutely, we need to shut it down; updated, thx!
Introduce property that defines timeout for shared and exclusive lock acquisition in lock clients.
In case if user configure timeout any acquisitions that take more then configured time interval will be terminated and LockAcquisitionTimeoutException will be raised.
Add support of new property to community and forseti lock managers.
Unify clocks usage during database construction/creation: now all components will use exactly the same instance of clock that provided by PlatformModule.