[
https://issues.apache.org/jira/browse/GEODE-9804?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17446129#comment-17446129
]
ASF GitHub Bot commented on GEODE-9804:
---------------------------------------
pivotal-jbarrett commented on a change in pull request #894:
URL: https://github.com/apache/geode-native/pull/894#discussion_r752586341
##########
File path: cppcache/integration/test/RegisterKeysTest.cpp
##########
@@ -39,19 +42,46 @@ namespace {
using apache::geode::client::binary_semaphore;
using apache::geode::client::Cache;
using apache::geode::client::CacheableInt16;
+using apache::geode::client::CacheableInt32;
using apache::geode::client::CacheableKey;
using apache::geode::client::CacheableString;
using apache::geode::client::CacheFactory;
+using apache::geode::client::CacheListener;
using apache::geode::client::CacheListenerMock;
+using apache::geode::client::EntryEvent;
using apache::geode::client::IllegalStateException;
using apache::geode::client::Region;
+using apache::geode::client::RegionEvent;
using apache::geode::client::RegionShortcut;
using ::testing::_;
using ::testing::DoAll;
using ::testing::InvokeWithoutArgs;
using ::testing::Return;
+constexpr auto kNumKeys = 100;
+
+class MyCacheListener : public CacheListener {
+ std::shared_ptr<boost::latch> m_allKeysUpdatedLatch;
Review comment:
Why are these `std::shared_ptr`? Just make them public and await on them
directly?
Also, style guide says `camelCaseFollowedByUnderscore_` for members names.
##########
File path: cppcache/integration/test/RegisterKeysTest.cpp
##########
@@ -575,4 +605,223 @@ TEST(RegisterKeysTest, RegisterAnyWithProxyRegion) {
cache.close();
}
+apache::geode::client::Cache createCache() {
+ return apache::geode::client::CacheFactory()
+ .set("log-level", "debug")
+ .set("log-file", "c:/temp/RegisterKeysTest.log")
+ .set("statistic-sampling-enabled", "false")
+ .create();
+}
+
+std::shared_ptr<apache::geode::client::Pool> createPool(
+ Cluster& cluster, apache::geode::client::Cache& cache) {
+ auto poolFactory = cache.getPoolManager().createFactory();
+ cluster.applyLocators(poolFactory);
+ poolFactory.setSubscriptionEnabled(true); // Per the customer.
+ return poolFactory.create("default");
+}
+
+std::shared_ptr<apache::geode::client::Region> setupRegion(
+ apache::geode::client::Cache& cache,
+ const std::shared_ptr<apache::geode::client::Pool>& pool) {
+ auto region =
+ cache
+ .createRegionFactory(apache::geode::client::RegionShortcut::
+ CACHING_PROXY) // Per the customer.
+ .setPoolName(pool->getName())
+ .create("region");
+
+ return region;
+}
+
+TEST(RegisterKeysTest, DontReceiveValues) {
+ Cluster cluster{LocatorCount{1}, ServerCount{1}};
+
+ cluster.start();
+
+ cluster.getGfsh()
+ .create()
+ .region()
+ .withName("region")
+ .withType("PARTITION")
+ .execute();
+
+ auto cache1 = createCache();
+ auto pool1 = createPool(cluster, cache1);
+ auto region1 = setupRegion(cache1, pool1);
+ auto attrMutator = region1->getAttributesMutator();
+
+ auto allKeysUpdatedLatch = std::make_shared<boost::latch>(kNumKeys);
+ auto allKeysInvalidLatch = std::make_shared<boost::latch>(kNumKeys);
+ auto listener = std::make_shared<MyCacheListener>(allKeysUpdatedLatch,
+ allKeysInvalidLatch);
+ attrMutator->setCacheListener(listener);
+
+ auto cache2 = createCache();
+ auto pool2 = createPool(cluster, cache2);
+ auto region2 = setupRegion(cache2, pool2);
+
+ for (int i = 0; i < kNumKeys; i++) {
+ region2->put(CacheableInt32::create(i), CacheableInt32::create(i));
+ }
+
+ region1->registerAllKeys(false, false, false);
+
+ for (int i = 0; i < kNumKeys; i++) {
+ auto hasKey = region1->containsKey(CacheableInt32::create(i));
+ EXPECT_FALSE(hasKey);
+ }
+
+ for (int i = 0; i < kNumKeys; i++) {
+ auto value = region1->get(CacheableInt32::create(i));
+ }
+
+ allKeysInvalidLatch->reset(kNumKeys);
+ listener->reset();
+
+ for (int i = 0; i < kNumKeys; i++) {
+ region2->put(CacheableInt32::create(i), CacheableInt32::create(i + 1000));
+ }
+
+ allKeysInvalidLatch->wait();
Review comment:
Should use `wait_for` and assert that it didn't timeout otherwise this
test will hang FOREVER if the latch is never satisfied.
##########
File path: cppcache/integration/test/RegisterKeysTest.cpp
##########
@@ -575,4 +605,223 @@ TEST(RegisterKeysTest, RegisterAnyWithProxyRegion) {
cache.close();
}
+apache::geode::client::Cache createCache() {
+ return apache::geode::client::CacheFactory()
+ .set("log-level", "debug")
+ .set("log-file", "c:/temp/RegisterKeysTest.log")
+ .set("statistic-sampling-enabled", "false")
+ .create();
+}
+
+std::shared_ptr<apache::geode::client::Pool> createPool(
+ Cluster& cluster, apache::geode::client::Cache& cache) {
+ auto poolFactory = cache.getPoolManager().createFactory();
+ cluster.applyLocators(poolFactory);
+ poolFactory.setSubscriptionEnabled(true); // Per the customer.
+ return poolFactory.create("default");
+}
+
+std::shared_ptr<apache::geode::client::Region> setupRegion(
+ apache::geode::client::Cache& cache,
+ const std::shared_ptr<apache::geode::client::Pool>& pool) {
+ auto region =
+ cache
+ .createRegionFactory(apache::geode::client::RegionShortcut::
+ CACHING_PROXY) // Per the customer.
+ .setPoolName(pool->getName())
+ .create("region");
+
+ return region;
+}
+
+TEST(RegisterKeysTest, DontReceiveValues) {
+ Cluster cluster{LocatorCount{1}, ServerCount{1}};
+
+ cluster.start();
+
+ cluster.getGfsh()
+ .create()
+ .region()
+ .withName("region")
+ .withType("PARTITION")
+ .execute();
+
+ auto cache1 = createCache();
+ auto pool1 = createPool(cluster, cache1);
+ auto region1 = setupRegion(cache1, pool1);
+ auto attrMutator = region1->getAttributesMutator();
+
+ auto allKeysUpdatedLatch = std::make_shared<boost::latch>(kNumKeys);
+ auto allKeysInvalidLatch = std::make_shared<boost::latch>(kNumKeys);
+ auto listener = std::make_shared<MyCacheListener>(allKeysUpdatedLatch,
+ allKeysInvalidLatch);
+ attrMutator->setCacheListener(listener);
+
+ auto cache2 = createCache();
+ auto pool2 = createPool(cluster, cache2);
+ auto region2 = setupRegion(cache2, pool2);
+
+ for (int i = 0; i < kNumKeys; i++) {
+ region2->put(CacheableInt32::create(i), CacheableInt32::create(i));
+ }
+
+ region1->registerAllKeys(false, false, false);
+
+ for (int i = 0; i < kNumKeys; i++) {
+ auto hasKey = region1->containsKey(CacheableInt32::create(i));
+ EXPECT_FALSE(hasKey);
+ }
+
+ for (int i = 0; i < kNumKeys; i++) {
+ auto value = region1->get(CacheableInt32::create(i));
+ }
+
+ allKeysInvalidLatch->reset(kNumKeys);
+ listener->reset();
+
+ for (int i = 0; i < kNumKeys; i++) {
+ region2->put(CacheableInt32::create(i), CacheableInt32::create(i + 1000));
+ }
+
+ allKeysInvalidLatch->wait();
Review comment:
Something like this snippet from another test.
```C++
ASSERT_THAT(task1.wait_for(minutes(1)), Eq(std::future_status::ready));
```
--
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]
> Both registerAllKeys and registerRegex always fetch initial entries.
> --------------------------------------------------------------------
>
> Key: GEODE-9804
> URL: https://issues.apache.org/jira/browse/GEODE-9804
> Project: Geode
> Issue Type: Bug
> Reporter: Jacob Barrett
> Priority: Major
> Labels: needsTriage, pull-request-available
>
> A inconsistency and bug in how the two regex interest methods configure the
> initial interest policy results in the both registerAllKeys and registerRegex
> fetching all initial entries despite the boolean parameter to get initial
> entries. Furthermore the misconfiguration results in none of the entries
> actually getting sent to the cache listener. The result is unnecessarily long
> registration times, network traffic and load on the servers. On servers with
> say millions of keys this can result in long GC pauses to unintentionally
> iterate over all those keys.
--
This message was sent by Atlassian Jira
(v8.20.1#820001)
