[ 
https://issues.apache.org/jira/browse/GEODE-8035?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17101015#comment-17101015
 ] 

ASF GitHub Bot commented on GEODE-8035:
---------------------------------------

kirklund commented on a change in pull request #5014:
URL: https://github.com/apache/geode/pull/5014#discussion_r420945761



##########
File path: 
geode-core/src/test/java/org/apache/geode/internal/cache/GemFireCacheImplTest.java
##########
@@ -620,6 +625,40 @@ public void getCacheServers_isCanonical() {
         .isSameAs(gemFireCacheImpl.getCacheServers());
   }
 
+  @Test
+  public void testLockDiskStore() throws InterruptedException {
+    int nThread = 10;
+    String diskStoreName = "MyDiskStore";
+    AtomicInteger nTrue = new AtomicInteger();
+    AtomicInteger nFalse = new AtomicInteger();
+    ExecutorService executorService = Executors.newFixedThreadPool(nThread);

Review comment:
       I recommend replacing ExecutorService with ExecutorServiceRule. You can 
limit the rule to a specific number of threads if you need to otherwise it 
defaults to as many threads as tasks that you submit:
   ```
   @Rule
   public ExecutorServiceRule executorServiceRule = new ExecutorServiceRule();
   ```
   The Rule will automatically do shutdown etc during tearDown().
   
   Also, you might want to capture the `Future<Void>` return value from 
`executorService/executorServiceRule.submit` to await on. You could even have a 
`Collection<Future<Void>>` if you wanted. 
   
   You could even move the assertions for locking/unlocking into the thread 
task (ie what you're submitting). If you await on the Futures, then any 
assertion failures will be thrown causing the test to fail:
   ```
   Future<Void> doLockUnlock = executorService.submit(() -> {
     try {
       assertThat(gemFireCacheImpl.doLockDiskStore(diskStoreName)).isTrue();
     } finally {
       assertThat(gemFireCacheImpl. doUnlockDiskStore(diskStoreName)).isTrue();
     }
   }
   
   doLockUnlock.get(GeodeAwaitility.getTimeout().toMillis(), 
TimeUnit.MILLISECONDS);
   ```
   

##########
File path: 
geode-core/src/test/java/org/apache/geode/internal/cache/GemFireCacheImplTest.java
##########
@@ -620,6 +625,40 @@ public void getCacheServers_isCanonical() {
         .isSameAs(gemFireCacheImpl.getCacheServers());
   }
 
+  @Test
+  public void testLockDiskStore() throws InterruptedException {
+    int nThread = 10;
+    String diskStoreName = "MyDiskStore";
+    AtomicInteger nTrue = new AtomicInteger();
+    AtomicInteger nFalse = new AtomicInteger();
+    ExecutorService executorService = Executors.newFixedThreadPool(nThread);
+    IntStream.range(0, nThread).forEach(tid -> {
+      executorService.submit(() -> {
+        try {
+          boolean lockResult = gemFireCacheImpl.doLockDiskStore(diskStoreName);
+          if (lockResult) {
+            nTrue.incrementAndGet();
+          } else {
+            nFalse.incrementAndGet();
+          }
+        } finally {
+          boolean unlockResult = 
gemFireCacheImpl.doUnlockDiskStore(diskStoreName);
+          if (unlockResult) {
+            nTrue.incrementAndGet();
+          } else {
+            nFalse.incrementAndGet();
+          }
+        }
+      });
+    });
+    executorService.shutdown();
+    executorService.awaitTermination(Long.MAX_VALUE, TimeUnit.NANOSECONDS);

Review comment:
       If you use the Rule, you shouldn't need this or the shutdown. In 
general, you should however use `GeodeAwaitility.getTimeout()` instead of 
`Long.MAX_VALUE`.




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


> Parallel Disk Store Recovery when Cluster Restarts
> --------------------------------------------------
>
>                 Key: GEODE-8035
>                 URL: https://issues.apache.org/jira/browse/GEODE-8035
>             Project: Geode
>          Issue Type: Improvement
>            Reporter: Jianxia Chen
>            Assignee: Jianxia Chen
>            Priority: Major
>              Labels: GeodeCommons
>
> Currently, when Geode cluster restarts, the disk store recovery is 
> serialized. When all regions share the same disk store, the restart process 
> is time consuming. To improve the performance, different regions can use 
> different disk stores with different disk controllers. And adding parallel 
> disk store recovery. This is expected to significantly reduce the time to 
> restart Geode cluster.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

Reply via email to