You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by GitBox <gi...@apache.org> on 2022/04/05 10:11:00 UTC
[GitHub] [ozone] tanvipenumudy opened a new pull request, #3271: HDDS-6436. Add write lock waiting and held time metrics
tanvipenumudy opened a new pull request, #3271:
URL: https://github.com/apache/ozone/pull/3271
## What changes were proposed in this pull request?
To provide useful information by exposing write lock waiting, held time metrics to understand OzoneManagerLock timestamp
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-6436
## How was this patch tested?
Added UTs
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] tanvipenumudy commented on a diff in pull request #3271: HDDS-6436. Add write lock waiting and held time metrics
Posted by GitBox <gi...@apache.org>.
tanvipenumudy commented on code in PR #3271:
URL: https://github.com/apache/ozone/pull/3271#discussion_r844637577
##########
hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/lock/TestOzoneManagerLock.java:
##########
@@ -426,8 +444,58 @@ public void testReadLockConcurrentStats() throws InterruptedException {
String readHeldStat = lock.getReadLockHeldTimeMsStat();
Assert.assertTrue(
- "Expected " + threadCount + " samples in " + readHeldStat,
+ "Expected " + threadCount +
+ " samples in readLockHeldTimeMsStat: " + readHeldStat,
readHeldStat.contains("Samples = " + threadCount));
+
+ String readWaitingStat = lock.getReadLockWaitingTimeMsStat();
+ Assert.assertTrue(
+ "Expected " + threadCount +
+ " samples in readLockWaitingTimeMsStat: " + readWaitingStat,
+ readWaitingStat.contains("Samples = " + threadCount));
+ }
+ }
+
+ @Test
+ public void testWriteLockConcurrentStats() throws InterruptedException {
Review Comment:
Thank you for the comment, added `testSyntheticReadWriteLockConcurrentStats()` to produce a mix of multiple readers + writers.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] rakeshadr commented on a diff in pull request #3271: HDDS-6436. Add write lock waiting and held time metrics
Posted by GitBox <gi...@apache.org>.
rakeshadr commented on code in PR #3271:
URL: https://github.com/apache/ozone/pull/3271#discussion_r843471208
##########
hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/lock/OzoneManagerLock.java:
##########
@@ -388,15 +405,22 @@ private void unlock(Resource resource, String resourceName,
// TODO: Not checking release of higher order level lock happened while
// releasing lower order level lock, as for that we need counter for
// locks, as some locks support acquiring lock again.
+ boolean isWriteLocked = manager.isWriteLockedByCurrentThread(resourceName);
Review Comment:
Please move
`boolean isWriteLocked = manager.isWriteLockedByCurrentThread(resourceName);`
to the above TODO comment because thats talking about the lockFn.accept() stetment.
##########
hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/lock/TestOzoneManagerLock.java:
##########
@@ -426,8 +444,58 @@ public void testReadLockConcurrentStats() throws InterruptedException {
String readHeldStat = lock.getReadLockHeldTimeMsStat();
Assert.assertTrue(
- "Expected " + threadCount + " samples in " + readHeldStat,
+ "Expected " + threadCount +
+ " samples in readLockHeldTimeMsStat: " + readHeldStat,
readHeldStat.contains("Samples = " + threadCount));
+
+ String readWaitingStat = lock.getReadLockWaitingTimeMsStat();
+ Assert.assertTrue(
+ "Expected " + threadCount +
+ " samples in readLockWaitingTimeMsStat: " + readWaitingStat,
+ readWaitingStat.contains("Samples = " + threadCount));
+ }
+ }
+
+ @Test
+ public void testWriteLockConcurrentStats() throws InterruptedException {
Review Comment:
@tanvipenumudy Can you add a test case for mix of (synthetic) readers/writers. Here you can have more readers than writers. For ex: 3 writers and 10 readers.
`public void testSyntheticReadWriteLockConcurrentStats()`
##########
hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/lock/OzoneManagerLock.java:
##########
@@ -195,18 +202,28 @@ private boolean lock(Resource resource, String resourceName,
}
}
- private void updateReadLockMetrics(Resource resource, String lockType,
+ private void updateReadLockMetrics(Resource resource,
long startWaitingTimeNanos) {
- if (lockType.equals(READ_LOCK)) {
- long readLockWaitingTimeNanos =
- Time.monotonicNowNanos() - startWaitingTimeNanos;
+ long readLockWaitingTimeNanos =
+ Time.monotonicNowNanos() - startWaitingTimeNanos;
- // Adds a snapshot to the metric readLockWaitingTimeMsStat.
- omLockMetrics.setReadLockWaitingTimeMsStat(
- TimeUnit.NANOSECONDS.toMillis(readLockWaitingTimeNanos));
+ // Adds a snapshot to the metric readLockWaitingTimeMsStat.
+ omLockMetrics.setReadLockWaitingTimeMsStat(
+ TimeUnit.NANOSECONDS.toMillis(readLockWaitingTimeNanos));
- resource.setStartHeldTimeNanos(Time.monotonicNowNanos());
- }
+ resource.setStartHeldTimeNanos(Time.monotonicNowNanos());
+ }
+
+ private void updateWriteLockMetrics(Resource resource,
+ long startWaitingTimeNanos) {
+ long writeLockWaitingTimeNanos =
+ Time.monotonicNowNanos() - startWaitingTimeNanos;
+
+ // Adds a snapshot to the metric writeLockWaitingTimeMsStat.
+ omLockMetrics.setWriteLockWaitingTimeMsStat(
+ TimeUnit.NANOSECONDS.toMillis(writeLockWaitingTimeNanos));
+
+ resource.setStartHeldTimeNanos(Time.monotonicNowNanos());
Review Comment:
`resource.setStartHeldTimeNanos(Time.monotonicNowNanos());`
```
OzoneManagerLock.java
void setStartHeldTimeNanos(long startHeldTimeNanos) {
readLockTimeStampNanos.get().setStartHeldTimeNanos(startHeldTimeNanos);
}
```
Its overwriting the held time for the read metrics. Please create a separate var `writeLockTimeStampNanos`. Since we have plan to allow multiple writers eventually(long term gaol) on disjoint OBS/FSO paths, we need to use `ThreadLocal<LockUsageInfo> writeLockTimeStampNanos`, so that we keep track of multiple writer threads stats.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] tanvipenumudy commented on a diff in pull request #3271: HDDS-6436. Add write lock waiting and held time metrics
Posted by GitBox <gi...@apache.org>.
tanvipenumudy commented on code in PR #3271:
URL: https://github.com/apache/ozone/pull/3271#discussion_r845251129
##########
hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/lock/TestOzoneManagerLock.java:
##########
@@ -393,41 +393,177 @@ private void testReadLockHoldCountUtil(OzoneManagerLock.Resource resource,
lock.releaseReadLock(resource, resourceName);
assertEquals(0, lock.getReadHoldCount(resourceLockName));
+
+ Assert.assertFalse(lock.isWriteLockedByCurrentThread(resourceLockName));
+ assertEquals(0, lock.getWriteHoldCount(resourceLockName));
+ lock.acquireWriteLock(resource, resourceName);
+ Assert.assertTrue(lock.isWriteLockedByCurrentThread(resourceLockName));
+ assertEquals(1, lock.getWriteHoldCount(resourceLockName));
+
+ lock.acquireWriteLock(resource, resourceName);
+ Assert.assertTrue(lock.isWriteLockedByCurrentThread(resourceLockName));
+ assertEquals(2, lock.getWriteHoldCount(resourceLockName));
+
+ lock.releaseWriteLock(resource, resourceName);
+ Assert.assertTrue(lock.isWriteLockedByCurrentThread(resourceLockName));
+ assertEquals(1, lock.getWriteHoldCount(resourceLockName));
+
+ lock.releaseWriteLock(resource, resourceName);
+ Assert.assertFalse(lock.isWriteLockedByCurrentThread(resourceLockName));
+ assertEquals(0, lock.getWriteHoldCount(resourceLockName));
}
@Test
- public void testReadLockConcurrentStats() throws InterruptedException {
+ public void testLockConcurrentStats() throws InterruptedException {
String[] resourceName;
for (OzoneManagerLock.Resource resource :
OzoneManagerLock.Resource.values()) {
resourceName = generateResourceName(resource);
+ testReadLockConcurrentStats(resource, resourceName, 10);
+ testWriteLockConcurrentStats(resource, resourceName, 5);
+ testSyntheticReadWriteLockConcurrentStats(resource, resourceName, 10, 3);
+ }
+ }
- OzoneManagerLock lock = new OzoneManagerLock(new OzoneConfiguration());
- final int threadCount = 10;
- Thread[] threads = new Thread[threadCount];
- for (int i = 0; i < threads.length; i++) {
- String[] finalResourceName = resourceName;
- threads[i] = new Thread(() -> {
- lock.acquireReadLock(resource, finalResourceName);
- try {
- Thread.sleep(1000);
- } catch (InterruptedException e) {
- e.printStackTrace();
- }
- lock.releaseReadLock(resource, finalResourceName);
- });
- threads[i].start();
- }
+ public void testReadLockConcurrentStats(OzoneManagerLock.Resource resource,
+ String[] resourceName,
+ int threadCount)
+ throws InterruptedException {
+ OzoneManagerLock lock = new OzoneManagerLock(new OzoneConfiguration());
+ Thread[] threads = new Thread[threadCount];
+
+ for (int i = 0; i < threads.length; i++) {
+ threads[i] = new Thread(() -> {
+ lock.acquireReadLock(resource, resourceName);
+ try {
+ Thread.sleep(1000);
+ } catch (InterruptedException e) {
+ e.printStackTrace();
+ }
+ lock.releaseReadLock(resource, resourceName);
+ });
+ threads[i].start();
+ }
- for (Thread t : threads) {
- t.join();
- }
+ for (Thread t : threads) {
+ t.join();
+ }
+
+ String readHeldStat = lock.getReadLockHeldTimeMsStat();
+ Assert.assertTrue(
+ "Expected " + threadCount +
+ " samples in readLockHeldTimeMsStat: " + readHeldStat,
+ readHeldStat.contains("Samples = " + threadCount));
+
+ String readWaitingStat = lock.getReadLockWaitingTimeMsStat();
+ Assert.assertTrue(
+ "Expected " + threadCount +
+ " samples in readLockWaitingTimeMsStat: " + readWaitingStat,
+ readWaitingStat.contains("Samples = " + threadCount));
+ }
+
+ public void testWriteLockConcurrentStats(OzoneManagerLock.Resource resource,
+ String[] resourceName,
+ int threadCount)
+ throws InterruptedException {
+ OzoneManagerLock lock = new OzoneManagerLock(new OzoneConfiguration());
+ Thread[] threads = new Thread[threadCount];
- String readHeldStat = lock.getReadLockHeldTimeMsStat();
- Assert.assertTrue(
- "Expected " + threadCount + " samples in " + readHeldStat,
- readHeldStat.contains("Samples = " + threadCount));
+ for (int i = 0; i < threads.length; i++) {
+ threads[i] = new Thread(() -> {
+ lock.acquireWriteLock(resource, resourceName);
+ try {
+ Thread.sleep(500);
+ } catch (InterruptedException e) {
+ e.printStackTrace();
+ }
+ lock.releaseWriteLock(resource, resourceName);
+ });
+ threads[i].start();
+ }
+
+ for (Thread t : threads) {
+ t.join();
+ }
+
+ String writeHeldStat = lock.getWriteLockHeldTimeMsStat();
+ Assert.assertTrue(
+ "Expected " + threadCount +
+ " samples in writeLockHeldTimeMsStat: " + writeHeldStat,
+ writeHeldStat.contains("Samples = " + threadCount));
+
+ String writeWaitingStat = lock.getWriteLockWaitingTimeMsStat();
+ Assert.assertTrue(
+ "Expected " + threadCount +
+ " samples in writeLockWaitingTimeMsStat" + writeWaitingStat,
+ writeWaitingStat.contains("Samples = " + threadCount));
+ }
+
+ public void testSyntheticReadWriteLockConcurrentStats(
+ OzoneManagerLock.Resource resource, String[] resourceName,
+ int readThreadCount, int writeThreadCount)
+ throws InterruptedException {
+ OzoneManagerLock lock = new OzoneManagerLock(new OzoneConfiguration());
+ Thread[] readThreads = new Thread[readThreadCount];
+ Thread[] writeThreads = new Thread[writeThreadCount];
+
+ for (int i = 0; i < readThreads.length; i++) {
+ readThreads[i] = new Thread(() -> {
+ lock.acquireReadLock(resource, resourceName);
+ try {
+ Thread.sleep(1000);
+ } catch (InterruptedException e) {
+ e.printStackTrace();
+ }
+ lock.releaseReadLock(resource, resourceName);
+ });
+ readThreads[i].start();
}
+
+ for (int i = 0; i < writeThreads.length; i++) {
+ writeThreads[i] = new Thread(() -> {
+ lock.acquireWriteLock(resource, resourceName);
+ try {
+ Thread.sleep(500);
+ } catch (InterruptedException e) {
+ e.printStackTrace();
+ }
+ lock.releaseWriteLock(resource, resourceName);
+ });
+ writeThreads[i].start();
Review Comment:
Thank you for the suggestion, I have added the same
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] rakeshadr commented on a diff in pull request #3271: HDDS-6436. Add write lock waiting and held time metrics
Posted by GitBox <gi...@apache.org>.
rakeshadr commented on code in PR #3271:
URL: https://github.com/apache/ozone/pull/3271#discussion_r845053091
##########
hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/lock/TestOzoneManagerLock.java:
##########
@@ -393,41 +393,177 @@ private void testReadLockHoldCountUtil(OzoneManagerLock.Resource resource,
lock.releaseReadLock(resource, resourceName);
assertEquals(0, lock.getReadHoldCount(resourceLockName));
+
+ Assert.assertFalse(lock.isWriteLockedByCurrentThread(resourceLockName));
+ assertEquals(0, lock.getWriteHoldCount(resourceLockName));
+ lock.acquireWriteLock(resource, resourceName);
+ Assert.assertTrue(lock.isWriteLockedByCurrentThread(resourceLockName));
+ assertEquals(1, lock.getWriteHoldCount(resourceLockName));
+
+ lock.acquireWriteLock(resource, resourceName);
+ Assert.assertTrue(lock.isWriteLockedByCurrentThread(resourceLockName));
+ assertEquals(2, lock.getWriteHoldCount(resourceLockName));
+
+ lock.releaseWriteLock(resource, resourceName);
+ Assert.assertTrue(lock.isWriteLockedByCurrentThread(resourceLockName));
+ assertEquals(1, lock.getWriteHoldCount(resourceLockName));
+
+ lock.releaseWriteLock(resource, resourceName);
+ Assert.assertFalse(lock.isWriteLockedByCurrentThread(resourceLockName));
+ assertEquals(0, lock.getWriteHoldCount(resourceLockName));
}
@Test
- public void testReadLockConcurrentStats() throws InterruptedException {
+ public void testLockConcurrentStats() throws InterruptedException {
String[] resourceName;
for (OzoneManagerLock.Resource resource :
OzoneManagerLock.Resource.values()) {
resourceName = generateResourceName(resource);
+ testReadLockConcurrentStats(resource, resourceName, 10);
+ testWriteLockConcurrentStats(resource, resourceName, 5);
+ testSyntheticReadWriteLockConcurrentStats(resource, resourceName, 10, 3);
+ }
+ }
- OzoneManagerLock lock = new OzoneManagerLock(new OzoneConfiguration());
- final int threadCount = 10;
- Thread[] threads = new Thread[threadCount];
- for (int i = 0; i < threads.length; i++) {
- String[] finalResourceName = resourceName;
- threads[i] = new Thread(() -> {
- lock.acquireReadLock(resource, finalResourceName);
- try {
- Thread.sleep(1000);
- } catch (InterruptedException e) {
- e.printStackTrace();
- }
- lock.releaseReadLock(resource, finalResourceName);
- });
- threads[i].start();
- }
+ public void testReadLockConcurrentStats(OzoneManagerLock.Resource resource,
+ String[] resourceName,
+ int threadCount)
+ throws InterruptedException {
+ OzoneManagerLock lock = new OzoneManagerLock(new OzoneConfiguration());
+ Thread[] threads = new Thread[threadCount];
+
+ for (int i = 0; i < threads.length; i++) {
+ threads[i] = new Thread(() -> {
+ lock.acquireReadLock(resource, resourceName);
+ try {
+ Thread.sleep(1000);
+ } catch (InterruptedException e) {
+ e.printStackTrace();
+ }
+ lock.releaseReadLock(resource, resourceName);
+ });
+ threads[i].start();
+ }
- for (Thread t : threads) {
- t.join();
- }
+ for (Thread t : threads) {
+ t.join();
+ }
+
+ String readHeldStat = lock.getReadLockHeldTimeMsStat();
+ Assert.assertTrue(
+ "Expected " + threadCount +
+ " samples in readLockHeldTimeMsStat: " + readHeldStat,
+ readHeldStat.contains("Samples = " + threadCount));
+
+ String readWaitingStat = lock.getReadLockWaitingTimeMsStat();
+ Assert.assertTrue(
+ "Expected " + threadCount +
+ " samples in readLockWaitingTimeMsStat: " + readWaitingStat,
+ readWaitingStat.contains("Samples = " + threadCount));
+ }
+
+ public void testWriteLockConcurrentStats(OzoneManagerLock.Resource resource,
+ String[] resourceName,
+ int threadCount)
+ throws InterruptedException {
+ OzoneManagerLock lock = new OzoneManagerLock(new OzoneConfiguration());
+ Thread[] threads = new Thread[threadCount];
- String readHeldStat = lock.getReadLockHeldTimeMsStat();
- Assert.assertTrue(
- "Expected " + threadCount + " samples in " + readHeldStat,
- readHeldStat.contains("Samples = " + threadCount));
+ for (int i = 0; i < threads.length; i++) {
+ threads[i] = new Thread(() -> {
+ lock.acquireWriteLock(resource, resourceName);
+ try {
+ Thread.sleep(500);
+ } catch (InterruptedException e) {
+ e.printStackTrace();
+ }
+ lock.releaseWriteLock(resource, resourceName);
+ });
+ threads[i].start();
+ }
+
+ for (Thread t : threads) {
+ t.join();
+ }
+
+ String writeHeldStat = lock.getWriteLockHeldTimeMsStat();
+ Assert.assertTrue(
+ "Expected " + threadCount +
+ " samples in writeLockHeldTimeMsStat: " + writeHeldStat,
+ writeHeldStat.contains("Samples = " + threadCount));
+
+ String writeWaitingStat = lock.getWriteLockWaitingTimeMsStat();
+ Assert.assertTrue(
+ "Expected " + threadCount +
+ " samples in writeLockWaitingTimeMsStat" + writeWaitingStat,
+ writeWaitingStat.contains("Samples = " + threadCount));
+ }
+
+ public void testSyntheticReadWriteLockConcurrentStats(
+ OzoneManagerLock.Resource resource, String[] resourceName,
+ int readThreadCount, int writeThreadCount)
+ throws InterruptedException {
+ OzoneManagerLock lock = new OzoneManagerLock(new OzoneConfiguration());
+ Thread[] readThreads = new Thread[readThreadCount];
+ Thread[] writeThreads = new Thread[writeThreadCount];
+
+ for (int i = 0; i < readThreads.length; i++) {
+ readThreads[i] = new Thread(() -> {
+ lock.acquireReadLock(resource, resourceName);
+ try {
+ Thread.sleep(1000);
+ } catch (InterruptedException e) {
+ e.printStackTrace();
+ }
+ lock.releaseReadLock(resource, resourceName);
+ });
+ readThreads[i].start();
Review Comment:
Please name the thread so that we could differentiate it in mix workload.
`readThreads[i].setName("ReadLockThread-" + i);`
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] rakeshadr commented on a diff in pull request #3271: HDDS-6436. Add write lock waiting and held time metrics
Posted by GitBox <gi...@apache.org>.
rakeshadr commented on code in PR #3271:
URL: https://github.com/apache/ozone/pull/3271#discussion_r845052294
##########
hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/lock/TestOzoneManagerLock.java:
##########
@@ -393,41 +393,177 @@ private void testReadLockHoldCountUtil(OzoneManagerLock.Resource resource,
lock.releaseReadLock(resource, resourceName);
assertEquals(0, lock.getReadHoldCount(resourceLockName));
+
+ Assert.assertFalse(lock.isWriteLockedByCurrentThread(resourceLockName));
+ assertEquals(0, lock.getWriteHoldCount(resourceLockName));
+ lock.acquireWriteLock(resource, resourceName);
+ Assert.assertTrue(lock.isWriteLockedByCurrentThread(resourceLockName));
+ assertEquals(1, lock.getWriteHoldCount(resourceLockName));
+
+ lock.acquireWriteLock(resource, resourceName);
+ Assert.assertTrue(lock.isWriteLockedByCurrentThread(resourceLockName));
+ assertEquals(2, lock.getWriteHoldCount(resourceLockName));
+
+ lock.releaseWriteLock(resource, resourceName);
+ Assert.assertTrue(lock.isWriteLockedByCurrentThread(resourceLockName));
+ assertEquals(1, lock.getWriteHoldCount(resourceLockName));
+
+ lock.releaseWriteLock(resource, resourceName);
+ Assert.assertFalse(lock.isWriteLockedByCurrentThread(resourceLockName));
+ assertEquals(0, lock.getWriteHoldCount(resourceLockName));
}
@Test
- public void testReadLockConcurrentStats() throws InterruptedException {
+ public void testLockConcurrentStats() throws InterruptedException {
String[] resourceName;
for (OzoneManagerLock.Resource resource :
OzoneManagerLock.Resource.values()) {
resourceName = generateResourceName(resource);
+ testReadLockConcurrentStats(resource, resourceName, 10);
+ testWriteLockConcurrentStats(resource, resourceName, 5);
+ testSyntheticReadWriteLockConcurrentStats(resource, resourceName, 10, 3);
+ }
+ }
- OzoneManagerLock lock = new OzoneManagerLock(new OzoneConfiguration());
- final int threadCount = 10;
- Thread[] threads = new Thread[threadCount];
- for (int i = 0; i < threads.length; i++) {
- String[] finalResourceName = resourceName;
- threads[i] = new Thread(() -> {
- lock.acquireReadLock(resource, finalResourceName);
- try {
- Thread.sleep(1000);
- } catch (InterruptedException e) {
- e.printStackTrace();
- }
- lock.releaseReadLock(resource, finalResourceName);
- });
- threads[i].start();
- }
+ public void testReadLockConcurrentStats(OzoneManagerLock.Resource resource,
+ String[] resourceName,
+ int threadCount)
+ throws InterruptedException {
+ OzoneManagerLock lock = new OzoneManagerLock(new OzoneConfiguration());
+ Thread[] threads = new Thread[threadCount];
+
+ for (int i = 0; i < threads.length; i++) {
+ threads[i] = new Thread(() -> {
+ lock.acquireReadLock(resource, resourceName);
+ try {
+ Thread.sleep(1000);
+ } catch (InterruptedException e) {
+ e.printStackTrace();
+ }
+ lock.releaseReadLock(resource, resourceName);
+ });
+ threads[i].start();
+ }
- for (Thread t : threads) {
- t.join();
- }
+ for (Thread t : threads) {
+ t.join();
+ }
+
+ String readHeldStat = lock.getReadLockHeldTimeMsStat();
+ Assert.assertTrue(
+ "Expected " + threadCount +
+ " samples in readLockHeldTimeMsStat: " + readHeldStat,
+ readHeldStat.contains("Samples = " + threadCount));
+
+ String readWaitingStat = lock.getReadLockWaitingTimeMsStat();
+ Assert.assertTrue(
+ "Expected " + threadCount +
+ " samples in readLockWaitingTimeMsStat: " + readWaitingStat,
+ readWaitingStat.contains("Samples = " + threadCount));
+ }
+
+ public void testWriteLockConcurrentStats(OzoneManagerLock.Resource resource,
+ String[] resourceName,
+ int threadCount)
+ throws InterruptedException {
+ OzoneManagerLock lock = new OzoneManagerLock(new OzoneConfiguration());
+ Thread[] threads = new Thread[threadCount];
- String readHeldStat = lock.getReadLockHeldTimeMsStat();
- Assert.assertTrue(
- "Expected " + threadCount + " samples in " + readHeldStat,
- readHeldStat.contains("Samples = " + threadCount));
+ for (int i = 0; i < threads.length; i++) {
+ threads[i] = new Thread(() -> {
+ lock.acquireWriteLock(resource, resourceName);
+ try {
+ Thread.sleep(500);
Review Comment:
Can you reduce sleep time to run the test quickly .
write Thread sleeping time -> Thread.sleep(100);
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] tanvipenumudy commented on a diff in pull request #3271: HDDS-6436. Add write lock waiting and held time metrics
Posted by GitBox <gi...@apache.org>.
tanvipenumudy commented on code in PR #3271:
URL: https://github.com/apache/ozone/pull/3271#discussion_r845796475
##########
hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/lock/OzoneManagerLock.java:
##########
@@ -514,29 +615,56 @@ public void cleanup() {
// Name of the resource.
private String name;
- // This helps in maintaining lock related variables locally confined to a
- // given thread.
+ // This helps in maintaining read lock related variables locally confined
+ // to a given thread.
private final ThreadLocal<LockUsageInfo> readLockTimeStampNanos =
ThreadLocal.withInitial(LockUsageInfo::new);
+ // This helps in maintaining write lock related variables locally confined
+ // to a given thread.
+ private final ThreadLocal<LockUsageInfo> writeLockTimeStampNanos =
+ ThreadLocal.withInitial(LockUsageInfo::new);
+
/**
- * Sets the time (ns) when the lock holding period begins specific to a
+ * Sets the time (ns) when the read lock holding period begins specific to a
* thread.
*
- * @param startHeldTimeNanos lock held start time (ns)
+ * @param startReadHeldTimeNanos read lock held start time (ns)
*/
- void setStartHeldTimeNanos(long startHeldTimeNanos) {
- readLockTimeStampNanos.get().setStartHeldTimeNanos(startHeldTimeNanos);
+ void setStartReadHeldTimeNanos(long startReadHeldTimeNanos) {
+ readLockTimeStampNanos.get()
+ .setStartReadHeldTimeNanos(startReadHeldTimeNanos);
}
/**
- * Returns the time (ns) when the lock holding period began specific to a
- * thread.
+ * Sets the time (ns) when the write lock holding period begins specific to
+ * a thread.
+ *
+ * @param startWriteHeldTimeNanos write lock held start time (ns)
+ */
+ void setStartWriteHeldTimeNanos(long startWriteHeldTimeNanos) {
+ writeLockTimeStampNanos.get()
+ .setStartWriteHeldTimeNanos(startWriteHeldTimeNanos);
+ }
+
+ /**
+ * Returns the time (ns) when the read lock holding period began specific to
+ * a thread.
+ *
+ * @return read lock held start time (ns)
+ */
+ long getStartReadHeldTimeNanos() {
+ return readLockTimeStampNanos.get().getStartReadHeldTimeNanos();
+ }
+
+ /**
+ * Returns the time (ns) when the write lock holding period began specific
+ * to a thread.
*
- * @return lock held start time (ns)
+ * @return write lock held start time (ns)
*/
- long getStartHeldTimeNanos() {
- return readLockTimeStampNanos.get().getStartHeldTimeNanos();
+ long getStartWriteHeldTimeNanos() {
+ return writeLockTimeStampNanos.get().getStartWriteHeldTimeNanos();
Review Comment:
Made the changes, thank you!
##########
hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/lock/OzoneManagerLock.java:
##########
@@ -514,29 +615,56 @@ public void cleanup() {
// Name of the resource.
private String name;
- // This helps in maintaining lock related variables locally confined to a
- // given thread.
+ // This helps in maintaining read lock related variables locally confined
+ // to a given thread.
private final ThreadLocal<LockUsageInfo> readLockTimeStampNanos =
ThreadLocal.withInitial(LockUsageInfo::new);
+ // This helps in maintaining write lock related variables locally confined
+ // to a given thread.
+ private final ThreadLocal<LockUsageInfo> writeLockTimeStampNanos =
+ ThreadLocal.withInitial(LockUsageInfo::new);
+
/**
- * Sets the time (ns) when the lock holding period begins specific to a
+ * Sets the time (ns) when the read lock holding period begins specific to a
* thread.
*
- * @param startHeldTimeNanos lock held start time (ns)
+ * @param startReadHeldTimeNanos read lock held start time (ns)
*/
- void setStartHeldTimeNanos(long startHeldTimeNanos) {
- readLockTimeStampNanos.get().setStartHeldTimeNanos(startHeldTimeNanos);
+ void setStartReadHeldTimeNanos(long startReadHeldTimeNanos) {
+ readLockTimeStampNanos.get()
+ .setStartReadHeldTimeNanos(startReadHeldTimeNanos);
}
/**
- * Returns the time (ns) when the lock holding period began specific to a
- * thread.
+ * Sets the time (ns) when the write lock holding period begins specific to
+ * a thread.
+ *
+ * @param startWriteHeldTimeNanos write lock held start time (ns)
+ */
+ void setStartWriteHeldTimeNanos(long startWriteHeldTimeNanos) {
+ writeLockTimeStampNanos.get()
+ .setStartWriteHeldTimeNanos(startWriteHeldTimeNanos);
+ }
+
+ /**
+ * Returns the time (ns) when the read lock holding period began specific to
+ * a thread.
+ *
+ * @return read lock held start time (ns)
+ */
+ long getStartReadHeldTimeNanos() {
+ return readLockTimeStampNanos.get().getStartReadHeldTimeNanos();
Review Comment:
Made the changes, thank you!
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] rakeshadr commented on a diff in pull request #3271: HDDS-6436. Add write lock waiting and held time metrics
Posted by GitBox <gi...@apache.org>.
rakeshadr commented on code in PR #3271:
URL: https://github.com/apache/ozone/pull/3271#discussion_r845051115
##########
hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/lock/TestOzoneManagerLock.java:
##########
@@ -393,41 +393,177 @@ private void testReadLockHoldCountUtil(OzoneManagerLock.Resource resource,
lock.releaseReadLock(resource, resourceName);
assertEquals(0, lock.getReadHoldCount(resourceLockName));
+
+ Assert.assertFalse(lock.isWriteLockedByCurrentThread(resourceLockName));
+ assertEquals(0, lock.getWriteHoldCount(resourceLockName));
+ lock.acquireWriteLock(resource, resourceName);
+ Assert.assertTrue(lock.isWriteLockedByCurrentThread(resourceLockName));
+ assertEquals(1, lock.getWriteHoldCount(resourceLockName));
+
+ lock.acquireWriteLock(resource, resourceName);
+ Assert.assertTrue(lock.isWriteLockedByCurrentThread(resourceLockName));
+ assertEquals(2, lock.getWriteHoldCount(resourceLockName));
+
+ lock.releaseWriteLock(resource, resourceName);
+ Assert.assertTrue(lock.isWriteLockedByCurrentThread(resourceLockName));
+ assertEquals(1, lock.getWriteHoldCount(resourceLockName));
+
+ lock.releaseWriteLock(resource, resourceName);
+ Assert.assertFalse(lock.isWriteLockedByCurrentThread(resourceLockName));
+ assertEquals(0, lock.getWriteHoldCount(resourceLockName));
}
@Test
- public void testReadLockConcurrentStats() throws InterruptedException {
+ public void testLockConcurrentStats() throws InterruptedException {
String[] resourceName;
for (OzoneManagerLock.Resource resource :
OzoneManagerLock.Resource.values()) {
resourceName = generateResourceName(resource);
+ testReadLockConcurrentStats(resource, resourceName, 10);
+ testWriteLockConcurrentStats(resource, resourceName, 5);
+ testSyntheticReadWriteLockConcurrentStats(resource, resourceName, 10, 3);
+ }
+ }
- OzoneManagerLock lock = new OzoneManagerLock(new OzoneConfiguration());
- final int threadCount = 10;
- Thread[] threads = new Thread[threadCount];
- for (int i = 0; i < threads.length; i++) {
- String[] finalResourceName = resourceName;
- threads[i] = new Thread(() -> {
- lock.acquireReadLock(resource, finalResourceName);
- try {
- Thread.sleep(1000);
- } catch (InterruptedException e) {
- e.printStackTrace();
- }
- lock.releaseReadLock(resource, finalResourceName);
- });
- threads[i].start();
- }
+ public void testReadLockConcurrentStats(OzoneManagerLock.Resource resource,
+ String[] resourceName,
+ int threadCount)
+ throws InterruptedException {
+ OzoneManagerLock lock = new OzoneManagerLock(new OzoneConfiguration());
+ Thread[] threads = new Thread[threadCount];
+
+ for (int i = 0; i < threads.length; i++) {
+ threads[i] = new Thread(() -> {
+ lock.acquireReadLock(resource, resourceName);
+ try {
+ Thread.sleep(1000);
+ } catch (InterruptedException e) {
+ e.printStackTrace();
+ }
+ lock.releaseReadLock(resource, resourceName);
+ });
+ threads[i].start();
+ }
- for (Thread t : threads) {
- t.join();
- }
+ for (Thread t : threads) {
+ t.join();
+ }
+
+ String readHeldStat = lock.getReadLockHeldTimeMsStat();
+ Assert.assertTrue(
+ "Expected " + threadCount +
+ " samples in readLockHeldTimeMsStat: " + readHeldStat,
+ readHeldStat.contains("Samples = " + threadCount));
+
+ String readWaitingStat = lock.getReadLockWaitingTimeMsStat();
+ Assert.assertTrue(
+ "Expected " + threadCount +
+ " samples in readLockWaitingTimeMsStat: " + readWaitingStat,
+ readWaitingStat.contains("Samples = " + threadCount));
+ }
+
+ public void testWriteLockConcurrentStats(OzoneManagerLock.Resource resource,
+ String[] resourceName,
+ int threadCount)
+ throws InterruptedException {
+ OzoneManagerLock lock = new OzoneManagerLock(new OzoneConfiguration());
+ Thread[] threads = new Thread[threadCount];
- String readHeldStat = lock.getReadLockHeldTimeMsStat();
- Assert.assertTrue(
- "Expected " + threadCount + " samples in " + readHeldStat,
- readHeldStat.contains("Samples = " + threadCount));
+ for (int i = 0; i < threads.length; i++) {
+ threads[i] = new Thread(() -> {
+ lock.acquireWriteLock(resource, resourceName);
+ try {
+ Thread.sleep(500);
+ } catch (InterruptedException e) {
+ e.printStackTrace();
+ }
+ lock.releaseWriteLock(resource, resourceName);
+ });
+ threads[i].start();
+ }
+
+ for (Thread t : threads) {
+ t.join();
+ }
+
+ String writeHeldStat = lock.getWriteLockHeldTimeMsStat();
+ Assert.assertTrue(
+ "Expected " + threadCount +
+ " samples in writeLockHeldTimeMsStat: " + writeHeldStat,
+ writeHeldStat.contains("Samples = " + threadCount));
+
+ String writeWaitingStat = lock.getWriteLockWaitingTimeMsStat();
+ Assert.assertTrue(
+ "Expected " + threadCount +
+ " samples in writeLockWaitingTimeMsStat" + writeWaitingStat,
+ writeWaitingStat.contains("Samples = " + threadCount));
+ }
+
+ public void testSyntheticReadWriteLockConcurrentStats(
+ OzoneManagerLock.Resource resource, String[] resourceName,
+ int readThreadCount, int writeThreadCount)
+ throws InterruptedException {
+ OzoneManagerLock lock = new OzoneManagerLock(new OzoneConfiguration());
+ Thread[] readThreads = new Thread[readThreadCount];
+ Thread[] writeThreads = new Thread[writeThreadCount];
+
+ for (int i = 0; i < readThreads.length; i++) {
+ readThreads[i] = new Thread(() -> {
+ lock.acquireReadLock(resource, resourceName);
+ try {
+ Thread.sleep(1000);
+ } catch (InterruptedException e) {
+ e.printStackTrace();
+ }
+ lock.releaseReadLock(resource, resourceName);
+ });
+ readThreads[i].start();
}
+
+ for (int i = 0; i < writeThreads.length; i++) {
+ writeThreads[i] = new Thread(() -> {
+ lock.acquireWriteLock(resource, resourceName);
+ try {
+ Thread.sleep(500);
Review Comment:
In synthetic test, can you reduce the sleeping time to below values. This would make the test faster and quickly finish.
write Thread sleeping time -> `Thread.sleep(100);`
read Thread sleeping time -> `Thread.sleep(500);`
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] tanvipenumudy commented on a diff in pull request #3271: HDDS-6436. Add write lock waiting and held time metrics
Posted by GitBox <gi...@apache.org>.
tanvipenumudy commented on code in PR #3271:
URL: https://github.com/apache/ozone/pull/3271#discussion_r844611379
##########
hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/lock/OzoneManagerLock.java:
##########
@@ -195,18 +202,28 @@ private boolean lock(Resource resource, String resourceName,
}
}
- private void updateReadLockMetrics(Resource resource, String lockType,
+ private void updateReadLockMetrics(Resource resource,
long startWaitingTimeNanos) {
- if (lockType.equals(READ_LOCK)) {
- long readLockWaitingTimeNanos =
- Time.monotonicNowNanos() - startWaitingTimeNanos;
+ long readLockWaitingTimeNanos =
+ Time.monotonicNowNanos() - startWaitingTimeNanos;
- // Adds a snapshot to the metric readLockWaitingTimeMsStat.
- omLockMetrics.setReadLockWaitingTimeMsStat(
- TimeUnit.NANOSECONDS.toMillis(readLockWaitingTimeNanos));
+ // Adds a snapshot to the metric readLockWaitingTimeMsStat.
+ omLockMetrics.setReadLockWaitingTimeMsStat(
+ TimeUnit.NANOSECONDS.toMillis(readLockWaitingTimeNanos));
- resource.setStartHeldTimeNanos(Time.monotonicNowNanos());
- }
+ resource.setStartHeldTimeNanos(Time.monotonicNowNanos());
+ }
+
+ private void updateWriteLockMetrics(Resource resource,
+ long startWaitingTimeNanos) {
+ long writeLockWaitingTimeNanos =
+ Time.monotonicNowNanos() - startWaitingTimeNanos;
+
+ // Adds a snapshot to the metric writeLockWaitingTimeMsStat.
+ omLockMetrics.setWriteLockWaitingTimeMsStat(
+ TimeUnit.NANOSECONDS.toMillis(writeLockWaitingTimeNanos));
+
+ resource.setStartHeldTimeNanos(Time.monotonicNowNanos());
Review Comment:
Thank you for the comment, I have incorporated the changes!
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] rakeshadr merged pull request #3271: HDDS-6436. Add write lock waiting and held time metrics
Posted by GitBox <gi...@apache.org>.
rakeshadr merged PR #3271:
URL: https://github.com/apache/ozone/pull/3271
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] rakeshadr commented on a diff in pull request #3271: HDDS-6436. Add write lock waiting and held time metrics
Posted by GitBox <gi...@apache.org>.
rakeshadr commented on code in PR #3271:
URL: https://github.com/apache/ozone/pull/3271#discussion_r845053306
##########
hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/lock/TestOzoneManagerLock.java:
##########
@@ -393,41 +393,177 @@ private void testReadLockHoldCountUtil(OzoneManagerLock.Resource resource,
lock.releaseReadLock(resource, resourceName);
assertEquals(0, lock.getReadHoldCount(resourceLockName));
+
+ Assert.assertFalse(lock.isWriteLockedByCurrentThread(resourceLockName));
+ assertEquals(0, lock.getWriteHoldCount(resourceLockName));
+ lock.acquireWriteLock(resource, resourceName);
+ Assert.assertTrue(lock.isWriteLockedByCurrentThread(resourceLockName));
+ assertEquals(1, lock.getWriteHoldCount(resourceLockName));
+
+ lock.acquireWriteLock(resource, resourceName);
+ Assert.assertTrue(lock.isWriteLockedByCurrentThread(resourceLockName));
+ assertEquals(2, lock.getWriteHoldCount(resourceLockName));
+
+ lock.releaseWriteLock(resource, resourceName);
+ Assert.assertTrue(lock.isWriteLockedByCurrentThread(resourceLockName));
+ assertEquals(1, lock.getWriteHoldCount(resourceLockName));
+
+ lock.releaseWriteLock(resource, resourceName);
+ Assert.assertFalse(lock.isWriteLockedByCurrentThread(resourceLockName));
+ assertEquals(0, lock.getWriteHoldCount(resourceLockName));
}
@Test
- public void testReadLockConcurrentStats() throws InterruptedException {
+ public void testLockConcurrentStats() throws InterruptedException {
String[] resourceName;
for (OzoneManagerLock.Resource resource :
OzoneManagerLock.Resource.values()) {
resourceName = generateResourceName(resource);
+ testReadLockConcurrentStats(resource, resourceName, 10);
+ testWriteLockConcurrentStats(resource, resourceName, 5);
+ testSyntheticReadWriteLockConcurrentStats(resource, resourceName, 10, 3);
+ }
+ }
- OzoneManagerLock lock = new OzoneManagerLock(new OzoneConfiguration());
- final int threadCount = 10;
- Thread[] threads = new Thread[threadCount];
- for (int i = 0; i < threads.length; i++) {
- String[] finalResourceName = resourceName;
- threads[i] = new Thread(() -> {
- lock.acquireReadLock(resource, finalResourceName);
- try {
- Thread.sleep(1000);
- } catch (InterruptedException e) {
- e.printStackTrace();
- }
- lock.releaseReadLock(resource, finalResourceName);
- });
- threads[i].start();
- }
+ public void testReadLockConcurrentStats(OzoneManagerLock.Resource resource,
+ String[] resourceName,
+ int threadCount)
+ throws InterruptedException {
+ OzoneManagerLock lock = new OzoneManagerLock(new OzoneConfiguration());
+ Thread[] threads = new Thread[threadCount];
+
+ for (int i = 0; i < threads.length; i++) {
+ threads[i] = new Thread(() -> {
+ lock.acquireReadLock(resource, resourceName);
+ try {
+ Thread.sleep(1000);
+ } catch (InterruptedException e) {
+ e.printStackTrace();
+ }
+ lock.releaseReadLock(resource, resourceName);
+ });
+ threads[i].start();
+ }
- for (Thread t : threads) {
- t.join();
- }
+ for (Thread t : threads) {
+ t.join();
+ }
+
+ String readHeldStat = lock.getReadLockHeldTimeMsStat();
+ Assert.assertTrue(
+ "Expected " + threadCount +
+ " samples in readLockHeldTimeMsStat: " + readHeldStat,
+ readHeldStat.contains("Samples = " + threadCount));
+
+ String readWaitingStat = lock.getReadLockWaitingTimeMsStat();
+ Assert.assertTrue(
+ "Expected " + threadCount +
+ " samples in readLockWaitingTimeMsStat: " + readWaitingStat,
+ readWaitingStat.contains("Samples = " + threadCount));
+ }
+
+ public void testWriteLockConcurrentStats(OzoneManagerLock.Resource resource,
+ String[] resourceName,
+ int threadCount)
+ throws InterruptedException {
+ OzoneManagerLock lock = new OzoneManagerLock(new OzoneConfiguration());
+ Thread[] threads = new Thread[threadCount];
- String readHeldStat = lock.getReadLockHeldTimeMsStat();
- Assert.assertTrue(
- "Expected " + threadCount + " samples in " + readHeldStat,
- readHeldStat.contains("Samples = " + threadCount));
+ for (int i = 0; i < threads.length; i++) {
+ threads[i] = new Thread(() -> {
+ lock.acquireWriteLock(resource, resourceName);
+ try {
+ Thread.sleep(500);
+ } catch (InterruptedException e) {
+ e.printStackTrace();
+ }
+ lock.releaseWriteLock(resource, resourceName);
+ });
+ threads[i].start();
+ }
+
+ for (Thread t : threads) {
+ t.join();
+ }
+
+ String writeHeldStat = lock.getWriteLockHeldTimeMsStat();
+ Assert.assertTrue(
+ "Expected " + threadCount +
+ " samples in writeLockHeldTimeMsStat: " + writeHeldStat,
+ writeHeldStat.contains("Samples = " + threadCount));
+
+ String writeWaitingStat = lock.getWriteLockWaitingTimeMsStat();
+ Assert.assertTrue(
+ "Expected " + threadCount +
+ " samples in writeLockWaitingTimeMsStat" + writeWaitingStat,
+ writeWaitingStat.contains("Samples = " + threadCount));
+ }
+
+ public void testSyntheticReadWriteLockConcurrentStats(
+ OzoneManagerLock.Resource resource, String[] resourceName,
+ int readThreadCount, int writeThreadCount)
+ throws InterruptedException {
+ OzoneManagerLock lock = new OzoneManagerLock(new OzoneConfiguration());
+ Thread[] readThreads = new Thread[readThreadCount];
+ Thread[] writeThreads = new Thread[writeThreadCount];
+
+ for (int i = 0; i < readThreads.length; i++) {
+ readThreads[i] = new Thread(() -> {
+ lock.acquireReadLock(resource, resourceName);
+ try {
+ Thread.sleep(1000);
+ } catch (InterruptedException e) {
+ e.printStackTrace();
+ }
+ lock.releaseReadLock(resource, resourceName);
+ });
+ readThreads[i].start();
}
+
+ for (int i = 0; i < writeThreads.length; i++) {
+ writeThreads[i] = new Thread(() -> {
+ lock.acquireWriteLock(resource, resourceName);
+ try {
+ Thread.sleep(500);
+ } catch (InterruptedException e) {
+ e.printStackTrace();
+ }
+ lock.releaseWriteLock(resource, resourceName);
+ });
+ writeThreads[i].start();
Review Comment:
Please name the thread so that we could differentiate it in mix workload.
`writeThreads[i].setName("WriteLockThread-" + i);`
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] rakeshadr commented on a diff in pull request #3271: HDDS-6436. Add write lock waiting and held time metrics
Posted by GitBox <gi...@apache.org>.
rakeshadr commented on code in PR #3271:
URL: https://github.com/apache/ozone/pull/3271#discussion_r845776776
##########
hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/lock/OzoneManagerLock.java:
##########
@@ -514,29 +615,56 @@ public void cleanup() {
// Name of the resource.
private String name;
- // This helps in maintaining lock related variables locally confined to a
- // given thread.
+ // This helps in maintaining read lock related variables locally confined
+ // to a given thread.
private final ThreadLocal<LockUsageInfo> readLockTimeStampNanos =
ThreadLocal.withInitial(LockUsageInfo::new);
+ // This helps in maintaining write lock related variables locally confined
+ // to a given thread.
+ private final ThreadLocal<LockUsageInfo> writeLockTimeStampNanos =
+ ThreadLocal.withInitial(LockUsageInfo::new);
+
/**
- * Sets the time (ns) when the lock holding period begins specific to a
+ * Sets the time (ns) when the read lock holding period begins specific to a
* thread.
*
- * @param startHeldTimeNanos lock held start time (ns)
+ * @param startReadHeldTimeNanos read lock held start time (ns)
*/
- void setStartHeldTimeNanos(long startHeldTimeNanos) {
- readLockTimeStampNanos.get().setStartHeldTimeNanos(startHeldTimeNanos);
+ void setStartReadHeldTimeNanos(long startReadHeldTimeNanos) {
+ readLockTimeStampNanos.get()
+ .setStartReadHeldTimeNanos(startReadHeldTimeNanos);
}
/**
- * Returns the time (ns) when the lock holding period began specific to a
- * thread.
+ * Sets the time (ns) when the write lock holding period begins specific to
+ * a thread.
+ *
+ * @param startWriteHeldTimeNanos write lock held start time (ns)
+ */
+ void setStartWriteHeldTimeNanos(long startWriteHeldTimeNanos) {
+ writeLockTimeStampNanos.get()
+ .setStartWriteHeldTimeNanos(startWriteHeldTimeNanos);
+ }
+
+ /**
+ * Returns the time (ns) when the read lock holding period began specific to
+ * a thread.
+ *
+ * @return read lock held start time (ns)
+ */
+ long getStartReadHeldTimeNanos() {
+ return readLockTimeStampNanos.get().getStartReadHeldTimeNanos();
Review Comment:
I think you can remove the element for safer side.
`readLockTimeStampNanos.remove();`
##########
hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/lock/OzoneManagerLock.java:
##########
@@ -514,29 +615,56 @@ public void cleanup() {
// Name of the resource.
private String name;
- // This helps in maintaining lock related variables locally confined to a
- // given thread.
+ // This helps in maintaining read lock related variables locally confined
+ // to a given thread.
private final ThreadLocal<LockUsageInfo> readLockTimeStampNanos =
ThreadLocal.withInitial(LockUsageInfo::new);
+ // This helps in maintaining write lock related variables locally confined
+ // to a given thread.
+ private final ThreadLocal<LockUsageInfo> writeLockTimeStampNanos =
+ ThreadLocal.withInitial(LockUsageInfo::new);
+
/**
- * Sets the time (ns) when the lock holding period begins specific to a
+ * Sets the time (ns) when the read lock holding period begins specific to a
* thread.
*
- * @param startHeldTimeNanos lock held start time (ns)
+ * @param startReadHeldTimeNanos read lock held start time (ns)
*/
- void setStartHeldTimeNanos(long startHeldTimeNanos) {
- readLockTimeStampNanos.get().setStartHeldTimeNanos(startHeldTimeNanos);
+ void setStartReadHeldTimeNanos(long startReadHeldTimeNanos) {
+ readLockTimeStampNanos.get()
+ .setStartReadHeldTimeNanos(startReadHeldTimeNanos);
}
/**
- * Returns the time (ns) when the lock holding period began specific to a
- * thread.
+ * Sets the time (ns) when the write lock holding period begins specific to
+ * a thread.
+ *
+ * @param startWriteHeldTimeNanos write lock held start time (ns)
+ */
+ void setStartWriteHeldTimeNanos(long startWriteHeldTimeNanos) {
+ writeLockTimeStampNanos.get()
+ .setStartWriteHeldTimeNanos(startWriteHeldTimeNanos);
+ }
+
+ /**
+ * Returns the time (ns) when the read lock holding period began specific to
+ * a thread.
+ *
+ * @return read lock held start time (ns)
+ */
+ long getStartReadHeldTimeNanos() {
+ return readLockTimeStampNanos.get().getStartReadHeldTimeNanos();
Review Comment:
I think you can remove the element for safer side.
`readLockTimeStampNanos.remove();`
##########
hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/lock/OzoneManagerLock.java:
##########
@@ -514,29 +615,56 @@ public void cleanup() {
// Name of the resource.
private String name;
- // This helps in maintaining lock related variables locally confined to a
- // given thread.
+ // This helps in maintaining read lock related variables locally confined
+ // to a given thread.
private final ThreadLocal<LockUsageInfo> readLockTimeStampNanos =
ThreadLocal.withInitial(LockUsageInfo::new);
+ // This helps in maintaining write lock related variables locally confined
+ // to a given thread.
+ private final ThreadLocal<LockUsageInfo> writeLockTimeStampNanos =
+ ThreadLocal.withInitial(LockUsageInfo::new);
+
/**
- * Sets the time (ns) when the lock holding period begins specific to a
+ * Sets the time (ns) when the read lock holding period begins specific to a
* thread.
*
- * @param startHeldTimeNanos lock held start time (ns)
+ * @param startReadHeldTimeNanos read lock held start time (ns)
*/
- void setStartHeldTimeNanos(long startHeldTimeNanos) {
- readLockTimeStampNanos.get().setStartHeldTimeNanos(startHeldTimeNanos);
+ void setStartReadHeldTimeNanos(long startReadHeldTimeNanos) {
+ readLockTimeStampNanos.get()
+ .setStartReadHeldTimeNanos(startReadHeldTimeNanos);
}
/**
- * Returns the time (ns) when the lock holding period began specific to a
- * thread.
+ * Sets the time (ns) when the write lock holding period begins specific to
+ * a thread.
+ *
+ * @param startWriteHeldTimeNanos write lock held start time (ns)
+ */
+ void setStartWriteHeldTimeNanos(long startWriteHeldTimeNanos) {
+ writeLockTimeStampNanos.get()
+ .setStartWriteHeldTimeNanos(startWriteHeldTimeNanos);
+ }
+
+ /**
+ * Returns the time (ns) when the read lock holding period began specific to
+ * a thread.
+ *
+ * @return read lock held start time (ns)
+ */
+ long getStartReadHeldTimeNanos() {
+ return readLockTimeStampNanos.get().getStartReadHeldTimeNanos();
+ }
+
+ /**
+ * Returns the time (ns) when the write lock holding period began specific
+ * to a thread.
*
- * @return lock held start time (ns)
+ * @return write lock held start time (ns)
*/
- long getStartHeldTimeNanos() {
- return readLockTimeStampNanos.get().getStartHeldTimeNanos();
+ long getStartWriteHeldTimeNanos() {
+ return writeLockTimeStampNanos.get().getStartWriteHeldTimeNanos();
Review Comment:
I think you can remove the element for safer side.
`writeLockTimeStampNanos.remove();`
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] tanvipenumudy commented on a diff in pull request #3271: HDDS-6436. Add write lock waiting and held time metrics
Posted by GitBox <gi...@apache.org>.
tanvipenumudy commented on code in PR #3271:
URL: https://github.com/apache/ozone/pull/3271#discussion_r845250704
##########
hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/lock/TestOzoneManagerLock.java:
##########
@@ -393,41 +393,177 @@ private void testReadLockHoldCountUtil(OzoneManagerLock.Resource resource,
lock.releaseReadLock(resource, resourceName);
assertEquals(0, lock.getReadHoldCount(resourceLockName));
+
+ Assert.assertFalse(lock.isWriteLockedByCurrentThread(resourceLockName));
+ assertEquals(0, lock.getWriteHoldCount(resourceLockName));
+ lock.acquireWriteLock(resource, resourceName);
+ Assert.assertTrue(lock.isWriteLockedByCurrentThread(resourceLockName));
+ assertEquals(1, lock.getWriteHoldCount(resourceLockName));
+
+ lock.acquireWriteLock(resource, resourceName);
+ Assert.assertTrue(lock.isWriteLockedByCurrentThread(resourceLockName));
+ assertEquals(2, lock.getWriteHoldCount(resourceLockName));
+
+ lock.releaseWriteLock(resource, resourceName);
+ Assert.assertTrue(lock.isWriteLockedByCurrentThread(resourceLockName));
+ assertEquals(1, lock.getWriteHoldCount(resourceLockName));
+
+ lock.releaseWriteLock(resource, resourceName);
+ Assert.assertFalse(lock.isWriteLockedByCurrentThread(resourceLockName));
+ assertEquals(0, lock.getWriteHoldCount(resourceLockName));
}
@Test
- public void testReadLockConcurrentStats() throws InterruptedException {
+ public void testLockConcurrentStats() throws InterruptedException {
String[] resourceName;
for (OzoneManagerLock.Resource resource :
OzoneManagerLock.Resource.values()) {
resourceName = generateResourceName(resource);
+ testReadLockConcurrentStats(resource, resourceName, 10);
+ testWriteLockConcurrentStats(resource, resourceName, 5);
+ testSyntheticReadWriteLockConcurrentStats(resource, resourceName, 10, 3);
+ }
+ }
- OzoneManagerLock lock = new OzoneManagerLock(new OzoneConfiguration());
- final int threadCount = 10;
- Thread[] threads = new Thread[threadCount];
- for (int i = 0; i < threads.length; i++) {
- String[] finalResourceName = resourceName;
- threads[i] = new Thread(() -> {
- lock.acquireReadLock(resource, finalResourceName);
- try {
- Thread.sleep(1000);
- } catch (InterruptedException e) {
- e.printStackTrace();
- }
- lock.releaseReadLock(resource, finalResourceName);
- });
- threads[i].start();
- }
+ public void testReadLockConcurrentStats(OzoneManagerLock.Resource resource,
+ String[] resourceName,
+ int threadCount)
+ throws InterruptedException {
+ OzoneManagerLock lock = new OzoneManagerLock(new OzoneConfiguration());
+ Thread[] threads = new Thread[threadCount];
+
+ for (int i = 0; i < threads.length; i++) {
+ threads[i] = new Thread(() -> {
+ lock.acquireReadLock(resource, resourceName);
+ try {
+ Thread.sleep(1000);
+ } catch (InterruptedException e) {
+ e.printStackTrace();
+ }
+ lock.releaseReadLock(resource, resourceName);
+ });
+ threads[i].start();
+ }
- for (Thread t : threads) {
- t.join();
- }
+ for (Thread t : threads) {
+ t.join();
+ }
+
+ String readHeldStat = lock.getReadLockHeldTimeMsStat();
+ Assert.assertTrue(
+ "Expected " + threadCount +
+ " samples in readLockHeldTimeMsStat: " + readHeldStat,
+ readHeldStat.contains("Samples = " + threadCount));
+
+ String readWaitingStat = lock.getReadLockWaitingTimeMsStat();
+ Assert.assertTrue(
+ "Expected " + threadCount +
+ " samples in readLockWaitingTimeMsStat: " + readWaitingStat,
+ readWaitingStat.contains("Samples = " + threadCount));
+ }
+
+ public void testWriteLockConcurrentStats(OzoneManagerLock.Resource resource,
+ String[] resourceName,
+ int threadCount)
+ throws InterruptedException {
+ OzoneManagerLock lock = new OzoneManagerLock(new OzoneConfiguration());
+ Thread[] threads = new Thread[threadCount];
- String readHeldStat = lock.getReadLockHeldTimeMsStat();
- Assert.assertTrue(
- "Expected " + threadCount + " samples in " + readHeldStat,
- readHeldStat.contains("Samples = " + threadCount));
+ for (int i = 0; i < threads.length; i++) {
+ threads[i] = new Thread(() -> {
+ lock.acquireWriteLock(resource, resourceName);
+ try {
+ Thread.sleep(500);
+ } catch (InterruptedException e) {
+ e.printStackTrace();
+ }
+ lock.releaseWriteLock(resource, resourceName);
+ });
+ threads[i].start();
+ }
+
+ for (Thread t : threads) {
+ t.join();
+ }
+
+ String writeHeldStat = lock.getWriteLockHeldTimeMsStat();
+ Assert.assertTrue(
+ "Expected " + threadCount +
+ " samples in writeLockHeldTimeMsStat: " + writeHeldStat,
+ writeHeldStat.contains("Samples = " + threadCount));
+
+ String writeWaitingStat = lock.getWriteLockWaitingTimeMsStat();
+ Assert.assertTrue(
+ "Expected " + threadCount +
+ " samples in writeLockWaitingTimeMsStat" + writeWaitingStat,
+ writeWaitingStat.contains("Samples = " + threadCount));
+ }
+
+ public void testSyntheticReadWriteLockConcurrentStats(
+ OzoneManagerLock.Resource resource, String[] resourceName,
+ int readThreadCount, int writeThreadCount)
+ throws InterruptedException {
+ OzoneManagerLock lock = new OzoneManagerLock(new OzoneConfiguration());
+ Thread[] readThreads = new Thread[readThreadCount];
+ Thread[] writeThreads = new Thread[writeThreadCount];
+
+ for (int i = 0; i < readThreads.length; i++) {
+ readThreads[i] = new Thread(() -> {
+ lock.acquireReadLock(resource, resourceName);
+ try {
+ Thread.sleep(1000);
+ } catch (InterruptedException e) {
+ e.printStackTrace();
+ }
+ lock.releaseReadLock(resource, resourceName);
+ });
+ readThreads[i].start();
Review Comment:
Thank you for the suggestion, I have added the same
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] tanvipenumudy commented on pull request #3271: HDDS-6436. Add write lock waiting and held time metrics
Posted by GitBox <gi...@apache.org>.
tanvipenumudy commented on PR #3271:
URL: https://github.com/apache/ozone/pull/3271#issuecomment-1089846384
Hi @JyotinderSingh, @siddhantsangwan, @rakeshadr it would be great if you can review the patch, thanks!
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org