GEODE-5036: Make "enable-chunk-handler-thread" = "false" default (#292)

* Sdded new integration test for chunkhandler
* Delete test adding no value
* Changing 'disable-chunk-handler-thread' to 'enable-chunk-handler-thread'

Signed-off-by: Michael Oleske <moleske@pivotal.io>
Signed-off-by: Ryan McMahon <mcmellawat@gmail.com>
Signed-off-by: Ivan Godwin <igodwin@pivotal.io>

clicache/integration-test2/geode.properties

@@ -85,7 +85,7 @@ durable-timeout=12345s
 #redundancy-monitor-interval=10
 #auto-ready-for-events=true
 #suspended-tx-timeout=30
-#disable-chunk-handler-thread=false
+#enable-chunk-handler-thread=false
 #tombstone-timeout=480000
 #
 ## module name of the initializer pointing to sample

cppcache/include/geode/SystemProperties.hpp

@@ -258,17 +258,15 @@ class APACHE_GEODE_EXPORT SystemProperties {
   }
 
   /**
-   * This can be call to know whether chunkhandler thread is disable for that
-   * opertaion
+   * Returns true if chunk handler thread is enabled, false if not
    */
-  bool disableChunkHandlerThread() const { return m_disableChunkHandlerThread; }
+  bool enableChunkHandlerThread() const { return m_enableChunkHandlerThread; }
 
   /**
-   * This can be call multiple time to disable chunkhandler thread for those
-   * operations
+   * Enables or disables the chunk handler thread
    */
-  void setDisableChunkHandlerThread(bool set) {
-    m_disableChunkHandlerThread = set;
+  void setEnableChunkHandlerThread(bool set) {
+    m_enableChunkHandlerThread = set;
   }
 
   /**
@@ -423,7 +421,7 @@ class APACHE_GEODE_EXPORT SystemProperties {
   uint32_t m_threadPoolSize;
   std::chrono::seconds m_suspendedTxTimeout;
   std::chrono::milliseconds m_tombstoneTimeout;
-  bool m_disableChunkHandlerThread;
+  bool m_enableChunkHandlerThread;
   bool m_onClientDisconnectClearPdxTypeIds;
 
   /**

cppcache/integration-test-2/CMakeLists.txt

@@ -27,7 +27,9 @@ add_executable(integration-test-2
   framework/GfshExecute.h
   RegionPutGetAllTest.cpp
   PdxInstanceTest.cpp
-  StructTest.cpp)
+  StructTest.cpp
+  EnableChunkHandlerThreadTest.cpp
+  )
 
 target_compile_definitions(integration-test-2
   PUBLIC

cppcache/integration-test-2/EnableChunkHandlerThreadTest.cpp

@@ -0,0 +1,138 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include <thread>
+
+#include <gtest/gtest.h>
+
+#include <geode/PdxSerializable.hpp>
+#include <geode/RegionShortcut.hpp>
+#include <geode/PdxReader.hpp>
+#include <geode/PdxWriter.hpp>
+#include <geode/QueryService.hpp>
+#include <geode/RegionFactory.hpp>
+#include <geode/TypeRegistry.hpp>
+
+#include "framework/Cluster.h"
+
+namespace {
+
+using namespace apache::geode::client;
+
+class SerializableWithThreadId : public PdxSerializable {
+ public:
+  SerializableWithThreadId() : SerializableWithThreadId(0) {}
+
+  SerializableWithThreadId(uint32_t id) : id_(id) {}
+
+  ~SerializableWithThreadId() noexcept override = default;
+
+  using PdxSerializable::fromData;
+  using PdxSerializable::toData;
+
+  void fromData(PdxReader& pdxReader) override {
+    id_ = static_cast<uint32_t>(pdxReader.readLong("id_"));
+    thread_id_ = std::this_thread::get_id();
+  }
+
+  void toData(PdxWriter& pdxWriter) const override {
+    pdxWriter.writeLong("id_", id_);
+    pdxWriter.markIdentityField("id_");
+  }
+
+  static std::shared_ptr<PdxSerializable> createDeserializable() {
+    return std::make_shared<SerializableWithThreadId>();
+  }
+
+  const std::string& getClassName() const override {
+    static const std::string CLASS_NAME =
+        "com.example.SerializableWithThreadId";
+    return CLASS_NAME;
+  }
+
+  std::thread::id getThreadId() { return thread_id_; }
+
+ private:
+  uint32_t id_;
+  std::thread::id thread_id_;
+};
+
+TEST(ChunkHandlerThreadTest, isDisabledUsesSameThread) {
+  Cluster cluster{LocatorCount{1}, ServerCount{1}};
+  cluster.getGfsh()
+      .create()
+      .region()
+      .withName("region")
+      .withType("PARTITION")
+      .execute();
+
+  auto cache = cluster.createCache();
+  auto region = cache.createRegionFactory(RegionShortcut::PROXY)
+                    .setPoolName("default")
+                    .create("region");
+
+  cache.getTypeRegistry().registerPdxType(
+      SerializableWithThreadId::createDeserializable);
+
+  auto objectOne = std::make_shared<SerializableWithThreadId>(1);
+
+  region->put("objectOne", objectOne);
+
+  auto queryResults =
+      cache.getQueryService()->newQuery("select * from /region")->execute();
+
+  auto returnedObjectOne =
+      std::dynamic_pointer_cast<SerializableWithThreadId>((*queryResults)[0]);
+
+  ASSERT_NE(nullptr, returnedObjectOne);
+
+  EXPECT_EQ(std::this_thread::get_id(), returnedObjectOne->getThreadId());
+}
+
+TEST(ChunkHandlerThreadTest, isEnabledUsesDifferentThread) {
+  Cluster cluster{LocatorCount{1}, ServerCount{1}};
+  cluster.getGfsh()
+      .create()
+      .region()
+      .withName("region")
+      .withType("PARTITION")
+      .execute();
+
+  auto cache = cluster.createCache({{"enable-chunk-handler-thread", "true"}});
+  auto region = cache.createRegionFactory(RegionShortcut::PROXY)
+                    .setPoolName("default")
+                    .create("region");
+
+  cache.getTypeRegistry().registerPdxType(
+      SerializableWithThreadId::createDeserializable);
+
+  auto objectOne = std::make_shared<SerializableWithThreadId>(1);
+
+  region->put("objectOne", objectOne);
+
+  auto queryResults =
+      cache.getQueryService()->newQuery("select * from /region")->execute();
+
+  auto returnedObjectOne =
+      std::dynamic_pointer_cast<SerializableWithThreadId>((*queryResults)[0]);
+
+  ASSERT_NE(nullptr, returnedObjectOne);
+
+  EXPECT_NE(std::this_thread::get_id(), returnedObjectOne->getThreadId());
+}
+
+}  // namespace

cppcache/integration-test-2/framework/Cluster.h

@@ -208,12 +208,19 @@ class Cluster {
 
   void stop();
 
-  apache::geode::client::Cache createCache() {
+
+  apache::geode::client::Cache createCache(const std::unordered_map<std::string, std::string>& properties) {
     using namespace apache::geode::client;
-    auto cache = CacheFactory()
-                     .set("log-level", "none")
-                     .set("statistic-sampling-enabled", "false")
-                     .create();
+    CacheFactory cacheFactory;
+
+    for (auto&& property : properties) {
+      cacheFactory.set(property.first, property.second);
+    }
+
+    auto cache = cacheFactory
+            .set("log-level", "none")
+            .set("statistic-sampling-enabled", "false")
+            .create();
 
     auto poolFactory = cache.getPoolManager().createFactory();
     for (const auto &locator : locators_) {
@@ -225,6 +232,10 @@ class Cluster {
     return cache;
   }
 
+  apache::geode::client::Cache createCache() {
+    return createCache({});
+  }
+
   Gfsh &getGfsh() noexcept { return gfsh_; }
 
  private:

cppcache/integration-test/CMakeLists.txt

@@ -216,7 +216,6 @@ set_property(TEST testThinClientTicket303 PROPERTY LABELS OMITTED)
 set_property(TEST testThinClientTicket304 PROPERTY LABELS OMITTED)
 set_property(TEST testThinClientTracking PROPERTY LABELS OMITTED)
 set_property(TEST testThinClientWriterException PROPERTY LABELS OMITTED)
-set_property(TEST testThinClientPoolExecuteFunctionDisableChunkHandlerThread PROPERTY LABELS OMITTED)
 
 add_custom_target(run-stable-cppcache-integration-tests
   COMMAND ctest -C $<CONFIGURATION> -L STABLE