You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@ignite.apache.org by GitBox <gi...@apache.org> on 2023/01/10 07:53:31 UTC

[GitHub] [ignite-3] ptupitsyn commented on a diff in pull request #1507: IGNITE-17604 C++ Client transactions

ptupitsyn commented on code in PR #1507:
URL: https://github.com/apache/ignite-3/pull/1507#discussion_r1065428311


##########
modules/platforms/cpp/ignite/client/detail/transaction/transaction_impl.h:
##########
@@ -0,0 +1,152 @@
+/*
+ * 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.
+ */
+
+#pragma once
+
+#include "ignite/client/detail/node_connection.h"
+
+#include "ignite/common/config.h"
+#include "ignite/common/ignite_result.h"
+
+#include <atomic>
+#include <memory>
+
+namespace ignite::detail {
+
+/**
+ * Ignite transaction implementation.
+ */
+class transaction_impl {
+public:
+    /** Transaction state. */
+    enum class state {
+        OPEN,
+
+        COMMITTED,
+
+        ROLLED_BACK
+    };
+
+    // Deleted
+    transaction_impl() = delete;
+    transaction_impl(transaction_impl &&) noexcept = delete;
+    transaction_impl(const transaction_impl &) = delete;
+    transaction_impl &operator=(transaction_impl &&) noexcept = delete;
+    transaction_impl &operator=(const transaction_impl &) = delete;
+
+    /**
+     * Constructor.
+     *
+     * @param id Transaction ID.
+     * @param connection Connection.
+     */
+    explicit transaction_impl(std::int64_t id, std::shared_ptr<node_connection> connection)
+        : m_id(id)
+        , m_state(state::OPEN)
+        , m_connection(std::move(connection)) {}
+
+    ~transaction_impl() {
+        state old = state::OPEN;
+        if (m_state.compare_exchange_strong(old, state::ROLLED_BACK, std::memory_order_relaxed))
+        {
+            finish(false, [](auto){});
+        }
+    }
+
+    /**
+     * Commits the transaction asynchronously.
+     *
+     * @param callback Callback to be called upon asynchronous operation completion.
+     */
+    void commit_async(ignite_callback<void> callback) {
+        set_state(state::COMMITTED);
+
+        finish(true, std::move(callback));
+    }
+
+    /**
+     * Rollbacks the transaction asynchronously.
+     *
+     * @param callback Callback to be called upon asynchronous operation completion.
+     */
+    void rollback_async(ignite_callback<void> callback) {
+        set_state(state::ROLLED_BACK);
+
+        finish(false, std::move(callback));
+    }
+
+    /**
+     * Get transaction ID.
+     *
+     * @return Transaction ID.
+     */
+    [[nodiscard]] std::int64_t get_id() const {
+        return m_id;
+    }
+
+    /**
+     * Get connection.
+     *
+     * @return Connection.
+     */
+    [[nodiscard]] std::shared_ptr<node_connection> get_connection() const {
+        return m_connection;
+    }
+
+private:
+    /**
+     * Perform operation.
+     *
+     * @param commit Flag indicating should transaction be committed or rolled back.
+     * @param callback Callback to be called upon asynchronous operation completion.
+     */
+    void finish(bool commit, ignite_callback<void> callback) {
+        auto writer_func = [id = m_id](protocol::writer &writer) { writer.write(id); };
+
+        m_connection->perform_request_wr<void>(
+            commit ? client_operation::TX_COMMIT : client_operation::TX_ROLLBACK, writer_func, std::move(callback));
+    }
+
+    /**
+     * Set state.
+     *
+     * @param st State.
+     */
+    void set_state(state st) {
+        state old = state::OPEN;
+        auto opened = m_state.compare_exchange_strong(old, st, std::memory_order_relaxed);
+        if (opened)
+            return;
+
+        auto message = old == state::COMMITTED
+            ? "Transaction is already committed."
+            : "Transaction is already rolled back.";
+
+        throw ignite_error(message);

Review Comment:
   This behavior will be changed soon on the Java side: https://issues.apache.org/jira/browse/IGNITE-18324
   As I understand, commit/rollback will be ignored if a tx is already committed or rolled back.



##########
modules/platforms/cpp/tests/client-test/main.cpp:
##########
@@ -74,21 +74,21 @@ void before_all() {
 
 int main(int argc, char **argv) {
     int res = 0;
-    before_all();
+//    before_all();

Review Comment:
   Is this a temporary change?



##########
modules/runner/src/integrationTest/java/org/apache/ignite/internal/runner/app/PlatformTestNodeRunner.java:
##########
@@ -73,12 +73,12 @@ public class PlatformTestNodeRunner {
     private static final String TABLE_NAME_ALL_COLUMNS_SQL = "tbl_all_columns_sql"; // All column types supported by SQL.
 
     /** Time to keep the node alive. */
-    private static final int RUN_TIME_MINUTES = 30;
+    private static final int RUN_TIME_MINUTES = 300;
 
     /** Nodes bootstrap configuration. */
     private static final Map<String, String> nodesBootstrapCfg = Map.of(
             NODE_NAME, "{\n"
-                    + "  \"clientConnector\":{\"port\": 10942,\"portRange\":10,\"idleTimeout\":3000,\""
+                    + "  \"clientConnector\":{\"port\": 10942,\"portRange\":10,\"idleTimeout\":0,\""

Review Comment:
   Just curious - what's the reason behind this change?



##########
modules/runner/src/integrationTest/java/org/apache/ignite/internal/runner/app/PlatformTestNodeRunner.java:
##########
@@ -73,12 +73,12 @@ public class PlatformTestNodeRunner {
     private static final String TABLE_NAME_ALL_COLUMNS_SQL = "tbl_all_columns_sql"; // All column types supported by SQL.
 
     /** Time to keep the node alive. */
-    private static final int RUN_TIME_MINUTES = 30;
+    private static final int RUN_TIME_MINUTES = 300;

Review Comment:
   The idea behind 30 minutes is to deal with hanging processes on TeamCity - the process will exit by itself after some time. Not sure if 300 minutes makes any sense.



-- 
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: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org