You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@geode.apache.org by bb...@apache.org on 2021/03/31 15:54:06 UTC
[geode-native] branch develop updated: GEODE-8968: Fix
toDataMutable coredump (#770)
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 8935ccd GEODE-8968: Fix toDataMutable coredump (#770)
8935ccd is described below
commit 8935ccd0a64fd871a9d8c3d41afa408025a567aa
Author: Mario Salazar de Torres <ma...@est.tech>
AuthorDate: Wed Mar 31 17:53:57 2021 +0200
GEODE-8968: Fix toDataMutable coredump (#770)
- In those scenarios in which Pdx serialization occurs while
PdxTypeRegistry is being cleaned up, it happened that NC crashed
because it couldn't find the PdxType.
- This commit adds retries to Pdx serialization, and in case retries
are exhausted, UnknownPdxTypeException is thrown, so it can be
handled by the user.
- Implemented IT to verify this behaviour.
---
cppcache/include/geode/ExceptionTypes.hpp | 12 +++
cppcache/integration/test/PdxInstanceTest.cpp | 119 ++++++++++++++++++++-
.../integration/test/mock/CacheListenerMock.hpp | 43 ++++++++
cppcache/src/PdxHelper.cpp | 26 +++++
cppcache/src/PdxHelper.hpp | 4 +
cppcache/src/PdxInstanceFactory.cpp | 2 +-
cppcache/src/PdxInstanceImpl.cpp | 13 +--
cppcache/src/SerializationRegistry.cpp | 2 +-
8 files changed, 208 insertions(+), 13 deletions(-)
diff --git a/cppcache/include/geode/ExceptionTypes.hpp b/cppcache/include/geode/ExceptionTypes.hpp
index e18ea63..76ee9c7 100644
--- a/cppcache/include/geode/ExceptionTypes.hpp
+++ b/cppcache/include/geode/ExceptionTypes.hpp
@@ -850,6 +850,18 @@ class APACHE_GEODE_EXPORT SslException : public Exception {
}
};
+/**
+ *@brief Thrown when an unknown pdx type is found.
+ **/
+class APACHE_GEODE_EXPORT UnknownPdxTypeException : public Exception {
+ public:
+ using Exception::Exception;
+ ~UnknownPdxTypeException() noexcept override {}
+ std::string getName() const override {
+ return "apache::geode::client::UnknownPdxTypeException";
+ }
+};
+
} // namespace client
} // namespace geode
} // namespace apache
diff --git a/cppcache/integration/test/PdxInstanceTest.cpp b/cppcache/integration/test/PdxInstanceTest.cpp
index 7a4c89f..2bae1cb 100644
--- a/cppcache/integration/test/PdxInstanceTest.cpp
+++ b/cppcache/integration/test/PdxInstanceTest.cpp
@@ -18,11 +18,7 @@
#include <framework/Cluster.h>
#include <framework/Gfsh.h>
-#include <future>
-#include <initializer_list>
-#include <iostream>
#include <memory>
-#include <thread>
#include <gtest/gtest.h>
@@ -31,24 +27,30 @@
#include <geode/PoolManager.hpp>
#include <geode/RegionFactory.hpp>
#include <geode/RegionShortcut.hpp>
-#include <geode/TypeRegistry.hpp>
#include "CacheImpl.hpp"
#include "LocalRegion.hpp"
#include "NestedPdxObject.hpp"
#include "PdxType.hpp"
+#include "mock/CacheListenerMock.hpp"
namespace {
using apache::geode::client::Cache;
using apache::geode::client::CacheableKey;
using apache::geode::client::CacheableString;
+using apache::geode::client::CacheFactory;
+using apache::geode::client::CacheListenerMock;
+using apache::geode::client::CacheRegionHelper;
using apache::geode::client::IllegalStateException;
using apache::geode::client::LocalRegion;
+using apache::geode::client::PdxInstance;
using apache::geode::client::PdxInstanceFactory;
using apache::geode::client::PdxSerializable;
+using apache::geode::client::PoolFactory;
using apache::geode::client::Region;
using apache::geode::client::RegionShortcut;
+using apache::geode::client::SelectResults;
using PdxTests::Address;
using PdxTests::PdxType;
@@ -56,8 +58,23 @@ using PdxTests::PdxType;
using testobject::ChildPdx;
using testobject::ParentPdx;
+using testing::_;
+using testing::DoAll;
+using testing::InvokeWithoutArgs;
+using testing::Return;
+
const std::string gemfireJsonClassName = "__GEMFIRE_JSON";
+Cache createTestCache() {
+ auto cache = CacheFactory()
+ .set("log-level", "none")
+ .set("on-client-disconnect-clear-pdxType-Ids", "true")
+ .set("statistic-sampling-enabled", "false")
+ .create();
+
+ return cache;
+}
+
std::shared_ptr<Region> setupRegion(Cache& cache) {
auto region = cache.createRegionFactory(RegionShortcut::PROXY)
.setPoolName("default")
@@ -309,4 +326,96 @@ TEST(PdxInstanceTest, testCreateJsonInstance) {
auto retrievedValue = region->get("simpleObject");
}
+TEST(PdxInstanceTest, testInstancePutAfterRestart) {
+ Cluster cluster{LocatorCount{1}, ServerCount{1}};
+ cluster.start();
+
+ auto& gfsh = cluster.getGfsh();
+ gfsh.create().region().withName("region").withType("REPLICATE").execute();
+
+ auto cache = createTestCache();
+ auto poolFactory = cache.getPoolManager()
+ .createFactory()
+ .setSubscriptionEnabled(true)
+ .setPingInterval(std::chrono::seconds{2});
+ cluster.applyLocators(poolFactory);
+ poolFactory.create("default");
+
+ bool status = false;
+ std::mutex mutex_status;
+ std::condition_variable cv_status;
+ auto listener = std::make_shared<CacheListenerMock>();
+ EXPECT_CALL(*listener, afterCreate(_)).WillRepeatedly(Return());
+ EXPECT_CALL(*listener, afterRegionLive(_))
+ .WillRepeatedly(InvokeWithoutArgs([&status, &cv_status] {
+ status = true;
+ cv_status.notify_one();
+ }));
+ EXPECT_CALL(*listener, afterRegionDisconnected(_))
+ .WillRepeatedly(InvokeWithoutArgs([&status, &cv_status] {
+ status = false;
+ cv_status.notify_one();
+ }));
+
+ auto region = cache.createRegionFactory(RegionShortcut::PROXY)
+ .setPoolName("default")
+ .setCacheListener(listener)
+ .create("region");
+
+ std::shared_ptr<PdxInstance> first_instance;
+ std::shared_ptr<PdxInstance> second_instance;
+
+ {
+ auto pdxInstanceFactory =
+ cache.createPdxInstanceFactory(gemfireJsonClassName, false);
+
+ pdxInstanceFactory.writeObject("foo",
+ CacheableString::create(std::string("bar")));
+ first_instance = pdxInstanceFactory.create();
+ }
+
+ {
+ auto pdxInstanceFactory =
+ cache.createPdxInstanceFactory(gemfireJsonClassName, false);
+
+ pdxInstanceFactory.writeObject("random",
+ CacheableString::create(std::string("bar")));
+
+ pdxInstanceFactory.writeInt("bar", -1);
+ second_instance = pdxInstanceFactory.create();
+ }
+
+ region->put("first_instance", first_instance);
+ region->put("second_instance", second_instance);
+
+ gfsh.shutdown().execute();
+
+ {
+ std::unique_lock<std::mutex> lock(mutex_status);
+ cv_status.wait(lock, [&status] { return !status; });
+ }
+
+ std::this_thread::sleep_for(std::chrono::seconds{30});
+
+ for (auto& server : cluster.getServers()) {
+ server.start();
+ }
+
+ {
+ std::unique_lock<std::mutex> lock(mutex_status);
+ cv_status.wait(lock, [&status] { return status; });
+ }
+
+ EXPECT_NO_THROW(region->put("first_instance", first_instance));
+ EXPECT_NO_THROW(region->put("second_instance", second_instance));
+
+ auto qs = cache.getQueryService();
+ auto q = qs->newQuery("SELECT * FROM /region WHERE bar = -1");
+
+ decltype(q->execute()) result;
+ EXPECT_NO_THROW(result = q->execute());
+ EXPECT_TRUE(result);
+ EXPECT_EQ(result->size(), 1UL);
+}
+
} // namespace
diff --git a/cppcache/integration/test/mock/CacheListenerMock.hpp b/cppcache/integration/test/mock/CacheListenerMock.hpp
new file mode 100644
index 0000000..df6fbe0
--- /dev/null
+++ b/cppcache/integration/test/mock/CacheListenerMock.hpp
@@ -0,0 +1,43 @@
+/*
+ * 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.
+ */
+
+#ifndef GEODE_CACHELISTENERMOCK_HPP_
+#define GEODE_CACHELISTENERMOCK_HPP_
+
+#include <gmock/gmock.h>
+
+#include <geode/CacheListener.hpp>
+
+namespace apache {
+namespace geode {
+namespace client {
+class CacheListenerMock : public CacheListener {
+ public:
+ MOCK_METHOD1(afterDestroy,
+ void(const EntryEvent& event));
+ MOCK_METHOD1(afterCreate, void(const EntryEvent&));
+ MOCK_METHOD1(afterRegionLive,
+ void(const RegionEvent&));
+ MOCK_METHOD1(afterRegionDisconnected, void(Region&));
+};
+
+} // namespace client
+} // namespace geode
+} // namespace apache
+
+
+#endif // GEODE_CACHELISTENERMOCK_HPP_
diff --git a/cppcache/src/PdxHelper.cpp b/cppcache/src/PdxHelper.cpp
index fc2f97a..9dd7f7f 100644
--- a/cppcache/src/PdxHelper.cpp
+++ b/cppcache/src/PdxHelper.cpp
@@ -47,6 +47,32 @@ PdxHelper::PdxHelper() {}
PdxHelper::~PdxHelper() {}
+void PdxHelper::serializePdxWithRetries(
+ DataOutput& output, const std::shared_ptr<PdxSerializable>& pdxObject,
+ int maxRetries) {
+ auto retries = 0;
+ auto before = output.getCursor();
+
+ for (;;) {
+ try {
+ serializePdx(output, pdxObject);
+ break;
+ } catch (UnknownPdxTypeException&) {
+ if (retries++ >= maxRetries) {
+ throw;
+ } else {
+ if (auto instance =
+ std::dynamic_pointer_cast<PdxInstanceImpl>(pdxObject)) {
+ instance->setPdxId(0);
+ }
+
+ output.advanceCursor(before - output.getCursor());
+ LOGDEBUG("Retrying PDX serialization due to PDX unknown exception");
+ }
+ }
+ }
+}
+
void PdxHelper::serializePdx(
DataOutput& output, const std::shared_ptr<PdxSerializable>& pdxObject) {
auto pdxII = std::dynamic_pointer_cast<PdxInstanceImpl>(pdxObject);
diff --git a/cppcache/src/PdxHelper.hpp b/cppcache/src/PdxHelper.hpp
index 8cd5aa2..89afb0b 100644
--- a/cppcache/src/PdxHelper.hpp
+++ b/cppcache/src/PdxHelper.hpp
@@ -48,6 +48,10 @@ class PdxHelper {
virtual ~PdxHelper();
+ static void serializePdxWithRetries(
+ DataOutput& output, const std::shared_ptr<PdxSerializable>& pdxObject,
+ int maxRetries = 1);
+
static void serializePdx(DataOutput& output,
const std::shared_ptr<PdxSerializable>& pdxObject);
diff --git a/cppcache/src/PdxInstanceFactory.cpp b/cppcache/src/PdxInstanceFactory.cpp
index 269a277..413bcd3 100644
--- a/cppcache/src/PdxInstanceFactory.cpp
+++ b/cppcache/src/PdxInstanceFactory.cpp
@@ -52,7 +52,7 @@ std::shared_ptr<PdxInstance> PdxInstanceFactory::create() {
// Forces registration of PdxType
auto dataOutput = m_cacheImpl.createDataOutput();
- PdxHelper::serializePdx(dataOutput, pi);
+ PdxHelper::serializePdxWithRetries(dataOutput, pi);
return std::move(pi);
}
diff --git a/cppcache/src/PdxInstanceImpl.cpp b/cppcache/src/PdxInstanceImpl.cpp
index 27e7fa5..ceadaf8 100644
--- a/cppcache/src/PdxInstanceImpl.cpp
+++ b/cppcache/src/PdxInstanceImpl.cpp
@@ -1417,6 +1417,11 @@ void PdxInstanceImpl::toData(PdxWriter& writer) const {
void PdxInstanceImpl::toDataMutable(PdxWriter& writer) {
auto pt = getPdxType();
+ if (pt == nullptr) {
+ m_typeId = 0;
+ throw UnknownPdxTypeException("Unknown pdx type while serializing");
+ }
+
std::vector<std::shared_ptr<PdxFieldType>>* pdxFieldList =
pt->getPdxFieldTypes();
int position = 0; // ignore typeid and length
@@ -1483,12 +1488,8 @@ const std::string& PdxInstanceImpl::getClassName() const {
}
void PdxInstanceImpl::setPdxId(int32_t typeId) {
- if (m_typeId == 0) {
- m_typeId = typeId;
- m_pdxType = nullptr;
- } else {
- throw IllegalStateException("PdxInstance's typeId is already set.");
- }
+ m_pdxType->setTypeId(typeId);
+ m_typeId = typeId;
}
std::vector<std::shared_ptr<PdxFieldType>>
diff --git a/cppcache/src/SerializationRegistry.cpp b/cppcache/src/SerializationRegistry.cpp
index a6d81c7..661435b 100644
--- a/cppcache/src/SerializationRegistry.cpp
+++ b/cppcache/src/SerializationRegistry.cpp
@@ -594,7 +594,7 @@ void TheTypeMap::unbindPdxSerializable(const std::string& objFullName) {
void PdxTypeHandler::serialize(
const std::shared_ptr<PdxSerializable>& pdxSerializable,
DataOutput& dataOutput) const {
- PdxHelper::serializePdx(dataOutput, pdxSerializable);
+ PdxHelper::serializePdxWithRetries(dataOutput, pdxSerializable);
}
std::shared_ptr<PdxSerializable> PdxTypeHandler::deserialize(