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-unsubscr...@geode.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
