[geode-native] branch develop updated: GEODE-9147: Revert to multi-hop PUTALL in the face of missing metadata (#784)

[bb...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

bbender pushed a commit to branch develop
in repository https://gitbox.apache.org/repos/asf/geode-native.git


The following commit(s) were added to refs/heads/develop by this push:
     new 1be2ee3  GEODE-9147: Revert to multi-hop PUTALL in the face of missing metadata (#784)
1be2ee3 is described below

commit 1be2ee31f592ef99cf588753529ec62f5e7cddfd
Author: Blake Bender <bb...@pivotal.io>
AuthorDate: Fri Apr 16 07:44:23 2021 -0700

    GEODE-9147: Revert to multi-hop PUTALL in the face of missing metadata (#784)
    
    - This matches the behavior of the Java client
    - Our current code to "tack on" values that we don't have metadata
      for will sometimes result in EventIds reaching a server out-of-order,
      causing them to be dropped and resulting in data loss.  Resorting
      to multi-hop avoids this altogether.
    - Add test to repro issue of missing keys on single-hop putAll
---
 cppcache/integration/test/RegionPutAllTest.cpp | 35 ++++++++++++++++++++++++++
 cppcache/src/ClientMetadataService.cpp         | 11 ++++----
 2 files changed, 41 insertions(+), 5 deletions(-)

diff --git a/cppcache/integration/test/RegionPutAllTest.cpp b/cppcache/integration/test/RegionPutAllTest.cpp
index 7fd7804..ca40fac 100644
--- a/cppcache/integration/test/RegionPutAllTest.cpp
+++ b/cppcache/integration/test/RegionPutAllTest.cpp
@@ -102,4 +102,39 @@ TEST(RegionPutAllTest, putAllToPartitionedRegion) {
   }
 }
 
+//
+// verifies that putall works when not all metadata is present, i.e. not all
+// buckets exist yet on the cluster.
+//
+TEST(RegionPutAllTest, putAllAndVerifyKeysExist) {
+  Cluster cluster{LocatorCount{1}, ServerCount{3}};
+
+  cluster.start();
+
+  cluster.getGfsh()
+      .create()
+      .region()
+      .withName("region")
+      .withType("PARTITION")
+      .execute();
+
+  auto cache = createCache();
+  auto pool = createPool(cluster, cache);
+  auto region = setupRegion(cache, pool);
+
+  for (int i = 0; i < 50; i++) {
+    region->put(std::to_string(i), Cacheable::create(i));
+  }
+
+  HashMapOfCacheable all;
+  for (int i = 0; i < 113; i++) {
+    all.emplace(CacheableKey::create(std::to_string(i)), Cacheable::create(i));
+  }
+
+  std::this_thread::sleep_for(std::chrono::seconds(10));
+  region->putAll(all);
+  for (auto& key : all) {
+    ASSERT_TRUE(region->containsKeyOnServer(key.first));
+  }
+}
 }  // namespace
diff --git a/cppcache/src/ClientMetadataService.cpp b/cppcache/src/ClientMetadataService.cpp
index 3b46e35..7b1bde6 100644
--- a/cppcache/src/ClientMetadataService.cpp
+++ b/cppcache/src/ClientMetadataService.cpp
@@ -173,8 +173,6 @@ std::shared_ptr<ClientMetadata> ClientMetadataService::SendClientPRMetadata(
       new DataOutput(m_cache->createDataOutput(m_pool)), regionPath);
   TcrMessageReply reply(true, nullptr);
   // send this message to server and get metadata from server.
-  LOGFINE("Now sending GET_CLIENT_PR_METADATA for getting from server: %s",
-          regionPath);
   std::shared_ptr<Region> region = nullptr;
   GfErrType err = m_pool->sendSyncRequest(request, reply);
   if (err == GF_NOERR &&
@@ -186,7 +184,9 @@ std::shared_ptr<ClientMetadata> ClientMetadataService::SendClientPRMetadata(
       }
     }
     auto metadata = reply.getMetadata();
-    if (metadata == nullptr) return nullptr;
+    if (metadata == nullptr) {
+      return nullptr;
+    }
     if (metadata->empty()) {
       delete metadata;
       return nullptr;
@@ -358,8 +358,9 @@ ClientMetadataService::getServerToFilterMap(
       clientMetadata->getServerLocation(bucketId, isPrimary, serverLocation,
                                         version);
       if (!(serverLocation && serverLocation->isValid())) {
-        keysWhichLeft.push_back(key);
-        continue;
+        // If we're missing any metadata, give up.  This will cause us to revert
+        // to multi-hop, which is consistent with the Java client.
+        return nullptr;
       }
 
       buckets[bucketId] = serverLocation;