szetszwo commented on code in PR #10852:
URL: https://github.com/apache/ozone/pull/10852#discussion_r3641386211
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/lock/OzoneManagerLock.java:
##########
@@ -257,113 +382,52 @@ public OMLockDetails acquireWriteLocks(Resource
resource, Collection<String[]> k
*/
@Override
public OMLockDetails acquireResourceWriteLock(Resource resource) {
- return acquireLocks(resource, false, this::getAllLocks);
- }
-
- private void acquireLock(Resource resource, boolean isReadLock,
ReadWriteLock lock,
- long startWaitingTimeNanos) {
- if (isReadLock) {
- lock.readLock().lock();
- updateReadLockMetrics(resource, (ReentrantReadWriteLock) lock,
startWaitingTimeNanos);
- } else {
- lock.writeLock().lock();
- updateWriteLockMetrics(resource, (ReentrantReadWriteLock) lock,
startWaitingTimeNanos);
- }
+ return getResourceLocks(resource)
+ .acquire(resource, false, this::getAllLocks);
}
- private OMLockDetails acquireLocks(Resource resource, boolean isReadLock,
- Function<Striped<ReadWriteLock>, Iterable<ReadWriteLock>>
lockListProvider) {
- Pair<Map<Resource, Striped<ReadWriteLock>>, ResourceLockTracker>
resourceLockPair =
- resourcelockMap.get(resource.getClass());
- ResourceLockTracker<Resource> resourceLockTracker =
resourceLockPair.getRight();
- resourceLockTracker.clearLockDetails();
- if (!resourceLockTracker.canLockResource(resource)) {
- String errorMessage = getErrorMessage(resource);
- LOG.error(errorMessage);
- throw new RuntimeException(errorMessage);
- }
-
- long startWaitingTimeNanos = Time.monotonicNowNanos();
-
- for (ReadWriteLock lock :
lockListProvider.apply(resourceLockPair.getKey().get(resource))) {
- acquireLock(resource, isReadLock, lock, startWaitingTimeNanos);
- }
- return resourceLockTracker.lockResource(resource);
- }
-
- private OMLockDetails acquireLock(Resource resource, boolean isReadLock,
String... keys) {
- Pair<Map<Resource, Striped<ReadWriteLock>>, ResourceLockTracker>
resourceLockPair =
- resourcelockMap.get(resource.getClass());
- ResourceLockTracker<Resource> resourceLockTracker =
resourceLockPair.getRight();
- resourceLockTracker.clearLockDetails();
- if (!resourceLockTracker.canLockResource(resource)) {
- String errorMessage = getErrorMessage(resource);
- LOG.error(errorMessage);
- throw new RuntimeException(errorMessage);
- }
-
- long startWaitingTimeNanos = Time.monotonicNowNanos();
-
- ReentrantReadWriteLock lock = getLock(resourceLockPair.getKey(), resource,
keys);
- acquireLock(resource, isReadLock, lock, startWaitingTimeNanos);
- return resourceLockTracker.lockResource(resource);
- }
-
- private void updateReadLockMetrics(Resource resource,
- ReentrantReadWriteLock lock, long startWaitingTimeNanos) {
+ private void updateReadLockMetrics(ResourceLocks<?>.Context context,
ReentrantReadWriteLock lock) {
/*
* readHoldCount helps in metrics updation only once in case
* of reentrant locks.
*/
if (lock.getReadHoldCount() == 1) {
- long readLockWaitingTimeNanos =
- Time.monotonicNowNanos() - startWaitingTimeNanos;
+ final long readLockWaitingTimeNanos = context.getWaitingTimeNanos();
// Adds a snapshot to the metric readLockWaitingTimeMsStat.
omLockMetrics.setReadLockWaitingTimeMsStat(
TimeUnit.NANOSECONDS.toMillis(readLockWaitingTimeNanos));
-
updateProcessingDetails(resourcelockMap.get(resource.getClass()).getValue(),
+ updateProcessingDetails(context.getTracker(),
Timing.LOCKWAIT, readLockWaitingTimeNanos);
-
resource.getResourceManager().setStartReadHeldTimeNanos(Time.monotonicNowNanos());
+
context.getResource().getResourceManager().setStartReadHeldTimeNanos(Time.monotonicNowNanos());
}
}
- private void updateWriteLockMetrics(Resource resource,
- ReentrantReadWriteLock lock, long startWaitingTimeNanos) {
+ private void updateWriteLockMetrics(ResourceLocks<?>.Context context,
ReentrantReadWriteLock lock) {
/*
* writeHoldCount helps in metrics updation only once in case
* of reentrant locks. Metrics are updated only if the write lock is held
* by the current thread.
*/
if ((lock.getWriteHoldCount() == 1) &&
lock.isWriteLockedByCurrentThread()) {
- long writeLockWaitingTimeNanos =
- Time.monotonicNowNanos() - startWaitingTimeNanos;
+ long writeLockWaitingTimeNanos = context.getWaitingTimeNanos();
// Adds a snapshot to the metric writeLockWaitingTimeMsStat.
omLockMetrics.setWriteLockWaitingTimeMsStat(
TimeUnit.NANOSECONDS.toMillis(writeLockWaitingTimeNanos));
-
updateProcessingDetails(resourcelockMap.get(resource.getClass()).getValue(),
Timing.LOCKWAIT,
+ updateProcessingDetails(context.getTracker(), Timing.LOCKWAIT,
writeLockWaitingTimeNanos);
-
resource.getResourceManager().setStartWriteHeldTimeNanos(Time.monotonicNowNanos());
+
context.getResource().getResourceManager().setStartWriteHeldTimeNanos(Time.monotonicNowNanos());
}
}
- private String getErrorMessage(Resource resource) {
- return "Thread '" + Thread.currentThread().getName() + "' cannot " +
- "acquire " + resource.getName() + " lock while holding " +
- getCurrentLocks().toString() + " lock(s).";
- }
-
@VisibleForTesting
- List<String> getCurrentLocks() {
- return resourcelockMap.values().stream().map(Pair::getValue)
- .flatMap(rlm -> ((ResourceLockTracker<? extends
Resource>)rlm).getCurrentLockedResources())
- .map(Resource::getName)
- .collect(Collectors.toList());
+ int getCurrentLockSizeForTesting() {
Review Comment:
> Previously I think it would have given the list ...
No, the test currently never uses the List.
```java
+++
b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/lock/TestKeyPathLock.java
@@ -216,7 +216,7 @@ private void testDiffKeyPathWriteLockMultiThreadingUtil(
// Waiting for all the threads to be instantiated/to reach
// acquireWriteLock.
countDown.countDown();
- assertEquals(1, lock.getCurrentLocks().size());
+ assertEquals(1, lock.getCurrentLockSizeForTesting());
```
--
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]
