[geode-native] branch develop updated: GEODE-8874: Fix for transaction commit failure (#731)

[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 a467cde  GEODE-8874: Fix for transaction commit failure (#731)
a467cde is described below

commit a467cdefdcb6d3d300be3dbde6663c8b6217004a
Author: Jakov Varenina <62...@users.noreply.github.com>
AuthorDate: Mon Feb 1 18:56:09 2021 +0100

    GEODE-8874: Fix for transaction commit failure (#731)
    
    Issue: Transaction commit fail when m_transactionID integer overflows to
    negative value, because server interprets negative value as
    non-transactional message.
    
    Fix: Client is impacted to reset transactionID to zero when INT_32MAX
    value is reached in order to avoid overflow.
---
 cppcache/src/TXId.cpp                        | 22 ++++++++++--
 cppcache/src/TXId.hpp                        |  4 +++
 cppcache/test/CMakeLists.txt                 |  1 +
 cppcache/{src/TXId.hpp => test/TXIdTest.cpp} | 50 +++++++++++++---------------
 4 files changed, 48 insertions(+), 29 deletions(-)

diff --git a/cppcache/src/TXId.cpp b/cppcache/src/TXId.cpp
index 3b8cc90..acb30db 100644
--- a/cppcache/src/TXId.cpp
+++ b/cppcache/src/TXId.cpp
@@ -27,15 +27,33 @@ namespace apache {
 namespace geode {
 namespace client {
 
-std::atomic<int32_t> TXId::m_transactionId(1);
+std::atomic<int32_t> TXId::m_transactionId(0);
 
-TXId::TXId() : m_TXId(m_transactionId++) {}
+TXId::TXId() {
+  // If m_transactionId has reached maximum value, then start again
+  // from zero, and increment by one until first unused value is found.
+  // This is done to avoid overflow of m_transactionId to negative value.
+  auto current = m_transactionId.load();
+  decltype(current) next;
+  do {
+    next = std::numeric_limits<decltype(current)>::max() == current
+               ? 1
+               : current + 1;
+  } while (!m_transactionId.compare_exchange_strong(current, next));
+  m_TXId = next;
+}
 
 TXId& TXId::operator=(const TXId& other) {
   m_TXId = other.m_TXId;
   return *this;
 }
 
+// This method is only for testing and should not be used for any
+// other purpose. See TXIdTest.cpp for more details.
+void TXId::setInitalTransactionIDValue(int32_t newTransactionID) {
+  m_transactionId.exchange(newTransactionID);
+}
+
 TXId::~TXId() {}
 
 int32_t TXId::getId() { return m_TXId; }
diff --git a/cppcache/src/TXId.hpp b/cppcache/src/TXId.hpp
index 2f2d280..759c959 100644
--- a/cppcache/src/TXId.hpp
+++ b/cppcache/src/TXId.hpp
@@ -45,6 +45,10 @@ class TXId : public apache::geode::client::TransactionId {
 
   int32_t getId();
 
+  // This method is only for testing and should not be used for any
+  // other purpose. See TXIdTest.cpp for more details.
+  static void setInitalTransactionIDValue(int32_t);
+
  private:
   int32_t m_TXId;
   static std::atomic<int32_t> m_transactionId;
diff --git a/cppcache/test/CMakeLists.txt b/cppcache/test/CMakeLists.txt
index a2a3e1e..d0cde8a 100644
--- a/cppcache/test/CMakeLists.txt
+++ b/cppcache/test/CMakeLists.txt
@@ -53,6 +53,7 @@ add_executable(apache-geode_unittests
   StructSetTest.cpp
   TcrMessageTest.cpp
   ThreadPoolTest.cpp
+  TXIdTest.cpp
   mock/MapEntryImplMock.hpp
   statistics/HostStatSamplerTest.cpp
   util/functionalTests.cpp
diff --git a/cppcache/src/TXId.hpp b/cppcache/test/TXIdTest.cpp
similarity index 58%
copy from cppcache/src/TXId.hpp
copy to cppcache/test/TXIdTest.cpp
index 2f2d280..9356307 100644
--- a/cppcache/src/TXId.hpp
+++ b/cppcache/test/TXIdTest.cpp
@@ -1,8 +1,3 @@
-#pragma once
-
-#ifndef GEODE_TXID_H_
-#define GEODE_TXID_H_
-
 /*
  * Licensed to the Apache Software Foundation (ASF) under one or more
  * contributor license agreements.  See the NOTICE file distributed with
@@ -19,39 +14,40 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-/*
- * TXId.h
- *
- *  Created on: 07-Feb-2011
- *      Author: ankurs
- */
 
-#include <atomic>
+#include <TXId.hpp>
 
-#include <geode/DataOutput.hpp>
-#include <geode/TransactionId.hpp>
+#include <gtest/gtest.h>
 
 namespace apache {
 namespace geode {
 namespace client {
 
-class TXId : public apache::geode::client::TransactionId {
- public:
-  TXId();
+TEST(TXIdTest, testIncrementTransactionID) {
+  TXId tx1;
+  ASSERT_EQ(1, tx1.getId());
+
+  TXId tx2;
+  ASSERT_EQ(2, tx2.getId());
+}
 
-  TXId& operator=(const TXId&);
+TEST(TXIdTest, testIncrementTransactionIdOverMaxValue) {
+  // Set inital value of transactionId to integer INT32_MAX.
+  TXId::setInitalTransactionIDValue(INT32_MAX);
 
-  virtual ~TXId();
+  // TransactionId is incremented by one each time TXId is created.
+  // When transactionId is incremented over the INT32_MAX value
+  // then it should be reset to one.
+  TXId tx1;
+  ASSERT_EQ(1, tx1.getId());
 
-  int32_t getId();
+  TXId tx2;
+  ASSERT_EQ(2, tx2.getId());
+
+  // reset value at the end of test
+  TXId::setInitalTransactionIDValue(0);
+}
 
- private:
-  int32_t m_TXId;
-  static std::atomic<int32_t> m_transactionId;
-  TXId(const TXId&);
-};
 }  // namespace client
 }  // namespace geode
 }  // namespace apache
-
-#endif  // GEODE_TXID_H_