[GitHub] [geode] DonalEvans commented on a diff in pull request #7628: GEODE-10260: make sure message is added before they are processed.

[GitBox <gi...@apache.org>]

DonalEvans commented on code in PR #7628:
URL: https://github.com/apache/geode/pull/7628#discussion_r860263817


##########
geode-cq/src/distributedTest/java/org/apache/geode/cache/query/cq/CQDistributedTest.java:
##########
@@ -87,6 +91,47 @@ public void before() throws Exception {
     cqa = cqaf.create();
   }
 
+  @Rule
+  public ExecutorServiceRule executor = new ExecutorServiceRule();
+
+  @Test
+  // Before the fix, this test will reproduce pretty consistently if we put a sleep statement before
+  // we do localFP.addToFilterProfileQueue in FilterProfile$OperationMessage.process().
+  public void filterProfileUpdate() throws Exception {
+    MemberVM newServer = clusterStartupRule.startServerVM(3, locator.getPort());
+
+    // create 10 cqs to begin with
+    for (int i = 0; i < 10; i++) {
+      qs.newCq("query_" + i, "Select * from " + SEPARATOR + "region r where r.ID = " + i, cqa)
+          .execute();
+    }
+
+    AsyncInvocation regionCreate = newServer.invokeAsync(() -> {
+      ClusterStartupRule.memberStarter.createRegion(RegionShortcut.PARTITION, "region");
+    });
+
+    Future<Void> createCqs = executor.submit(() -> {
+      for (int i = 10; i < 100; i++) {
+        qs.newCq("query_" + i, "Select * from " + SEPARATOR + "region r where r.ID = " + i, cqa)
+            .execute();
+      }
+    });
+
+    regionCreate.await();
+    createCqs.get();
+
+    newServer.invoke(() -> {
+      Region regionOnServer = ClusterStartupRule.getCache().getRegion("region");

Review Comment:
   50 warnings in this file can be fixed by using `Region<Integer, Portfolio>` throughout. Making this change would also require that line 81 change to:
   ```
       region = clientCache
           .<Integer, Portfolio>createClientRegionFactory(ClientRegionShortcut.CACHING_PROXY)
           .create("region");
   ```



##########
geode-cq/src/distributedTest/java/org/apache/geode/cache/query/cq/CQDistributedTest.java:
##########
@@ -87,6 +91,47 @@ public void before() throws Exception {
     cqa = cqaf.create();
   }
 
+  @Rule
+  public ExecutorServiceRule executor = new ExecutorServiceRule();
+
+  @Test
+  // Before the fix, this test will reproduce pretty consistently if we put a sleep statement before
+  // we do localFP.addToFilterProfileQueue in FilterProfile$OperationMessage.process().
+  public void filterProfileUpdate() throws Exception {
+    MemberVM newServer = clusterStartupRule.startServerVM(3, locator.getPort());
+
+    // create 10 cqs to begin with
+    for (int i = 0; i < 10; i++) {
+      qs.newCq("query_" + i, "Select * from " + SEPARATOR + "region r where r.ID = " + i, cqa)
+          .execute();
+    }
+
+    AsyncInvocation regionCreate = newServer.invokeAsync(() -> {
+      ClusterStartupRule.memberStarter.createRegion(RegionShortcut.PARTITION, "region");
+    });
+
+    Future<Void> createCqs = executor.submit(() -> {
+      for (int i = 10; i < 100; i++) {
+        qs.newCq("query_" + i, "Select * from " + SEPARATOR + "region r where r.ID = " + i, cqa)
+            .execute();
+      }
+    });
+
+    regionCreate.await();
+    createCqs.get();
+
+    newServer.invoke(() -> {
+      Region regionOnServer = ClusterStartupRule.getCache().getRegion("region");
+      for (int i = 0; i < 100; i++) {
+        regionOnServer.put(i, new Portfolio(i));
+      }
+    });
+
+    // make sure all cq's will get its own event, so total events = total # of cqs.
+    await().atMost(10, TimeUnit.SECONDS)

Review Comment:
   The `atMost()` here could potentially make the test flaky if there are resource issues when running it. It would be better to just use the default await timeout here, since in the failure case, we expect that we will never reach 100, not that we will get there specifically within 10 seconds.



-- 
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: notifications-unsubscribe@geode.apache.org

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