spacemonkd commented on code in PR #10852:
URL: https://github.com/apache/ozone/pull/10852#discussion_r3641023874
##########
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:
This simplifies the method, but then in tests we will only see the count.
For example it might be something like: expected 1 but got 2.
Previously I think it would have given the list of the locks as well to help
debug any test issues. This is removing that ability. Do you think it might be
good to return the locks that are held as well to debug later on?
--
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]
