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 2020/07/05 20:57:13 UTC

[GitHub] [ignite] gurustron opened a new pull request #7992: IGNITE-7369 Thin Client Transactions

gurustron opened a new pull request #7992:
URL: https://github.com/apache/ignite/pull/7992


   Implemented  Thin Client Transactions


----------------------------------------------------------------
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.

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



[GitHub] [ignite] gurustron commented on a change in pull request #7992: IGNITE-7369 .NET: Thin Client Transactions

Posted by GitBox <gi...@apache.org>.
gurustron commented on a change in pull request #7992:
URL: https://github.com/apache/ignite/pull/7992#discussion_r490360071



##########
File path: modules/platforms/dotnet/Apache.Ignite.Core/Impl/Client/Transactions/TransactionClient.cs
##########
@@ -0,0 +1,172 @@
+/*
+ * 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.
+ */
+
+namespace Apache.Ignite.Core.Impl.Client.Transactions
+{
+    using System;
+    using System.Globalization;
+    using Apache.Ignite.Core.Client.Transactions;
+    using Apache.Ignite.Core.Transactions;
+
+    /// <summary>
+    /// Ignite Thin Client transaction facade.
+    /// </summary>
+    internal class TransactionClient : ITransactionClient
+    {
+        /** Unique  transaction ID.*/
+        private readonly int _id;
+
+        /** Ignite. */
+        private readonly IgniteClient _ignite;
+
+        /** Socket. */
+        private readonly ClientSocket _socket;
+
+        /** Transaction is closed. */
+        private bool _closed;
+
+        /// <summary>
+        /// Constructor.
+        /// </summary>
+        /// <param name="id">ID.</param>
+        /// <param name="ignite">Ignite.</param>
+        /// <param name="socket">Socket.</param>
+        /// <param name="concurrency">Concurrency.</param>
+        /// <param name="isolation">Isolation.</param>
+        /// <param name="timeout">Timeout.</param>
+        /// <param name="label">Label.</param>
+        public TransactionClient(int id,
+            IgniteClient ignite,
+            ClientSocket socket,
+            TransactionConcurrency concurrency,
+            TransactionIsolation isolation,
+            TimeSpan timeout,
+            string label)
+        {
+            _id = id;
+            _ignite = ignite;
+            _socket = socket;
+            Concurrency = concurrency;
+            Isolation = isolation;
+            Timeout = timeout;
+            Label = label;
+        }
+
+        /** <inheritdoc /> */
+        public void Commit()
+        {
+            ThrowIfClosed();
+            Close(true);
+        }
+
+        /** <inheritdoc /> */
+        public void Rollback()
+        {
+            ThrowIfClosed();
+            Close(false);
+        }
+
+        /** <inheritdoc /> */
+        public TransactionConcurrency Concurrency { get; private set; }
+
+        /** <inheritdoc /> */
+        public TransactionIsolation Isolation { get; private set; }
+
+        /** <inheritdoc /> */
+        public TimeSpan Timeout { get; private set; }
+
+        /** <inheritdoc /> */
+        public string Label { get; private set; }
+
+        /** <inheritdoc /> */
+        public void Dispose()
+        {
+            try
+            {
+                Close(false);
+            }
+            finally
+            {
+                GC.SuppressFinalize(this);
+            }
+        }
+
+        /// <summary>
+        /// Transaction Id.
+        /// </summary>
+        public int Id
+        {
+            get { return _id; }
+        }
+
+        public ClientSocket Socket
+        {
+            get { return _socket; }
+        }
+
+        /// <summary>
+        /// Returns if transaction is closed.
+        /// </summary>
+        internal bool Closed
+        {
+            get { return _closed; }
+        }
+
+        /// <summary>
+        /// Closes the transaction. 
+        /// </summary>
+        private void Close(bool commit)
+        {
+            if (!_closed)
+            {
+                try
+                {
+                    _ignite.Socket.DoOutInOp<object>(ClientOp.TxEnd,

Review comment:
       Fixed.




----------------------------------------------------------------
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.

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



[GitHub] [ignite] ptupitsyn merged pull request #7992: IGNITE-7369 .NET: Thin Client Transactions

Posted by GitBox <gi...@apache.org>.
ptupitsyn merged pull request #7992:
URL: https://github.com/apache/ignite/pull/7992


   


----------------------------------------------------------------
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.

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



[GitHub] [ignite] ptupitsyn commented on a change in pull request #7992: IGNITE-7369 Thin Client Transactions [DRAFT]

Posted by GitBox <gi...@apache.org>.
ptupitsyn commented on a change in pull request #7992:
URL: https://github.com/apache/ignite/pull/7992#discussion_r456802106



##########
File path: modules/platforms/dotnet/Apache.Ignite.Core/Client/Transactions/ITransactionsClient.cs
##########
@@ -0,0 +1,69 @@
+/*
+ * 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.
+ */
+
+namespace Apache.Ignite.Core.Client.Transactions
+{
+    using System;
+    using Apache.Ignite.Core.Transactions;
+
+    /// <summary>
+    /// Ignite Thin Client transactions facade.
+    /// <para /> Transactions are bound to the thread started the transaction. After that, each cache operation within this thread
+    /// will belong to the corresponding transaction until the transaction is committed, rolled back or closed.
+    /// <para /> Should not be used with async calls.
+    /// </summary> 
+    public interface ITransactionsClient
+    {
+        /// <summary>
+        /// Starts a new transaction with the default isolation level, concurrency and timeout.
+        /// <para /> Default values for transaction isolation level, concurrency and timeout can be configured via
+        /// <see cref="TransactionClientConfiguration"/>.
+        /// <para /> Should not be used with async calls.
+        /// </summary>
+        /// <returns>New transaction.</returns>
+        ITransactionClient TxStart();
+
+        /// <summary>
+        /// Starts new transaction with the specified concurrency and isolation.

Review comment:
       `starts a new` - here and below

##########
File path: modules/platforms/dotnet/Apache.Ignite.Core/Impl/Client/ClientFailoverSocket.cs
##########
@@ -54,6 +55,8 @@ internal class ClientFailoverSocket : IDisposable
         /** Marshaller. */
         private readonly Marshaller _marsh;
 
+        private readonly ITransactionsClientInternal _transactions;

Review comment:
       Comment

##########
File path: modules/platforms/dotnet/Apache.Ignite.Core/Impl/Client/Transactions/ITransactionsClientInternal.cs
##########
@@ -0,0 +1,51 @@
+/*
+ * 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.
+ */
+
+namespace Apache.Ignite.Core.Impl.Client.Transactions
+{
+    using System;
+    using Apache.Ignite.Core.Client.Transactions;
+    using Apache.Ignite.Core.Transactions;
+
+    internal interface ITransactionsClientInternal : ITransactionsClient

Review comment:
       xmldoc

##########
File path: modules/platforms/dotnet/Apache.Ignite.Core/Client/Transactions/ITransactionsClient.cs
##########
@@ -0,0 +1,69 @@
+/*
+ * 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.
+ */
+
+namespace Apache.Ignite.Core.Client.Transactions
+{
+    using System;
+    using Apache.Ignite.Core.Transactions;
+
+    /// <summary>
+    /// Ignite Thin Client transactions facade.
+    /// <para /> Transactions are bound to the thread started the transaction. After that, each cache operation within this thread
+    /// will belong to the corresponding transaction until the transaction is committed, rolled back or closed.
+    /// <para /> Should not be used with async calls.
+    /// </summary> 
+    public interface ITransactionsClient
+    {
+        /// <summary>
+        /// Starts a new transaction with the default isolation level, concurrency and timeout.
+        /// <para /> Default values for transaction isolation level, concurrency and timeout can be configured via
+        /// <see cref="TransactionClientConfiguration"/>.
+        /// <para /> Should not be used with async calls.
+        /// </summary>
+        /// <returns>New transaction.</returns>
+        ITransactionClient TxStart();
+
+        /// <summary>
+        /// Starts new transaction with the specified concurrency and isolation.
+        /// <para /> Should not be used with async calls.
+        /// </summary>
+        /// <param name="concurrency">Concurrency.</param>
+        /// <param name="isolation">Isolation.</param>
+        /// <returns>New transaction.</returns>
+        ITransactionClient TxStart(TransactionConcurrency concurrency, TransactionIsolation isolation);
+
+        /// <summary>
+        /// Starts new transaction with the specified concurrency, isolation and timeout.
+        /// <para /> Should not be used with async calls.
+        /// </summary>
+        /// <param name="concurrency">Concurrency.</param>
+        /// <param name="isolation">Isolation.</param>
+        /// <param name="timeout">Timeout. TimeSpan. Zero for indefinite timeout.</param>
+        /// <returns>New transaction.</returns>
+        ITransactionClient TxStart(TransactionConcurrency concurrency, TransactionIsolation isolation, TimeSpan timeout);
+
+        /// <summary>
+        /// Returns instance of <see cref="ITransactionsClient"/> to mark a transaction with a special label.
+        /// The label is helpful for diagnostic and exposed to some diagnostic tools like
+        /// SYS.TRANSACTIONS system view, control.sh commands, JMX TransactionsMXBean,
+        /// long-running transactions dump in logs.

Review comment:
       We can add `<see cref="ITransaction.Label" /> and <see cref="ITransactions.GetLocalActiveTransactions" />`

##########
File path: modules/platforms/dotnet/Apache.Ignite.Core/Client/Transactions/ITransactionsClient.cs
##########
@@ -0,0 +1,69 @@
+/*
+ * 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.
+ */
+
+namespace Apache.Ignite.Core.Client.Transactions
+{
+    using System;
+    using Apache.Ignite.Core.Transactions;
+
+    /// <summary>
+    /// Ignite Thin Client transactions facade.
+    /// <para /> Transactions are bound to the thread started the transaction. After that, each cache operation within this thread
+    /// will belong to the corresponding transaction until the transaction is committed, rolled back or closed.
+    /// <para /> Should not be used with async calls.
+    /// </summary> 
+    public interface ITransactionsClient
+    {
+        /// <summary>
+        /// Starts a new transaction with the default isolation level, concurrency and timeout.
+        /// <para /> Default values for transaction isolation level, concurrency and timeout can be configured via
+        /// <see cref="TransactionClientConfiguration"/>.
+        /// <para /> Should not be used with async calls.
+        /// </summary>
+        /// <returns>New transaction.</returns>
+        ITransactionClient TxStart();
+
+        /// <summary>
+        /// Starts new transaction with the specified concurrency and isolation.
+        /// <para /> Should not be used with async calls.
+        /// </summary>
+        /// <param name="concurrency">Concurrency.</param>
+        /// <param name="isolation">Isolation.</param>
+        /// <returns>New transaction.</returns>
+        ITransactionClient TxStart(TransactionConcurrency concurrency, TransactionIsolation isolation);
+
+        /// <summary>
+        /// Starts new transaction with the specified concurrency, isolation and timeout.
+        /// <para /> Should not be used with async calls.
+        /// </summary>
+        /// <param name="concurrency">Concurrency.</param>
+        /// <param name="isolation">Isolation.</param>
+        /// <param name="timeout">Timeout. TimeSpan. Zero for indefinite timeout.</param>
+        /// <returns>New transaction.</returns>
+        ITransactionClient TxStart(TransactionConcurrency concurrency, TransactionIsolation isolation, TimeSpan timeout);
+
+        /// <summary>
+        /// Returns instance of <see cref="ITransactionsClient"/> to mark a transaction with a special label.

Review comment:
       `an instance`

##########
File path: modules/platforms/dotnet/Apache.Ignite.Core/Client/Transactions/TransactionClientConfiguration.cs
##########
@@ -0,0 +1,81 @@
+/*
+ * 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.
+ */
+
+namespace Apache.Ignite.Core.Client.Transactions
+{
+    using System;
+    using System.ComponentModel;
+    using Apache.Ignite.Core.Impl.Common;
+    using Apache.Ignite.Core.Transactions;
+
+    /// <summary>
+    /// Transactions configuration.
+    /// </summary>
+    public class TransactionClientConfiguration
+    {
+        /// <summary> The default value for <see cref="DefaultTransactionConcurrency"/> property. </summary>
+        public const TransactionConcurrency DefaultDefaultTransactionConcurrency = TransactionConcurrency.Pessimistic;
+
+        /// <summary> The default value for <see cref="DefaultTransactionIsolation"/> property. </summary>
+        public const TransactionIsolation DefaultDefaultTransactionIsolation = TransactionIsolation.RepeatableRead;
+
+        /// <summary> The default value for <see cref="DefaultTransactionIsolation"/> property. </summary>
+        public static readonly TimeSpan DefaultDefaultTimeout = TimeSpan.Zero;
+
+        /// <summary>
+        /// Gets or sets the cache transaction concurrency to use when one is not explicitly specified.
+        /// </summary>
+        [DefaultValue(DefaultDefaultTransactionConcurrency)]
+        public TransactionConcurrency DefaultTransactionConcurrency { get; set; }
+
+        /// <summary>
+        /// Gets or sets the cache transaction isolation to use when one is not explicitly specified.
+        /// </summary>
+        [DefaultValue(DefaultDefaultTransactionIsolation)]
+        public TransactionIsolation DefaultTransactionIsolation { get; set; }
+
+        /// <summary>
+        /// Gets or sets the cache transaction timeout to use when one is not explicitly specified.
+        /// <see cref="TimeSpan.Zero"/> for infinite timeout.
+        /// </summary>
+        [DefaultValue(typeof(TimeSpan), "00:00:00")]
+        public TimeSpan DefaultTimeout { get; set; }
+
+        /// <summary>
+        /// Initializes a new instance of the <see cref="TransactionClientConfiguration" /> class.
+        /// </summary>
+        public TransactionClientConfiguration()
+        {
+            DefaultTransactionConcurrency = DefaultDefaultTransactionConcurrency;
+            DefaultTransactionIsolation = DefaultDefaultTransactionIsolation;
+            DefaultTimeout = DefaultDefaultTimeout;
+        }
+        
+        /// <summary>
+        /// Initializes a new instance of the <see cref="TransactionClientConfiguration" /> class.
+        /// </summary>
+        /// <param name="configuration">The configuration to copy.</param>
+        public TransactionClientConfiguration(TransactionClientConfiguration configuration)
+        {
+            IgniteArgumentCheck.NotNull(configuration, "configuration");
+
+            DefaultTransactionConcurrency = DefaultDefaultTransactionConcurrency;

Review comment:
       We should copy the values from the `configuration` parameter, right? Please add a test for this.

##########
File path: modules/platforms/dotnet/Apache.Ignite.Core.Tests/Client/Cache/CacheClientAbstractTxTest.cs
##########
@@ -0,0 +1,756 @@
+/*
+ * 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.
+ */
+
+namespace Apache.Ignite.Core.Tests.Client.Cache
+{
+    using System;
+    using System.Collections.Generic;
+    using System.Linq;
+    using System.Threading;
+    using System.Transactions;
+    using Apache.Ignite.Core.Cache.Configuration;
+    using Apache.Ignite.Core.Client;
+    using Apache.Ignite.Core.Client.Cache;
+    using Apache.Ignite.Core.Client.Transactions;
+    using Apache.Ignite.Core.Impl.Client.Transactions;
+    using Apache.Ignite.Core.Transactions;
+    using NUnit.Framework;
+    using NUnit.Framework.Constraints;
+
+    /// <summary>
+    /// Transactional cache client tests.
+    /// </summary>
+    public abstract class CacheClientAbstractTxTest : ClientTestBase
+    {
+        /** All concurrency controls. */
+        private static readonly TransactionConcurrency[] AllConcurrencyControls =
+        {
+            TransactionConcurrency.Optimistic, 
+            TransactionConcurrency.Pessimistic
+        };
+
+        /** All isolation levels*/
+        private static readonly TransactionIsolation[] AllIsolationLevels = 
+        {
+            TransactionIsolation.Serializable,
+            TransactionIsolation.ReadCommitted,
+            TransactionIsolation.RepeatableRead
+        };
+
+        protected CacheClientAbstractTxTest(int serverCount, bool enablePartitionAwareness) : base(serverCount,
+            enablePartitionAwareness: enablePartitionAwareness)
+        {
+            // No-op.
+        }
+
+        /// <summary>
+        /// Tests that custom client transactions configuration is applied.
+        /// </summary>
+        [Test]
+        public void TestClientTransactionConfiguration()
+        {
+            var timeout = TransactionClientConfiguration.DefaultDefaultTimeout.Add(TimeSpan.FromMilliseconds(1000));
+            var cfg = GetClientConfiguration();
+            cfg.TransactionConfiguration = new TransactionClientConfiguration
+            {
+                DefaultTimeout = timeout
+            };
+
+            foreach (var concurrency in AllConcurrencyControls)
+            {
+                foreach (var isolation in AllIsolationLevels)
+                {
+                    cfg.TransactionConfiguration.DefaultTransactionConcurrency = concurrency;
+                    cfg.TransactionConfiguration.DefaultTransactionIsolation = isolation;
+                    using (var client = Ignition.StartClient(cfg))
+                    {
+                        ITransaction tx;
+                        using (client.GetTransactions().TxStart())
+                        {
+                            tx = GetSingleLocalTransaction();
+                            Assert.AreEqual(concurrency, tx.Concurrency);
+                            Assert.AreEqual(isolation, tx.Isolation);
+                            Assert.AreEqual(timeout, tx.Timeout);
+                        }
+                        tx.Dispose();
+                    }
+                }
+            }
+        }
+
+        /// <summary>
+        /// Tests that parameters passed to TxStart are applied.
+        /// </summary>
+        [Test]
+        public void TestTxStartPassesParameters()
+        {
+            var timeout = TransactionClientConfiguration.DefaultDefaultTimeout.Add(TimeSpan.FromMilliseconds(1000));
+            var acts = new List<Func<ITransactionsClient>>
+            {
+                () => Client.GetTransactions(),
+                () => Client.GetTransactions().WithLabel("label"),
+            };
+            foreach (var concurrency in AllConcurrencyControls)
+            {
+                foreach (var isolation in AllIsolationLevels)
+                {
+                    foreach (var act in acts)
+                    {
+                        ITransaction tx;
+                        using (act().TxStart(concurrency, isolation))
+                        {
+                            tx = GetSingleLocalTransaction();
+                            Assert.AreEqual(concurrency, tx.Concurrency);
+                            Assert.AreEqual(isolation, tx.Isolation);
+                        }
+                        tx.Dispose();
+                        using (act().TxStart(concurrency, isolation, timeout))
+                        {
+                            tx = GetSingleLocalTransaction();
+                            Assert.AreEqual(concurrency, tx.Concurrency);
+                            Assert.AreEqual(isolation, tx.Isolation);
+                            Assert.AreEqual(timeout, tx.Timeout);
+                        }
+                        tx.Dispose();
+                    }
+                }
+            }
+        }
+
+        /// <summary>
+        /// Tests that transaction can't be committed/rollback after being already completed.
+        /// </summary>
+        [Test]
+        public void TestThrowsIfEndAlreadyCompletedTransaction()
+        {
+            var tx = Client.GetTransactions().TxStart();
+            tx.Commit();
+
+            var constraint = new ReusableConstraint(Is.TypeOf<InvalidOperationException>()
+               .And.Message.Contains("Transaction")
+               .And.Message.Contains("is closed"));
+
+            Assert.Throws(constraint, () => tx.Commit());
+            Assert.Throws(constraint, () => tx.Rollback());
+
+            using (tx = Client.GetTransactions().TxStart())
+            {
+            }
+
+            Assert.Throws(constraint, () => tx.Commit());
+            Assert.Throws(constraint, () => tx.Rollback());
+        }
+
+
+        /// <summary>
+        /// Tests that transaction throws if timeout elapsed.
+        /// </summary>
+        [Test]
+        public void TestTimeout()
+        {
+            var timeout = TimeSpan.FromMilliseconds(200);
+            var cache = GetTransactionalCache();
+            cache.Put(1, 1);
+            using (var tx = Client.GetTransactions().TxStart(TransactionConcurrency.Pessimistic,
+                TransactionIsolation.ReadCommitted,
+                timeout))
+            {
+                Thread.Sleep(TimeSpan.FromMilliseconds(300));
+                var constraint = new ReusableConstraint(Is.TypeOf<IgniteClientException>()
+                   .And.Message.Contains("Cache transaction timed out"));
+                Assert.Throws(constraint, () => cache.Put(1, 10));
+                Assert.Throws(constraint, () => tx.Commit());
+            }
+
+            Assert.AreEqual(1, cache.Get(1));
+        }
+
+        /// <summary>
+        /// Tests that commit applies cache changes.
+        /// </summary>
+        [Test]
+        public void TestTxCommit()
+        {
+            var cache = GetTransactionalCache();
+
+            cache.Put(1, 1);
+            cache.Put(2, 2);
+
+            using (var tx = Client.GetTransactions().TxStart())
+            {
+                cache.Put(1, 10);
+                cache.Put(2, 20);
+
+                tx.Commit();
+            }
+
+            Assert.AreEqual(10, cache.Get(1));
+            Assert.AreEqual(20, cache.Get(2));
+        }
+
+        /// <summary>
+        /// Tests that rollback reverts cache changes.
+        /// </summary>
+        [Test]
+        public void TestTxRollback()
+        {
+            var cache = GetTransactionalCache();
+            cache.Put(1, 1);
+            cache.Put(2, 2);
+
+            using (var tx = Client.GetTransactions().TxStart())
+            {
+                cache.Put(1, 10);
+                cache.Put(2, 20);
+
+                Assert.AreEqual(10, cache.Get(1));
+                Assert.AreEqual(20, cache.Get(2));
+                tx.Rollback();
+            }
+
+            Assert.AreEqual(1, cache.Get(1));
+            Assert.AreEqual(2, cache.Get(2));
+        }
+
+        /// <summary>
+        /// Tests that closing transaction without commit reverts cache changes.
+        /// </summary>
+        [Test]
+        public void TestTxClose()
+        {
+            var cache = GetTransactionalCache();
+
+            cache.Put(1, 1);
+            cache.Put(2, 2);
+
+            using (Client.GetTransactions().TxStart())
+            {
+                cache.Put(1, 10);
+                cache.Put(2, 20);
+            }
+
+            Assert.AreEqual(1, cache.Get(1));
+            Assert.AreEqual(2, cache.Get(2));
+        }
+
+        /// <summary>
+        /// Tests that client can't start multiple transactions in one thread.
+        /// </summary>
+        [Test]
+        public void TestThrowsIfMultipleStarted()
+        {
+            TestThrowsIfMultipleStarted(
+                () => Client.GetTransactions().TxStart(),
+                () => Client.GetTransactions().TxStart());
+        }
+
+        /// <summary>
+        /// Tests that different clients can start transactions in one thread.
+        /// </summary>
+        [Test]
+        public void TestDifferentClientsCanStartTransactions()
+        {
+            Assert.DoesNotThrow(() =>
+            {
+                using (Client.GetTransactions().TxStart())
+                using (GetClient().GetTransactions().TxStart())
+                {
+                    // No-op.
+                }
+            });
+        }
+
+        /// <summary>
+        /// Test Ignite thin client transaction with label.
+        /// </summary>
+        [Test]
+        public void TestWithLabel()
+        {
+            const string label1 = "label1";
+            const string label2 = "label2";
+
+            var cache = GetTransactionalCache();
+            cache.Put(1, 1);
+            cache.Put(2, 2);
+
+            ITransaction igniteTx;
+            using (Client.GetTransactions().WithLabel(label1).TxStart())
+            {
+                igniteTx = GetSingleLocalTransaction();
+
+                Assert.AreEqual(igniteTx.Label, label1);
+
+                cache.Put(1, 10);
+                cache.Put(2, 20);
+            }
+            igniteTx.Dispose();
+
+            Assert.AreEqual(1, cache.Get(1));
+            Assert.AreEqual(2, cache.Get(2));
+
+            using (var tx = Client.GetTransactions().WithLabel(label1).TxStart())
+            {
+                igniteTx = GetSingleLocalTransaction();
+
+                Assert.AreEqual(igniteTx.Label, label1);
+
+                cache.Put(1, 10);
+                cache.Put(2, 20);
+                tx.Commit();
+            }
+            igniteTx.Dispose();
+
+            Assert.AreEqual(10, cache.Get(1));
+            Assert.AreEqual(20, cache.Get(2));
+
+            using (Client.GetTransactions().WithLabel(label1).WithLabel(label2).TxStart())
+            {
+                igniteTx = GetSingleLocalTransaction();
+
+                Assert.AreEqual(igniteTx.Label, label2);
+            }
+            igniteTx.Dispose();
+
+            TestThrowsIfMultipleStarted(
+                () => Client.GetTransactions().WithLabel(label1).TxStart(),
+                () => Client.GetTransactions().TxStart());
+
+            TestThrowsIfMultipleStarted(
+                () => Client.GetTransactions().TxStart(),
+                () => Client.GetTransactions().WithLabel(label1).TxStart());
+
+            TestThrowsIfMultipleStarted(
+                () => Client.GetTransactions().WithLabel(label1).TxStart(),
+                () => Client.GetTransactions().WithLabel(label2).TxStart());
+        }
+
+        /// <summary>
+        /// Test Ignite thin client transaction enlistment in ambient <see cref="TransactionScope"/>.
+        /// </summary>
+        [Test]
+        public void TestTransactionScopeSingleCache()
+        {
+            var cache = GetTransactionalCache();
+
+            cache[1] = 1;
+            cache[2] = 2;
+
+            // Commit.
+            using (var ts = new TransactionScope())
+            {
+                cache[1] = 10;
+                cache[2] = 20;
+
+                ts.Complete();
+            }
+
+            Assert.AreEqual(10, cache[1]);
+            Assert.AreEqual(20, cache[2]);
+
+            // Rollback.
+            using (new TransactionScope())
+            {
+                cache[1] = 100;
+                cache[2] = 200;
+            }
+
+            Assert.AreEqual(10, cache[1]);
+            Assert.AreEqual(20, cache[2]);
+        }
+
+        /// <summary>
+        /// Test Ignite thin client transaction enlistment in ambient <see cref="TransactionScope"/>
+        /// with multiple participating caches.
+        /// </summary>
+        [Test]
+        public void TestTransactionScopeMultiCache()
+        {
+            var cache1 = GetTransactionalCache();
+            var cache2 = GetTransactionalCache(cache1.Name + "1");
+
+            cache1[1] = 1;
+            cache2[1] = 2;
+
+            // Commit.
+            using (var ts = new TransactionScope())
+            {
+                cache1.Put(1, 10);
+                cache2.Put(1, 20);
+
+                ts.Complete();
+            }
+
+            Assert.AreEqual(10, cache1[1]);
+            Assert.AreEqual(20, cache2[1]);
+
+            // Rollback.
+            using (new TransactionScope())
+            {
+                cache1.Put(1, 100);
+                cache2.Put(1, 200);
+            }
+
+            Assert.AreEqual(10, cache1[1]);
+            Assert.AreEqual(20, cache2[1]);
+        }
+
+        /// <summary>
+        /// Test Ignite thin client transaction enlistment in ambient <see cref="TransactionScope"/>
+        /// when Ignite tx is started manually.
+        /// </summary>
+        [Test]
+        public void TestTransactionScopeWithManualIgniteTx()
+        {
+            var cache = GetTransactionalCache();
+            var transactions = Client.GetTransactions();
+
+            cache[1] = 1;
+
+            // When Ignite tx is started manually, it won't be enlisted in TransactionScope.
+            using (var tx = transactions.TxStart())
+            {
+                using (new TransactionScope())
+                {
+                    cache[1] = 2;
+                }  // Revert transaction scope.
+
+                tx.Commit();  // Commit manual tx.
+            }
+
+            Assert.AreEqual(2, cache[1]);
+        }
+
+        /// <summary>
+        /// Test Ignite transaction with <see cref="TransactionScopeOption.Suppress"/> option.
+        /// </summary>
+        [Test]
+        public void TestSuppressedTransactionScope()
+        {
+            var cache = GetTransactionalCache();
+
+            cache[1] = 1;
+
+            using (new TransactionScope(TransactionScopeOption.Suppress))
+            {
+                cache[1] = 2;
+            }
+
+            // Even though transaction is not completed, the value is updated, because tx is suppressed.
+            Assert.AreEqual(2, cache[1]);
+        }
+
+        /// <summary>
+        /// Test Ignite thin client transaction enlistment in ambient <see cref="TransactionScope"/> with nested scopes.
+        /// </summary>
+        [Test]
+        public void TestNestedTransactionScope()
+        {
+            var cache = GetTransactionalCache();
+
+            cache[1] = 1;
+
+            foreach (var option in new[] {TransactionScopeOption.Required, TransactionScopeOption.RequiresNew})
+            {
+                // Commit.
+                using (var ts1 = new TransactionScope())
+                {
+                    using (var ts2 = new TransactionScope(option))
+                    {
+                        cache[1] = 2;
+                        ts2.Complete();
+                    }
+
+                    cache[1] = 3;
+                    ts1.Complete();
+                }
+
+                Assert.AreEqual(3, cache[1]);
+
+                // Rollback.
+                using (new TransactionScope())
+                {
+                    using (new TransactionScope(option))
+                        cache[1] = 4;
+
+                    cache[1] = 5;
+                }
+
+                // In case with Required option there is a single tx
+                // that gets aborted, second put executes outside the tx.
+                Assert.AreEqual(option == TransactionScopeOption.Required ? 5 : 3, cache[1], option.ToString());
+            }
+        }
+
+        /// <summary>
+        /// Test that ambient <see cref="TransactionScope"/> options propagate to Ignite transaction.
+        /// </summary>
+        [Test]
+        public void TestTransactionScopeOptions()
+        {
+            var cache = GetTransactionalCache();
+            var transactions = (ITransactionsClientInternal) Client.GetTransactions();
+
+            var modes = new[]
+            {
+                Tuple.Create(IsolationLevel.Serializable, TransactionIsolation.Serializable),
+                Tuple.Create(IsolationLevel.RepeatableRead, TransactionIsolation.RepeatableRead),
+                Tuple.Create(IsolationLevel.ReadCommitted, TransactionIsolation.ReadCommitted),
+                Tuple.Create(IsolationLevel.ReadUncommitted, TransactionIsolation.ReadCommitted),
+                Tuple.Create(IsolationLevel.Snapshot, TransactionIsolation.ReadCommitted),
+                Tuple.Create(IsolationLevel.Chaos, TransactionIsolation.ReadCommitted),
+            };
+
+            foreach (var mode in modes)
+            {
+                ITransaction tx;
+                using (new TransactionScope(TransactionScopeOption.Required, new TransactionOptions
+                {
+                    IsolationLevel = mode.Item1
+                }))
+                {
+                    cache[1] = 1;
+
+                    tx = GetSingleLocalTransaction();
+                    Assert.AreEqual(mode.Item2, tx.Isolation);
+                    Assert.AreEqual(transactions.DefaultTxConcurrency, tx.Concurrency);
+                }
+                tx.Dispose();
+            }
+        }
+
+        /// <summary>
+        /// Tests all synchronous transactional operations with <see cref="TransactionScope"/>.
+        /// </summary>
+        [Test]
+        public void TestTransactionScopeAllOperationsSync()
+        {
+            CheckTxOp((cache, key) => cache.Put(key, -5));
+
+            CheckReadTxOp((cache, key) => cache.Get(key));
+            
+            CheckReadTxOp((cache, key) =>
+            {
+                int value;
+                Assert.IsTrue(cache.TryGet(key, out value));
+            });
+
+            CheckReadTxOp((cache, key) => cache.ContainsKey(key));
+
+            CheckReadTxOp((cache, key) => cache.ContainsKeys(new[] {key}));
+
+            CheckTxOp((cache, key) => cache.PutAll(new Dictionary<int, int> {{key, -7}}));
+
+            CheckTxOp((cache, key) =>
+            {
+                cache.Remove(key);
+                cache.PutIfAbsent(key, -10);
+            });
+
+            CheckTxOp((cache, key) => cache.GetAndPut(key, -9));
+
+            CheckTxOp((cache, key) =>
+            {
+                cache.Remove(key);
+                cache.GetAndPutIfAbsent(key, -10);
+            });
+
+            CheckTxOp((cache, key) => cache.GetAndRemove(key));
+
+            CheckTxOp((cache, key) => cache.GetAndReplace(key, -11));
+
+            CheckTxOp((cache, key) => cache.Remove(key));
+
+            CheckTxOp((cache, key) => cache.RemoveAll(new[] {key}));
+
+            CheckTxOp((cache, key) => cache.Replace(key, 100));
+
+            CheckTxOp((cache, key) => cache.Replace(key, cache[key], 100));
+        }
+
+        /// <summary>
+        /// Tests all transactional async operations with <see cref="TransactionScope"/>.
+        /// </summary>
+        [Test]
+        [Ignore("Async thin client transactional operations not supported.")]
+        public void TestTransactionScopeAllOperationsAsync()
+        {
+            CheckTxOp((cache, key) => cache.PutAsync(key, -5));
+            CheckTxOp((cache, key) => cache.PutAllAsync(new Dictionary<int, int> {{key, -7}}));
+
+            CheckTxOp((cache, key) =>
+            {
+                cache.Remove(key);
+                cache.PutIfAbsentAsync(key, -10);
+            });
+
+            CheckTxOp((cache, key) => cache.GetAndPutAsync(key, -9));
+
+            CheckTxOp((cache, key) =>
+            {
+                cache.Remove(key);
+                cache.GetAndPutIfAbsentAsync(key, -10);
+            });
+
+            CheckTxOp((cache, key) => cache.GetAndRemoveAsync(key));
+
+            CheckTxOp((cache, key) => cache.GetAndReplaceAsync(key, -11));
+
+            CheckTxOp((cache, key) => cache.RemoveAsync(key));
+
+            CheckTxOp((cache, key) => cache.RemoveAllAsync(new[] {key}));
+
+            CheckTxOp((cache, key) => cache.ReplaceAsync(key, 100));
+
+            CheckTxOp((cache, key) => cache.ReplaceAsync(key, cache[key], 100));
+        }
+
+        /// <summary>
+        /// Checks that read cache operation starts ambient transaction.
+        /// </summary>
+        private void CheckReadTxOp(Action<ICacheClient<int, int>, int> act)
+        {
+            var txOpts = new TransactionOptions {IsolationLevel = IsolationLevel.RepeatableRead};
+            const TransactionScopeOption scope = TransactionScopeOption.Required;
+
+            var cache = GetTransactionalCache();
+            cache[1] = 1;
+
+            // Rollback.
+            using (new TransactionScope(scope, txOpts))
+            {
+                act(cache, 1);
+
+                Assert.IsNotNull(((ITransactionsClientInternal) Client.GetTransactions()).CurrentTx,
+                    "Transaction has not started.");
+            }
+        }
+
+        /// <summary>
+        /// Checks that cache operation behaves transactionally.
+        /// </summary>
+        private void CheckTxOp(Action<ICacheClient<int, int>, int> act)
+        {
+            var isolationLevels = new[]
+            {
+                IsolationLevel.Serializable, IsolationLevel.RepeatableRead, IsolationLevel.ReadCommitted,
+                IsolationLevel.ReadUncommitted, IsolationLevel.Snapshot, IsolationLevel.Chaos
+            };
+
+            foreach (var isolationLevel in isolationLevels)
+            {
+                var txOpts = new TransactionOptions {IsolationLevel = isolationLevel};
+                const TransactionScopeOption scope = TransactionScopeOption.Required;
+
+                var cache = GetTransactionalCache();
+
+                cache[1] = 1;
+                cache[2] = 2;
+
+                // Rollback.
+                using (new TransactionScope(scope, txOpts))
+                {
+                    act(cache, 1);
+
+                    Assert.IsNotNull(((ITransactionsClientInternal)Client.GetTransactions()).CurrentTx,
+                        "Transaction has not started.");
+                }
+
+                Assert.AreEqual(1, cache[1]);
+                Assert.AreEqual(2, cache[2]);
+
+                using (new TransactionScope(scope, txOpts))
+                {
+                    act(cache, 1);
+                    act(cache, 2);
+                }
+
+                Assert.AreEqual(1, cache[1]);
+                Assert.AreEqual(2, cache[2]);
+
+                // Commit.
+                using (var ts = new TransactionScope(scope, txOpts))
+                {
+                    act(cache, 1);
+                    ts.Complete();
+                }
+
+                Assert.IsTrue(!cache.ContainsKey(1) || cache[1] != 1);
+                Assert.AreEqual(2, cache[2]);
+
+                using (var ts = new TransactionScope(scope, txOpts))
+                {
+                    act(cache, 1);
+                    act(cache, 2);
+                    ts.Complete();
+                }
+
+                Assert.IsTrue(!cache.ContainsKey(1) || cache[1] != 1);
+                Assert.IsTrue(!cache.ContainsKey(2) || cache[2] != 2);
+            }
+        }
+
+        /// <summary>
+        /// Gets single transaction from Ignite.
+        /// </summary>
+        private static ITransaction GetSingleLocalTransaction()
+        {
+            return GetIgnite()
+               .GetTransactions()
+               .GetLocalActiveTransactions()
+               .Single();
+        }
+
+        /// <summary>
+        /// Tests that client can't start multiple transactions in one thread.
+        /// </summary>
+        private void TestThrowsIfMultipleStarted(Func<IDisposable> outer, Func<IDisposable> inner)
+        {
+

Review comment:
       Blank line

##########
File path: modules/platforms/dotnet/Apache.Ignite.Core.Tests/Client/Cache/CacheClientAbstractTxTest.cs
##########
@@ -0,0 +1,756 @@
+/*
+ * 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.
+ */
+
+namespace Apache.Ignite.Core.Tests.Client.Cache
+{
+    using System;
+    using System.Collections.Generic;
+    using System.Linq;
+    using System.Threading;
+    using System.Transactions;
+    using Apache.Ignite.Core.Cache.Configuration;
+    using Apache.Ignite.Core.Client;
+    using Apache.Ignite.Core.Client.Cache;
+    using Apache.Ignite.Core.Client.Transactions;
+    using Apache.Ignite.Core.Impl.Client.Transactions;
+    using Apache.Ignite.Core.Transactions;
+    using NUnit.Framework;
+    using NUnit.Framework.Constraints;
+
+    /// <summary>
+    /// Transactional cache client tests.
+    /// </summary>
+    public abstract class CacheClientAbstractTxTest : ClientTestBase
+    {
+        /** All concurrency controls. */
+        private static readonly TransactionConcurrency[] AllConcurrencyControls =
+        {
+            TransactionConcurrency.Optimistic, 
+            TransactionConcurrency.Pessimistic
+        };
+
+        /** All isolation levels*/
+        private static readonly TransactionIsolation[] AllIsolationLevels = 
+        {
+            TransactionIsolation.Serializable,
+            TransactionIsolation.ReadCommitted,
+            TransactionIsolation.RepeatableRead
+        };
+
+        protected CacheClientAbstractTxTest(int serverCount, bool enablePartitionAwareness) : base(serverCount,
+            enablePartitionAwareness: enablePartitionAwareness)
+        {
+            // No-op.
+        }
+
+        /// <summary>
+        /// Tests that custom client transactions configuration is applied.
+        /// </summary>
+        [Test]
+        public void TestClientTransactionConfiguration()
+        {
+            var timeout = TransactionClientConfiguration.DefaultDefaultTimeout.Add(TimeSpan.FromMilliseconds(1000));
+            var cfg = GetClientConfiguration();
+            cfg.TransactionConfiguration = new TransactionClientConfiguration
+            {
+                DefaultTimeout = timeout
+            };
+
+            foreach (var concurrency in AllConcurrencyControls)
+            {
+                foreach (var isolation in AllIsolationLevels)
+                {
+                    cfg.TransactionConfiguration.DefaultTransactionConcurrency = concurrency;
+                    cfg.TransactionConfiguration.DefaultTransactionIsolation = isolation;
+                    using (var client = Ignition.StartClient(cfg))
+                    {
+                        ITransaction tx;
+                        using (client.GetTransactions().TxStart())
+                        {
+                            tx = GetSingleLocalTransaction();
+                            Assert.AreEqual(concurrency, tx.Concurrency);
+                            Assert.AreEqual(isolation, tx.Isolation);
+                            Assert.AreEqual(timeout, tx.Timeout);
+                        }
+                        tx.Dispose();
+                    }
+                }
+            }
+        }
+
+        /// <summary>
+        /// Tests that parameters passed to TxStart are applied.
+        /// </summary>
+        [Test]
+        public void TestTxStartPassesParameters()
+        {
+            var timeout = TransactionClientConfiguration.DefaultDefaultTimeout.Add(TimeSpan.FromMilliseconds(1000));
+            var acts = new List<Func<ITransactionsClient>>
+            {
+                () => Client.GetTransactions(),
+                () => Client.GetTransactions().WithLabel("label"),
+            };
+            foreach (var concurrency in AllConcurrencyControls)
+            {
+                foreach (var isolation in AllIsolationLevels)
+                {
+                    foreach (var act in acts)
+                    {
+                        ITransaction tx;
+                        using (act().TxStart(concurrency, isolation))
+                        {
+                            tx = GetSingleLocalTransaction();
+                            Assert.AreEqual(concurrency, tx.Concurrency);
+                            Assert.AreEqual(isolation, tx.Isolation);
+                        }
+                        tx.Dispose();
+                        using (act().TxStart(concurrency, isolation, timeout))
+                        {
+                            tx = GetSingleLocalTransaction();
+                            Assert.AreEqual(concurrency, tx.Concurrency);
+                            Assert.AreEqual(isolation, tx.Isolation);
+                            Assert.AreEqual(timeout, tx.Timeout);
+                        }
+                        tx.Dispose();
+                    }
+                }
+            }
+        }
+
+        /// <summary>
+        /// Tests that transaction can't be committed/rollback after being already completed.
+        /// </summary>
+        [Test]
+        public void TestThrowsIfEndAlreadyCompletedTransaction()
+        {
+            var tx = Client.GetTransactions().TxStart();
+            tx.Commit();
+
+            var constraint = new ReusableConstraint(Is.TypeOf<InvalidOperationException>()
+               .And.Message.Contains("Transaction")
+               .And.Message.Contains("is closed"));
+
+            Assert.Throws(constraint, () => tx.Commit());
+            Assert.Throws(constraint, () => tx.Rollback());
+
+            using (tx = Client.GetTransactions().TxStart())
+            {
+            }
+
+            Assert.Throws(constraint, () => tx.Commit());
+            Assert.Throws(constraint, () => tx.Rollback());
+        }
+

Review comment:
       Extra blank line

##########
File path: modules/platforms/dotnet/Apache.Ignite.Core/Impl/Client/Transactions/ClientCacheTransactionManager.cs
##########
@@ -0,0 +1,154 @@
+/*
+ * 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.
+ */
+
+namespace Apache.Ignite.Core.Impl.Client.Transactions
+{
+    using System;
+    using System.Diagnostics;
+    using System.Diagnostics.CodeAnalysis;
+    using System.Threading;
+    using System.Transactions;
+    using Apache.Ignite.Core.Impl.Transactions;
+
+    /// <summary>
+    /// Cache transaction enlistment manager, 
+    /// allows using Ignite transactions via standard <see cref="TransactionScope"/>.
+    /// </summary>
+    internal class ClientCacheTransactionManager : ISinglePhaseNotification, IDisposable
+    {
+        /** */
+        private readonly ITransactionsClientInternal _transactions;
+
+        /** */
+        private readonly ThreadLocal<Enlistment> _enlistment = new ThreadLocal<Enlistment>();
+
+        /// <summary>
+        /// Initializes a new instance of <see cref="ClientCacheTransactionManager"/> class.
+        /// </summary>
+        /// <param name="transactions">Transactions.</param>
+        public ClientCacheTransactionManager(ITransactionsClientInternal transactions)
+        {
+            _transactions = transactions;
+        }
+
+        /// <summary>
+        /// If ambient transaction is present, starts an Ignite transaction and enlists it.
+        /// </summary>
+        public void StartTxIfNeeded()
+        {
+            if (_transactions.CurrentTx != null)
+            {
+                // Ignite transaction is already present.
+                // We have either enlisted it already, or it has been started manually and should not be enlisted.
+                // Java enlists existing Ignite tx in this case (see CacheJtaManager.java), but we do not.
+                return;
+            }
+
+            if (_enlistment.Value != null)

Review comment:
       This condition seems to be unnecessary, please see my changes in #8004 .

##########
File path: modules/platforms/dotnet/Apache.Ignite.Core/Impl/Client/ClientFailoverSocket.cs
##########
@@ -247,6 +256,16 @@ public IEnumerable<IClientConnection> GetConnections()
         /// </summary>
         private ClientSocket GetSocket()
         {
+            var tx = _transactions.CurrentTx;
+            if (tx != null)
+            {
+                if (tx.Socket.IsDisposed)
+                {
+                    throw new IgniteClientException("Transaction context has been lost due to connection errors.");

Review comment:
       Is there a test for this case? How are users supposed to handle this?

##########
File path: modules/platforms/dotnet/Apache.Ignite.Core/Impl/Client/Transactions/ClientCacheTransactionManager.cs
##########
@@ -0,0 +1,154 @@
+/*
+ * 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.
+ */
+
+namespace Apache.Ignite.Core.Impl.Client.Transactions
+{
+    using System;
+    using System.Diagnostics;
+    using System.Diagnostics.CodeAnalysis;
+    using System.Threading;
+    using System.Transactions;
+    using Apache.Ignite.Core.Impl.Transactions;
+
+    /// <summary>
+    /// Cache transaction enlistment manager, 
+    /// allows using Ignite transactions via standard <see cref="TransactionScope"/>.
+    /// </summary>
+    internal class ClientCacheTransactionManager : ISinglePhaseNotification, IDisposable
+    {
+        /** */
+        private readonly ITransactionsClientInternal _transactions;
+
+        /** */
+        private readonly ThreadLocal<Enlistment> _enlistment = new ThreadLocal<Enlistment>();
+
+        /// <summary>
+        /// Initializes a new instance of <see cref="ClientCacheTransactionManager"/> class.
+        /// </summary>
+        /// <param name="transactions">Transactions.</param>
+        public ClientCacheTransactionManager(ITransactionsClientInternal transactions)
+        {
+            _transactions = transactions;
+        }
+
+        /// <summary>
+        /// If ambient transaction is present, starts an Ignite transaction and enlists it.
+        /// </summary>
+        public void StartTxIfNeeded()
+        {
+            if (_transactions.CurrentTx != null)
+            {
+                // Ignite transaction is already present.
+                // We have either enlisted it already, or it has been started manually and should not be enlisted.
+                // Java enlists existing Ignite tx in this case (see CacheJtaManager.java), but we do not.
+                return;
+            }
+
+            if (_enlistment.Value != null)
+            {
+                // We are already enlisted.
+                // .NET transaction mechanism allows nested transactions,
+                // and they can be processed differently depending on TransactionScopeOption.
+                // Ignite, however, allows only one active transaction per thread.
+                // Therefore we enlist only once on the first transaction that we encounter.
+                return;
+            }
+
+            var ambientTx = System.Transactions.Transaction.Current;
+
+            if (ambientTx != null && ambientTx.TransactionInformation.Status == TransactionStatus.Active)
+            {
+                _transactions.TxStart(_transactions.DefaultTxConcurrency,
+                    CacheTransactionManager.ConvertTransactionIsolation(ambientTx.IsolationLevel),
+                    _transactions.DefaultTimeout);
+
+                _enlistment.Value = ambientTx.EnlistVolatile(this, EnlistmentOptions.None);
+            }
+        }
+
+        void IEnlistmentNotification.Prepare(PreparingEnlistment preparingEnlistment)

Review comment:
       Missing xmldoc

##########
File path: modules/platforms/dotnet/Apache.Ignite.Core/Impl/Client/Transactions/TransactionsClient.cs
##########
@@ -0,0 +1,249 @@
+/*
+ * 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.
+ */
+
+namespace Apache.Ignite.Core.Impl.Client.Transactions
+{
+    using System;
+    using System.Diagnostics;
+    using System.Diagnostics.CodeAnalysis;
+    using System.Threading;
+    using Apache.Ignite.Core.Client;
+    using Apache.Ignite.Core.Client.Transactions;
+    using Apache.Ignite.Core.Impl.Binary;
+    using Apache.Ignite.Core.Impl.Common;
+    using Apache.Ignite.Core.Transactions;
+
+    /// <summary>
+    /// Ignite Thin Client transactions facade.
+    /// </summary>
+    internal class TransactionsClient : ITransactionsClientInternal, IDisposable
+    {
+        /** Ignite. */
+        private readonly IgniteClient _ignite;
+
+        /** Default transaction configuration. */
+        private readonly TransactionClientConfiguration _cfg;
+
+        /** Transaction for this thread and client. */
+        private readonly ThreadLocal<TransactionClient> _currentTx = new ThreadLocal<TransactionClient>();
+
+        /** Transaction manager. */
+        private readonly ClientCacheTransactionManager _txManager;
+
+        /// <summary>
+        /// Constructor.
+        /// </summary>
+        /// <param name="ignite">Ignite.</param>
+        /// <param name="cfg"></param>
+        public TransactionsClient(IgniteClient ignite, TransactionClientConfiguration cfg)
+        {
+            _ignite = ignite;
+            _cfg = cfg ?? new TransactionClientConfiguration();
+            _txManager = new ClientCacheTransactionManager(this);
+        }
+
+        /** <inheritdoc /> */
+        public ITransactionClientInternal CurrentTx
+        {
+            get
+            {
+                var tx = _currentTx.Value;
+
+                if (tx == null)
+                    return null;
+
+                if (tx.Closed)
+                {
+                    _currentTx.Value = null;
+
+                    return null;
+                }
+
+                return tx;
+            }
+        }
+
+        /** <inheritDoc /> */
+        public void StartTxIfNeeded()
+        {
+            _txManager.StartTxIfNeeded();
+        }
+
+        /** <inheritDoc /> */
+        public TransactionConcurrency DefaultTxConcurrency
+        {
+            get { return _cfg.DefaultTransactionConcurrency; }
+        }
+
+        /** <inheritDoc /> */
+        public TransactionIsolation DefaultTxIsolation
+        {
+            get { return _cfg.DefaultTransactionIsolation; }
+        }
+
+        /** <inheritDoc /> */
+        public TimeSpan DefaultTimeout
+        {
+            get { return _cfg.DefaultTimeout; }
+        }
+
+        /** <inheritDoc /> */
+        public ITransactionClient TxStart()
+        {
+            return TxStart(_cfg.DefaultTransactionConcurrency, _cfg.DefaultTransactionIsolation);
+        }
+
+        /** <inheritDoc /> */
+        public ITransactionClient TxStart(TransactionConcurrency concurrency, TransactionIsolation isolation)
+        {
+            return TxStart(concurrency, isolation, _cfg.DefaultTimeout);
+        }
+
+        /** <inheritDoc /> */
+        public ITransactionClient TxStart(TransactionConcurrency concurrency, 
+            TransactionIsolation isolation,
+            TimeSpan timeout)
+        {
+            return TxStart(concurrency, isolation, timeout, null);
+        }
+
+        private ITransactionClient TxStart(TransactionConcurrency concurrency,
+            TransactionIsolation isolation,
+            TimeSpan timeout,
+            string label)
+        {
+            if (CurrentTx != null)
+            {
+                throw new IgniteClientException("A transaction has already been started by the current thread.");
+            }
+
+            var tx = _ignite.Socket.DoOutInOp(
+                ClientOp.TxStart,
+                ctx =>
+                {
+                    ctx.Writer.WriteByte((byte) concurrency);
+                    ctx.Writer.WriteByte((byte) isolation);
+                    ctx.Writer.WriteTimeSpanAsLong(timeout);
+                    ctx.Writer.WriteString(label);
+                },
+                ctx => new TransactionClient(
+                    ctx.Reader.ReadInt(),
+                    _ignite,
+                    ctx.Socket)
+            );
+
+            _currentTx.Value = tx;
+            return _currentTx.Value;

Review comment:
       `return tx`

##########
File path: modules/platforms/dotnet/Apache.Ignite.Core/Impl/Client/Transactions/TransactionsClient.cs
##########
@@ -0,0 +1,249 @@
+/*
+ * 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.
+ */
+
+namespace Apache.Ignite.Core.Impl.Client.Transactions
+{
+    using System;
+    using System.Diagnostics;
+    using System.Diagnostics.CodeAnalysis;
+    using System.Threading;
+    using Apache.Ignite.Core.Client;
+    using Apache.Ignite.Core.Client.Transactions;
+    using Apache.Ignite.Core.Impl.Binary;
+    using Apache.Ignite.Core.Impl.Common;
+    using Apache.Ignite.Core.Transactions;
+
+    /// <summary>
+    /// Ignite Thin Client transactions facade.
+    /// </summary>
+    internal class TransactionsClient : ITransactionsClientInternal, IDisposable
+    {
+        /** Ignite. */
+        private readonly IgniteClient _ignite;
+
+        /** Default transaction configuration. */
+        private readonly TransactionClientConfiguration _cfg;
+
+        /** Transaction for this thread and client. */
+        private readonly ThreadLocal<TransactionClient> _currentTx = new ThreadLocal<TransactionClient>();
+
+        /** Transaction manager. */
+        private readonly ClientCacheTransactionManager _txManager;
+
+        /// <summary>
+        /// Constructor.
+        /// </summary>
+        /// <param name="ignite">Ignite.</param>
+        /// <param name="cfg"></param>
+        public TransactionsClient(IgniteClient ignite, TransactionClientConfiguration cfg)
+        {
+            _ignite = ignite;
+            _cfg = cfg ?? new TransactionClientConfiguration();
+            _txManager = new ClientCacheTransactionManager(this);
+        }
+
+        /** <inheritdoc /> */
+        public ITransactionClientInternal CurrentTx
+        {
+            get
+            {
+                var tx = _currentTx.Value;
+
+                if (tx == null)
+                    return null;
+
+                if (tx.Closed)
+                {
+                    _currentTx.Value = null;
+
+                    return null;
+                }
+
+                return tx;
+            }
+        }
+
+        /** <inheritDoc /> */
+        public void StartTxIfNeeded()
+        {
+            _txManager.StartTxIfNeeded();
+        }
+
+        /** <inheritDoc /> */
+        public TransactionConcurrency DefaultTxConcurrency
+        {
+            get { return _cfg.DefaultTransactionConcurrency; }
+        }
+
+        /** <inheritDoc /> */
+        public TransactionIsolation DefaultTxIsolation
+        {
+            get { return _cfg.DefaultTransactionIsolation; }
+        }
+
+        /** <inheritDoc /> */
+        public TimeSpan DefaultTimeout
+        {
+            get { return _cfg.DefaultTimeout; }
+        }
+
+        /** <inheritDoc /> */
+        public ITransactionClient TxStart()
+        {
+            return TxStart(_cfg.DefaultTransactionConcurrency, _cfg.DefaultTransactionIsolation);
+        }
+
+        /** <inheritDoc /> */
+        public ITransactionClient TxStart(TransactionConcurrency concurrency, TransactionIsolation isolation)
+        {
+            return TxStart(concurrency, isolation, _cfg.DefaultTimeout);
+        }
+
+        /** <inheritDoc /> */
+        public ITransactionClient TxStart(TransactionConcurrency concurrency, 
+            TransactionIsolation isolation,
+            TimeSpan timeout)
+        {
+            return TxStart(concurrency, isolation, timeout, null);
+        }
+
+        private ITransactionClient TxStart(TransactionConcurrency concurrency,
+            TransactionIsolation isolation,
+            TimeSpan timeout,
+            string label)
+        {
+            if (CurrentTx != null)
+            {
+                throw new IgniteClientException("A transaction has already been started by the current thread.");
+            }
+
+            var tx = _ignite.Socket.DoOutInOp(
+                ClientOp.TxStart,
+                ctx =>
+                {
+                    ctx.Writer.WriteByte((byte) concurrency);
+                    ctx.Writer.WriteByte((byte) isolation);
+                    ctx.Writer.WriteTimeSpanAsLong(timeout);
+                    ctx.Writer.WriteString(label);
+                },
+                ctx => new TransactionClient(
+                    ctx.Reader.ReadInt(),
+                    _ignite,
+                    ctx.Socket)
+            );
+
+            _currentTx.Value = tx;
+            return _currentTx.Value;
+        }
+
+        /** <inheritDoc /> */
+        public ITransactionsClient WithLabel(string label)
+        {
+            IgniteArgumentCheck.NotNullOrEmpty(label, "label");
+
+            return new TransactionsClientWithLabel(this, label); 
+        }
+
+        /** <inheritDoc /> */
+        [SuppressMessage("Microsoft.Usage",
+            "CA1816:CallGCSuppressFinalizeCorrectly",
+            Justification = "There is no finalizer.")]
+        public void Dispose()
+        {
+            _currentTx.Dispose();
+            _txManager.Dispose();
+        }
+
+        /// <summary>
+        /// Wrapper for transactions with label.
+        /// </summary>
+        private class TransactionsClientWithLabel : ITransactionsClientInternal

Review comment:
       I think it will be simpler to incorporate label into the `TransactionsClient` class.

##########
File path: modules/platforms/dotnet/Apache.Ignite.Core/Impl/Client/Transactions/ITransactionClientInternal.cs
##########
@@ -0,0 +1,34 @@
+/*
+ * 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.
+ */
+
+namespace Apache.Ignite.Core.Impl.Client.Transactions
+{
+    using Apache.Ignite.Core.Client.Transactions;
+
+    internal interface ITransactionClientInternal : ITransactionClient

Review comment:
       xmldoc




----------------------------------------------------------------
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.

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



[GitHub] [ignite] gurustron commented on a change in pull request #7992: IGNITE-7369 .NET: Thin Client Transactions

Posted by GitBox <gi...@apache.org>.
gurustron commented on a change in pull request #7992:
URL: https://github.com/apache/ignite/pull/7992#discussion_r458400450



##########
File path: modules/platforms/dotnet/Apache.Ignite.Core/Client/Transactions/TransactionClientConfiguration.cs
##########
@@ -0,0 +1,81 @@
+/*
+ * 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.
+ */
+
+namespace Apache.Ignite.Core.Client.Transactions
+{
+    using System;
+    using System.ComponentModel;
+    using Apache.Ignite.Core.Impl.Common;
+    using Apache.Ignite.Core.Transactions;
+
+    /// <summary>
+    /// Transactions configuration.
+    /// </summary>
+    public class TransactionClientConfiguration
+    {
+        /// <summary> The default value for <see cref="DefaultTransactionConcurrency"/> property. </summary>
+        public const TransactionConcurrency DefaultDefaultTransactionConcurrency = TransactionConcurrency.Pessimistic;
+
+        /// <summary> The default value for <see cref="DefaultTransactionIsolation"/> property. </summary>
+        public const TransactionIsolation DefaultDefaultTransactionIsolation = TransactionIsolation.RepeatableRead;
+
+        /// <summary> The default value for <see cref="DefaultTransactionIsolation"/> property. </summary>
+        public static readonly TimeSpan DefaultDefaultTimeout = TimeSpan.Zero;
+
+        /// <summary>
+        /// Gets or sets the cache transaction concurrency to use when one is not explicitly specified.
+        /// </summary>
+        [DefaultValue(DefaultDefaultTransactionConcurrency)]
+        public TransactionConcurrency DefaultTransactionConcurrency { get; set; }
+
+        /// <summary>
+        /// Gets or sets the cache transaction isolation to use when one is not explicitly specified.
+        /// </summary>
+        [DefaultValue(DefaultDefaultTransactionIsolation)]
+        public TransactionIsolation DefaultTransactionIsolation { get; set; }
+
+        /// <summary>
+        /// Gets or sets the cache transaction timeout to use when one is not explicitly specified.
+        /// <see cref="TimeSpan.Zero"/> for infinite timeout.
+        /// </summary>
+        [DefaultValue(typeof(TimeSpan), "00:00:00")]
+        public TimeSpan DefaultTimeout { get; set; }
+
+        /// <summary>
+        /// Initializes a new instance of the <see cref="TransactionClientConfiguration" /> class.
+        /// </summary>
+        public TransactionClientConfiguration()

Review comment:
       1.  There are already tests for it - IgniteConfigurationTest.TestDefaultValueAttributes:103 for default ctor And IgniteClientConfigurationTest.TestTransactionConfigurationCopyCtor for copy one.
   2.  CacheClientAbstractTxTest.TestClientTransactionConfiguration and TestTxStartPassesParameters would not be sufficient?




----------------------------------------------------------------
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.

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



[GitHub] [ignite] gurustron commented on a change in pull request #7992: IGNITE-7369 Thin Client Transactions [DRAFT]

Posted by GitBox <gi...@apache.org>.
gurustron commented on a change in pull request #7992:
URL: https://github.com/apache/ignite/pull/7992#discussion_r455796028



##########
File path: modules/platforms/dotnet/Apache.Ignite.Core/Impl/Client/Transactions/ClientTransaction.cs
##########
@@ -0,0 +1,125 @@
+/*
+ * 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.
+ */
+
+namespace Apache.Ignite.Core.Impl.Client.Transactions
+{
+    using System;
+    using System.Threading;
+
+    /// <summary>
+    /// Ignite Thin Client transaction facade.
+    /// </summary>
+    internal class ClientTransaction: IClientTransactionInternal
+    {
+        /** Unique  transaction ID.*/
+        private readonly int _id;
+
+        /** Ignite. */
+        private readonly IgniteClient _ignite;
+
+        /* Transactions. */
+        private readonly ClientTransactions _transactions;
+
+        /** Transaction is closed. */
+        private volatile bool _closed; 

Review comment:
       In theory transaction returned by `TxStart` can be closed from any thread.




----------------------------------------------------------------
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.

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



[GitHub] [ignite] ptupitsyn commented on a change in pull request #7992: IGNITE-7369 .NET: Thin Client Transactions

Posted by GitBox <gi...@apache.org>.
ptupitsyn commented on a change in pull request #7992:
URL: https://github.com/apache/ignite/pull/7992#discussion_r464087502



##########
File path: modules/platforms/dotnet/Apache.Ignite.Core/Impl/Client/Transactions/TransactionClient.cs
##########
@@ -0,0 +1,172 @@
+/*
+ * 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.
+ */
+
+namespace Apache.Ignite.Core.Impl.Client.Transactions
+{
+    using System;
+    using System.Globalization;
+    using Apache.Ignite.Core.Client.Transactions;
+    using Apache.Ignite.Core.Transactions;
+
+    /// <summary>
+    /// Ignite Thin Client transaction facade.
+    /// </summary>
+    internal class TransactionClient : ITransactionClient
+    {
+        /** Unique  transaction ID.*/
+        private readonly int _id;
+
+        /** Ignite. */
+        private readonly IgniteClient _ignite;
+
+        /** Socket. */
+        private readonly ClientSocket _socket;
+
+        /** Transaction is closed. */
+        private bool _closed;
+
+        /// <summary>
+        /// Constructor.
+        /// </summary>
+        /// <param name="id">ID.</param>
+        /// <param name="ignite">Ignite.</param>
+        /// <param name="socket">Socket.</param>
+        /// <param name="concurrency">Concurrency.</param>
+        /// <param name="isolation">Isolation.</param>
+        /// <param name="timeout">Timeout.</param>
+        /// <param name="label">Label.</param>
+        public TransactionClient(int id,
+            IgniteClient ignite,
+            ClientSocket socket,
+            TransactionConcurrency concurrency,
+            TransactionIsolation isolation,
+            TimeSpan timeout,
+            string label)
+        {
+            _id = id;
+            _ignite = ignite;
+            _socket = socket;
+            Concurrency = concurrency;
+            Isolation = isolation;
+            Timeout = timeout;
+            Label = label;
+        }
+
+        /** <inheritdoc /> */
+        public void Commit()
+        {
+            ThrowIfClosed();
+            Close(true);
+        }
+
+        /** <inheritdoc /> */
+        public void Rollback()
+        {
+            ThrowIfClosed();
+            Close(false);
+        }
+
+        /** <inheritdoc /> */
+        public TransactionConcurrency Concurrency { get; private set; }
+
+        /** <inheritdoc /> */
+        public TransactionIsolation Isolation { get; private set; }
+
+        /** <inheritdoc /> */
+        public TimeSpan Timeout { get; private set; }
+
+        /** <inheritdoc /> */
+        public string Label { get; private set; }
+
+        /** <inheritdoc /> */
+        public void Dispose()
+        {
+            try
+            {
+                Close(false);
+            }
+            finally
+            {
+                GC.SuppressFinalize(this);
+            }
+        }
+
+        /// <summary>
+        /// Transaction Id.
+        /// </summary>
+        public int Id
+        {
+            get { return _id; }
+        }
+
+        public ClientSocket Socket
+        {
+            get { return _socket; }
+        }
+
+        /// <summary>
+        /// Returns if transaction is closed.
+        /// </summary>
+        internal bool Closed
+        {
+            get { return _closed; }
+        }
+
+        /// <summary>
+        /// Closes the transaction. 
+        /// </summary>
+        private void Close(bool commit)
+        {
+            if (!_closed)
+            {
+                try
+                {
+                    _ignite.Socket.DoOutInOp<object>(ClientOp.TxEnd,

Review comment:
       I think the only situation to be covered here is a socket disconnect.
   We can catch `Exception`, and swallow it only if `_socket` is disconnected, otherwise `throw`.




----------------------------------------------------------------
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.

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



[GitHub] [ignite] gurustron commented on a change in pull request #7992: IGNITE-7369 .NET: Thin Client Transactions

Posted by GitBox <gi...@apache.org>.
gurustron commented on a change in pull request #7992:
URL: https://github.com/apache/ignite/pull/7992#discussion_r464068368



##########
File path: modules/platforms/dotnet/Apache.Ignite.Core/Client/Transactions/ITransactionClient.cs
##########
@@ -0,0 +1,46 @@
+/*
+ * 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.
+ */
+
+namespace Apache.Ignite.Core.Client.Transactions
+{
+    using System;
+    using Apache.Ignite.Core.Transactions;
+
+    /// <summary>
+    /// Thin client transaction.
+    /// </summary>
+    public interface ITransactionClient : IDisposable
+    {
+        /// <summary>
+        /// Commits this transaction.
+        /// </summary>
+        void Commit();
+
+        /// <summary>
+        /// Rolls back this transaction.
+        /// </summary>
+        void Rollback();
+        
+        TransactionConcurrency Concurrency { get; } 

Review comment:
       Fixed.




----------------------------------------------------------------
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.

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



[GitHub] [ignite] ptupitsyn commented on a change in pull request #7992: IGNITE-7369 .NET: Thin Client Transactions

Posted by GitBox <gi...@apache.org>.
ptupitsyn commented on a change in pull request #7992:
URL: https://github.com/apache/ignite/pull/7992#discussion_r458278482



##########
File path: modules/platforms/dotnet/Apache.Ignite.Core/Client/Transactions/TransactionClientConfiguration.cs
##########
@@ -0,0 +1,81 @@
+/*
+ * 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.
+ */
+
+namespace Apache.Ignite.Core.Client.Transactions
+{
+    using System;
+    using System.ComponentModel;
+    using Apache.Ignite.Core.Impl.Common;
+    using Apache.Ignite.Core.Transactions;
+
+    /// <summary>
+    /// Transactions configuration.
+    /// </summary>
+    public class TransactionClientConfiguration
+    {
+        /// <summary> The default value for <see cref="DefaultTransactionConcurrency"/> property. </summary>
+        public const TransactionConcurrency DefaultDefaultTransactionConcurrency = TransactionConcurrency.Pessimistic;
+
+        /// <summary> The default value for <see cref="DefaultTransactionIsolation"/> property. </summary>
+        public const TransactionIsolation DefaultDefaultTransactionIsolation = TransactionIsolation.RepeatableRead;
+
+        /// <summary> The default value for <see cref="DefaultTransactionIsolation"/> property. </summary>
+        public static readonly TimeSpan DefaultDefaultTimeout = TimeSpan.Zero;
+
+        /// <summary>
+        /// Gets or sets the cache transaction concurrency to use when one is not explicitly specified.
+        /// </summary>
+        [DefaultValue(DefaultDefaultTransactionConcurrency)]
+        public TransactionConcurrency DefaultTransactionConcurrency { get; set; }
+
+        /// <summary>
+        /// Gets or sets the cache transaction isolation to use when one is not explicitly specified.
+        /// </summary>
+        [DefaultValue(DefaultDefaultTransactionIsolation)]
+        public TransactionIsolation DefaultTransactionIsolation { get; set; }
+
+        /// <summary>
+        /// Gets or sets the cache transaction timeout to use when one is not explicitly specified.
+        /// <see cref="TimeSpan.Zero"/> for infinite timeout.
+        /// </summary>
+        [DefaultValue(typeof(TimeSpan), "00:00:00")]
+        public TimeSpan DefaultTimeout { get; set; }
+
+        /// <summary>
+        /// Initializes a new instance of the <see cref="TransactionClientConfiguration" /> class.
+        /// </summary>
+        public TransactionClientConfiguration()
+        {
+            DefaultTransactionConcurrency = DefaultDefaultTransactionConcurrency;
+            DefaultTransactionIsolation = DefaultDefaultTransactionIsolation;
+            DefaultTimeout = DefaultDefaultTimeout;
+        }
+        
+        /// <summary>
+        /// Initializes a new instance of the <see cref="TransactionClientConfiguration" /> class.
+        /// </summary>
+        /// <param name="cfg">The configuration to copy.</param>
+        public TransactionClientConfiguration(TransactionClientConfiguration cfg)
+        {
+            IgniteArgumentCheck.NotNull(cfg, "configuration");
+
+            DefaultTransactionConcurrency = cfg.DefaultTransactionConcurrency;
+            DefaultTransactionIsolation = cfg.DefaultTransactionIsolation;
+            DefaultTimeout =cfg.DefaultTimeout;

Review comment:
       Missing space after `=`

##########
File path: modules/platforms/dotnet/Apache.Ignite.Core/Impl/Client/Transactions/ITransactionClientInternal.cs
##########
@@ -0,0 +1,37 @@
+/*
+ * 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.
+ */
+
+namespace Apache.Ignite.Core.Impl.Client.Transactions
+{
+    using Apache.Ignite.Core.Client.Transactions;
+
+    /// <summary>
+    /// Internal Ignite Thin Client transaction facade.
+    /// </summary>
+    internal interface ITransactionClientInternal : ITransactionClient

Review comment:
       Looks like we don't really need this interface and the one below, since they have only one implementation.

##########
File path: modules/platforms/dotnet/Apache.Ignite.Core/Client/Transactions/ITransactionsClient.cs
##########
@@ -0,0 +1,70 @@
+/*
+ * 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.
+ */
+
+namespace Apache.Ignite.Core.Client.Transactions
+{
+    using System;
+    using Apache.Ignite.Core.Transactions;
+
+    /// <summary>
+    /// Ignite Thin Client transactions facade.
+    /// <para /> Transactions are bound to the thread started the transaction. After that, each cache operation within this thread
+    /// will belong to the corresponding transaction until the transaction is committed, rolled back or closed.
+    /// <para /> Should not be used with async calls.
+    /// </summary> 
+    public interface ITransactionsClient

Review comment:
       Please add properties:
   * `Tx`
   * `DefaultTransactionConcurrency`
   * `DefaultTransactionIsolation`
   * `DefaultTimeout`

##########
File path: modules/platforms/dotnet/Apache.Ignite.Core/Client/Transactions/ITransactionClient.cs
##########
@@ -0,0 +1,37 @@
+/*
+ * 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.
+ */
+
+namespace Apache.Ignite.Core.Client.Transactions
+{
+    using System;
+
+    /// <summary>
+    /// Thin client transaction.
+    /// </summary>
+    public interface ITransactionClient : IDisposable

Review comment:
       Can we add tx properties here?
   * `Isolation`
   * `Concurrency`
   * `Timeout`
   * `Label`

##########
File path: modules/platforms/dotnet/Apache.Ignite.Core/Client/Transactions/TransactionClientConfiguration.cs
##########
@@ -0,0 +1,81 @@
+/*
+ * 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.
+ */
+
+namespace Apache.Ignite.Core.Client.Transactions
+{
+    using System;
+    using System.ComponentModel;
+    using Apache.Ignite.Core.Impl.Common;
+    using Apache.Ignite.Core.Transactions;
+
+    /// <summary>
+    /// Transactions configuration.
+    /// </summary>
+    public class TransactionClientConfiguration
+    {
+        /// <summary> The default value for <see cref="DefaultTransactionConcurrency"/> property. </summary>
+        public const TransactionConcurrency DefaultDefaultTransactionConcurrency = TransactionConcurrency.Pessimistic;
+
+        /// <summary> The default value for <see cref="DefaultTransactionIsolation"/> property. </summary>
+        public const TransactionIsolation DefaultDefaultTransactionIsolation = TransactionIsolation.RepeatableRead;
+
+        /// <summary> The default value for <see cref="DefaultTransactionIsolation"/> property. </summary>
+        public static readonly TimeSpan DefaultDefaultTimeout = TimeSpan.Zero;
+
+        /// <summary>
+        /// Gets or sets the cache transaction concurrency to use when one is not explicitly specified.
+        /// </summary>
+        [DefaultValue(DefaultDefaultTransactionConcurrency)]
+        public TransactionConcurrency DefaultTransactionConcurrency { get; set; }
+
+        /// <summary>
+        /// Gets or sets the cache transaction isolation to use when one is not explicitly specified.
+        /// </summary>
+        [DefaultValue(DefaultDefaultTransactionIsolation)]
+        public TransactionIsolation DefaultTransactionIsolation { get; set; }
+
+        /// <summary>
+        /// Gets or sets the cache transaction timeout to use when one is not explicitly specified.
+        /// <see cref="TimeSpan.Zero"/> for infinite timeout.
+        /// </summary>
+        [DefaultValue(typeof(TimeSpan), "00:00:00")]
+        public TimeSpan DefaultTimeout { get; set; }
+
+        /// <summary>
+        /// Initializes a new instance of the <see cref="TransactionClientConfiguration" /> class.
+        /// </summary>
+        public TransactionClientConfiguration()

Review comment:
       * Please add simple tests for both constructors (check that properties are set as expected)
   * Add tests for custom values - use newly added properties in `ITransactionClient` and `ITransactionsClient` to make sure that custom defaults are used

##########
File path: modules/platforms/dotnet/Apache.Ignite.Core/Impl/Client/Transactions/TransactionsClient.cs
##########
@@ -0,0 +1,249 @@
+/*
+ * 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.
+ */
+
+namespace Apache.Ignite.Core.Impl.Client.Transactions
+{
+    using System;
+    using System.Diagnostics;
+    using System.Diagnostics.CodeAnalysis;
+    using System.Threading;
+    using Apache.Ignite.Core.Client;
+    using Apache.Ignite.Core.Client.Transactions;
+    using Apache.Ignite.Core.Impl.Binary;
+    using Apache.Ignite.Core.Impl.Common;
+    using Apache.Ignite.Core.Transactions;
+
+    /// <summary>
+    /// Ignite Thin Client transactions facade.
+    /// </summary>
+    internal class TransactionsClient : ITransactionsClientInternal, IDisposable
+    {
+        /** Ignite. */
+        private readonly IgniteClient _ignite;
+
+        /** Default transaction configuration. */
+        private readonly TransactionClientConfiguration _cfg;
+
+        /** Transaction for this thread and client. */
+        private readonly ThreadLocal<TransactionClient> _currentTx = new ThreadLocal<TransactionClient>();
+
+        /** Transaction manager. */
+        private readonly ClientCacheTransactionManager _txManager;
+
+        /// <summary>
+        /// Constructor.
+        /// </summary>
+        /// <param name="ignite">Ignite.</param>
+        /// <param name="cfg"></param>
+        public TransactionsClient(IgniteClient ignite, TransactionClientConfiguration cfg)
+        {
+            _ignite = ignite;
+            _cfg = cfg ?? new TransactionClientConfiguration();
+            _txManager = new ClientCacheTransactionManager(this);
+        }
+
+        /** <inheritdoc /> */
+        public ITransactionClientInternal CurrentTx
+        {
+            get
+            {
+                var tx = _currentTx.Value;
+
+                if (tx == null)
+                    return null;
+
+                if (tx.Closed)
+                {
+                    _currentTx.Value = null;
+
+                    return null;
+                }
+
+                return tx;
+            }
+        }
+
+        /** <inheritDoc /> */
+        public void StartTxIfNeeded()
+        {
+            _txManager.StartTxIfNeeded();
+        }
+
+        /** <inheritDoc /> */
+        public TransactionConcurrency DefaultTxConcurrency
+        {
+            get { return _cfg.DefaultTransactionConcurrency; }
+        }
+
+        /** <inheritDoc /> */
+        public TransactionIsolation DefaultTxIsolation
+        {
+            get { return _cfg.DefaultTransactionIsolation; }
+        }
+
+        /** <inheritDoc /> */
+        public TimeSpan DefaultTimeout
+        {
+            get { return _cfg.DefaultTimeout; }
+        }
+
+        /** <inheritDoc /> */
+        public ITransactionClient TxStart()
+        {
+            return TxStart(_cfg.DefaultTransactionConcurrency, _cfg.DefaultTransactionIsolation);
+        }
+
+        /** <inheritDoc /> */
+        public ITransactionClient TxStart(TransactionConcurrency concurrency, TransactionIsolation isolation)
+        {
+            return TxStart(concurrency, isolation, _cfg.DefaultTimeout);
+        }
+
+        /** <inheritDoc /> */
+        public ITransactionClient TxStart(TransactionConcurrency concurrency, 
+            TransactionIsolation isolation,
+            TimeSpan timeout)
+        {
+            return TxStart(concurrency, isolation, timeout, null);
+        }
+
+        private ITransactionClient TxStart(TransactionConcurrency concurrency,

Review comment:
       XMLDoc

##########
File path: modules/platforms/dotnet/Apache.Ignite.Core.Tests/Client/Cache/CachePartitionedTxTest.cs
##########
@@ -0,0 +1,101 @@
+/*
+ * 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.
+ */
+
+namespace Apache.Ignite.Core.Tests.Client.Cache
+{
+    using System.Linq;
+    using System.Transactions;
+    using Apache.Ignite.Core.Client;
+    using Apache.Ignite.Core.Impl.Client.Transactions;
+    using NUnit.Framework;
+    using NUnit.Framework.Constraints;
+
+    /// <summary>
+    /// Tests client transactions for multiple nodes with partition awareness.
+    /// </summary>
+    public class CachePartitionedTxTest : CacheClientAbstractTxTest
+    {
+        /// <summary>
+        ///  Initializes a new instance of the <see cref="CachePartitionedTxTest"/> class.
+        /// </summary>
+        public CachePartitionedTxTest() : base(3, true)
+        {
+            // No-op.
+        }
+
+        /// <summary>
+        /// Test transaction for partition aware client. 
+        /// </summary>
+        [Test]
+        public void TestTxPartitioned()
+        {
+            var cache = GetTransactionalCache();
+            var ignite1 = GetIgnite();
+            var ignite2 = GetIgnite(1);
+            var key1 = TestUtils.GetPrimaryKey(ignite1, GetCacheName());
+            var key2 = TestUtils.GetPrimaryKey(ignite2, GetCacheName());
+
+            cache.Put(key1, 1);
+            cache.Put(key2, 2);
+
+            using (Client.GetTransactions().TxStart())
+            {
+                cache.Put(key1, 10);
+                cache.Put(key2, 20);
+
+                Assert.AreEqual(10, cache.Get(key1));
+                Assert.AreEqual(20, cache.Get(key2));
+            }
+
+            Assert.AreEqual(1, cache.Get(key1));
+            Assert.AreEqual(2, cache.Get(key2));
+        }
+        
+        /// <summary>
+        /// Test transaction scope for partition aware client. 
+        /// </summary>
+        [Test]
+        public void TestTransactionScopePartitioned()
+        {
+            var cache = GetTransactionalCache();
+            var ignite1 = GetIgnite();
+            var ignite2 = GetIgnite(1);
+            var key1 = TestUtils.GetPrimaryKey(ignite1, GetCacheName());
+            var key2 = TestUtils.GetPrimaryKey(ignite2, GetCacheName());
+
+            cache.Put(key1, 1);
+            cache.Put(key2, 2);
+
+            using (new TransactionScope())
+            {
+                cache.Put(key1, 10);
+                cache.Put(key2, 20);
+
+                Assert.AreEqual(10, cache.Get(key1));
+                Assert.AreEqual(20, cache.Get(key2));
+            }
+
+            Assert.AreEqual(1, cache.Get(key1));
+            Assert.AreEqual(2, cache.Get(key2));
+        }
+
+        protected override string GetCacheName()

Review comment:
       inheritdoc




----------------------------------------------------------------
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.

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



[GitHub] [ignite] gurustron commented on a change in pull request #7992: IGNITE-7369 .NET: Thin Client Transactions

Posted by GitBox <gi...@apache.org>.
gurustron commented on a change in pull request #7992:
URL: https://github.com/apache/ignite/pull/7992#discussion_r458397631



##########
File path: modules/platforms/dotnet/Apache.Ignite.Core/Impl/Client/ClientFailoverSocket.cs
##########
@@ -247,6 +256,16 @@ public IEnumerable<IClientConnection> GetConnections()
         /// </summary>
         private ClientSocket GetSocket()
         {
+            var tx = _transactions.CurrentTx;
+            if (tx != null)
+            {
+                if (tx.Socket.IsDisposed)
+                {
+                    throw new IgniteClientException("Transaction context has been lost due to connection errors.");

Review comment:
       I've added one. Calling dispose o tx should take care of it.




----------------------------------------------------------------
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.

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



[GitHub] [ignite] gurustron commented on a change in pull request #7992: IGNITE-7369 .NET: Thin Client Transactions

Posted by GitBox <gi...@apache.org>.
gurustron commented on a change in pull request #7992:
URL: https://github.com/apache/ignite/pull/7992#discussion_r464069966



##########
File path: modules/platforms/dotnet/Apache.Ignite.Core/Impl/Client/Transactions/TransactionClient.cs
##########
@@ -0,0 +1,172 @@
+/*
+ * 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.
+ */
+
+namespace Apache.Ignite.Core.Impl.Client.Transactions
+{
+    using System;
+    using System.Globalization;
+    using Apache.Ignite.Core.Client.Transactions;
+    using Apache.Ignite.Core.Transactions;
+
+    /// <summary>
+    /// Ignite Thin Client transaction facade.
+    /// </summary>
+    internal class TransactionClient : ITransactionClient
+    {
+        /** Unique  transaction ID.*/
+        private readonly int _id;
+
+        /** Ignite. */
+        private readonly IgniteClient _ignite;
+
+        /** Socket. */
+        private readonly ClientSocket _socket;
+
+        /** Transaction is closed. */
+        private bool _closed;
+
+        /// <summary>
+        /// Constructor.
+        /// </summary>
+        /// <param name="id">ID.</param>
+        /// <param name="ignite">Ignite.</param>
+        /// <param name="socket">Socket.</param>
+        /// <param name="concurrency">Concurrency.</param>
+        /// <param name="isolation">Isolation.</param>
+        /// <param name="timeout">Timeout.</param>
+        /// <param name="label">Label.</param>
+        public TransactionClient(int id,
+            IgniteClient ignite,
+            ClientSocket socket,
+            TransactionConcurrency concurrency,
+            TransactionIsolation isolation,
+            TimeSpan timeout,
+            string label)
+        {
+            _id = id;
+            _ignite = ignite;
+            _socket = socket;
+            Concurrency = concurrency;
+            Isolation = isolation;
+            Timeout = timeout;
+            Label = label;
+        }
+
+        /** <inheritdoc /> */
+        public void Commit()
+        {
+            ThrowIfClosed();
+            Close(true);
+        }
+
+        /** <inheritdoc /> */
+        public void Rollback()
+        {
+            ThrowIfClosed();
+            Close(false);
+        }
+
+        /** <inheritdoc /> */
+        public TransactionConcurrency Concurrency { get; private set; }
+
+        /** <inheritdoc /> */
+        public TransactionIsolation Isolation { get; private set; }
+
+        /** <inheritdoc /> */
+        public TimeSpan Timeout { get; private set; }
+
+        /** <inheritdoc /> */
+        public string Label { get; private set; }
+
+        /** <inheritdoc /> */
+        public void Dispose()
+        {
+            try
+            {
+                Close(false);
+            }
+            finally
+            {
+                GC.SuppressFinalize(this);
+            }
+        }
+
+        /// <summary>
+        /// Transaction Id.
+        /// </summary>
+        public int Id
+        {
+            get { return _id; }
+        }
+
+        public ClientSocket Socket
+        {
+            get { return _socket; }
+        }
+
+        /// <summary>
+        /// Returns if transaction is closed.
+        /// </summary>
+        internal bool Closed
+        {
+            get { return _closed; }
+        }
+
+        /// <summary>
+        /// Closes the transaction. 
+        /// </summary>
+        private void Close(bool commit)
+        {
+            if (!_closed)
+            {
+                try
+                {
+                    _ignite.Socket.DoOutInOp<object>(ClientOp.TxEnd,

Review comment:
       1. Fixed.
   2. What exceptions we should cover?




----------------------------------------------------------------
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.

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



[GitHub] [ignite] ptupitsyn commented on a change in pull request #7992: IGNITE-7369 .NET: Thin Client Transactions

Posted by GitBox <gi...@apache.org>.
ptupitsyn commented on a change in pull request #7992:
URL: https://github.com/apache/ignite/pull/7992#discussion_r477398864



##########
File path: modules/platforms/dotnet/Apache.Ignite.Core.Tests/Client/Cache/CacheClientAbstractTxTest.cs
##########
@@ -0,0 +1,822 @@
+/*
+ * 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.
+ */
+
+namespace Apache.Ignite.Core.Tests.Client.Cache
+{
+    using System;
+    using System.Collections.Generic;
+    using System.Linq;
+    using System.Threading;
+    using System.Threading.Tasks;
+    using System.Transactions;
+    using Apache.Ignite.Core.Cache.Configuration;
+    using Apache.Ignite.Core.Client;
+    using Apache.Ignite.Core.Client.Cache;
+    using Apache.Ignite.Core.Client.Transactions;
+    using Apache.Ignite.Core.Impl.Client.Transactions;
+    using Apache.Ignite.Core.Transactions;
+    using NUnit.Framework;
+    using NUnit.Framework.Constraints;
+
+    /// <summary>
+    /// Transactional cache client tests.
+    /// </summary>
+    public abstract class CacheClientAbstractTxTest : ClientTestBase
+    {
+        /** All concurrency controls. */
+        private static readonly TransactionConcurrency[] AllConcurrencyControls =
+        {
+            TransactionConcurrency.Optimistic,
+            TransactionConcurrency.Pessimistic
+        };
+
+        /** All isolation levels*/
+        private static readonly TransactionIsolation[] AllIsolationLevels =
+        {
+            TransactionIsolation.Serializable,
+            TransactionIsolation.ReadCommitted,
+            TransactionIsolation.RepeatableRead
+        };
+
+        /// <summary>
+        /// Initializes a new instance of the <see cref="CacheClientAbstractTxTest"/> class.
+        /// </summary>
+        protected CacheClientAbstractTxTest(int serverCount, bool enablePartitionAwareness) : base(serverCount,
+            enablePartitionAwareness: enablePartitionAwareness)
+        {
+            // No-op.
+        }
+
+        /// <summary>
+        /// Tests that custom client transactions configuration is applied.
+        /// </summary>
+        [Test]
+        public void TestClientTransactionConfiguration()
+        {
+            var timeout = TransactionClientConfiguration.DefaultDefaultTimeout.Add(TimeSpan.FromMilliseconds(1000));
+            var cfg = GetClientConfiguration();
+            cfg.TransactionConfiguration = new TransactionClientConfiguration
+            {
+                DefaultTimeout = timeout
+            };
+
+            foreach (var concurrency in AllConcurrencyControls)
+            {
+                foreach (var isolation in AllIsolationLevels)
+                {
+                    cfg.TransactionConfiguration.DefaultTransactionConcurrency = concurrency;
+                    cfg.TransactionConfiguration.DefaultTransactionIsolation = isolation;
+                    using (var client = Ignition.StartClient(cfg))
+                    {
+                        var transactions = client.GetTransactions();
+                        Assert.AreEqual(concurrency, transactions.DefaultTransactionConcurrency);
+                        Assert.AreEqual(isolation, transactions.DefaultTransactionIsolation);
+                        Assert.AreEqual(timeout, transactions.DefaultTimeout);
+
+                        ITransaction igniteTx;
+                        using (var tx = transactions.TxStart())
+                        {
+                            Assert.AreEqual(tx, transactions.Tx);
+                            Assert.AreEqual(concurrency, tx.Concurrency);
+                            Assert.AreEqual(isolation, tx.Isolation);
+                            Assert.AreEqual(timeout, tx.Timeout);
+
+                            igniteTx = GetSingleLocalTransaction();
+                            Assert.AreEqual(concurrency, igniteTx.Concurrency);
+                            Assert.AreEqual(isolation, igniteTx.Isolation);
+                            Assert.AreEqual(timeout, igniteTx.Timeout);
+                        }
+
+                        igniteTx.Dispose();
+                    }
+                }
+            }
+        }
+
+        /// <summary>
+        /// Tests that parameters passed to TxStart are applied.
+        /// </summary>
+        [Test]
+        public void TestTxStartPassesParameters()
+        {
+            var timeout = TransactionClientConfiguration.DefaultDefaultTimeout.Add(TimeSpan.FromMilliseconds(1000));
+            var acts = new List<Func<ITransactionsClient>>
+            {
+                () => Client.GetTransactions(),
+                () => Client.GetTransactions().WithLabel("label"),
+            };
+            foreach (var concurrency in AllConcurrencyControls)
+            {
+                foreach (var isolation in AllIsolationLevels)
+                {
+                    foreach (var act in acts)
+                    {
+                        var client = act();
+
+                        ITransaction igniteTx;
+                        using (var tx = client.TxStart(concurrency, isolation))
+                        {
+                            Assert.AreEqual(tx, client.Tx);
+                            Assert.AreEqual(concurrency, tx.Concurrency);
+                            Assert.AreEqual(isolation, tx.Isolation);
+
+                            igniteTx = GetSingleLocalTransaction();
+                            Assert.AreEqual(concurrency, igniteTx.Concurrency);
+                            Assert.AreEqual(isolation, igniteTx.Isolation);
+                        }
+
+                        igniteTx.Dispose();
+                        using (var tx = client.TxStart(concurrency, isolation, timeout))
+                        {
+                            Assert.AreEqual(concurrency, tx.Concurrency);
+                            Assert.AreEqual(isolation, tx.Isolation);
+                            Assert.AreEqual(timeout, tx.Timeout);
+
+                            igniteTx = GetSingleLocalTransaction();
+                            Assert.AreEqual(concurrency, igniteTx.Concurrency);
+                            Assert.AreEqual(isolation, igniteTx.Isolation);
+                            Assert.AreEqual(timeout, igniteTx.Timeout);
+                        }
+
+                        igniteTx.Dispose();
+                    }
+                }
+            }
+        }
+
+        /// <summary>
+        /// Tests that transaction can't be committed/rollback after being already completed.
+        /// </summary>
+        [Test]
+        public void TestThrowsIfEndAlreadyCompletedTransaction()
+        {
+            var tx = Client.GetTransactions().TxStart();
+            tx.Commit();
+
+            var constraint = new ReusableConstraint(Is.TypeOf<InvalidOperationException>()
+                .And.Message.Contains("Transaction")
+                .And.Message.Contains("is closed"));
+
+            Assert.Throws(constraint, () => tx.Commit());
+            Assert.Throws(constraint, () => tx.Rollback());
+
+            using (tx = Client.GetTransactions().TxStart())
+            {
+            }
+
+            Assert.Throws(constraint, () => tx.Commit());
+            Assert.Throws(constraint, () => tx.Rollback());
+        }
+
+        /// <summary>
+        /// Tests that transaction throws if timeout elapsed.
+        /// </summary>
+        [Test]
+        public void TestTimeout()
+        {
+            var timeout = TimeSpan.FromMilliseconds(200);
+            var cache = GetTransactionalCache();
+            cache.Put(1, 1);
+            using (var tx = Client.GetTransactions().TxStart(TransactionConcurrency.Pessimistic,
+                TransactionIsolation.ReadCommitted,
+                timeout))
+            {
+                Thread.Sleep(TimeSpan.FromMilliseconds(300));
+                var constraint = new ReusableConstraint(Is.TypeOf<IgniteClientException>()
+                    .And.Message.Contains("Cache transaction timed out"));
+                Assert.Throws(constraint, () => cache.Put(1, 10));
+                Assert.Throws(constraint, () => tx.Commit());
+            }
+
+            Assert.AreEqual(1, cache.Get(1));
+        }
+
+        /// <summary>
+        /// Tests that commit applies cache changes.
+        /// </summary>
+        [Test]
+        public void TestTxCommit()
+        {
+            var cache = GetTransactionalCache();
+
+            cache.Put(1, 1);
+            cache.Put(2, 2);
+
+            using (var tx = Client.GetTransactions().TxStart())
+            {
+                cache.Put(1, 10);
+                cache.Put(2, 20);
+
+                tx.Commit();
+            }
+
+            Assert.AreEqual(10, cache.Get(1));
+            Assert.AreEqual(20, cache.Get(2));
+        }
+
+        /// <summary>
+        /// Tests that rollback reverts cache changes.
+        /// </summary>
+        [Test]
+        public void TestTxRollback()
+        {
+            var cache = GetTransactionalCache();
+            cache.Put(1, 1);
+            cache.Put(2, 2);
+
+            using (var tx = Client.GetTransactions().TxStart())
+            {
+                cache.Put(1, 10);
+                cache.Put(2, 20);
+
+                Assert.AreEqual(10, cache.Get(1));
+                Assert.AreEqual(20, cache.Get(2));
+                tx.Rollback();
+            }
+
+            Assert.AreEqual(1, cache.Get(1));
+            Assert.AreEqual(2, cache.Get(2));
+        }
+
+        /// <summary>
+        /// Tests that closing transaction without commit reverts cache changes.
+        /// </summary>
+        [Test]
+        public void TestTxClose()
+        {
+            var cache = GetTransactionalCache();
+
+            cache.Put(1, 1);
+            cache.Put(2, 2);
+
+            using (Client.GetTransactions().TxStart())
+            {
+                cache.Put(1, 10);
+                cache.Put(2, 20);
+            }
+
+            Assert.AreEqual(1, cache.Get(1));
+            Assert.AreEqual(2, cache.Get(2));
+        }
+
+        /// <summary>
+        /// Tests that client can't start multiple transactions in one thread.
+        /// </summary>
+        [Test]
+        public void TestThrowsIfMultipleStarted()
+        {
+            TestThrowsIfMultipleStarted(
+                () => Client.GetTransactions().TxStart(),
+                () => Client.GetTransactions().TxStart());
+        }
+
+        /// <summary>
+        /// Tests that different clients can start transactions in one thread.
+        /// </summary>
+        [Test]
+        public void TestDifferentClientsCanStartTransactions()
+        {
+            Assert.DoesNotThrow(() =>
+            {
+                using (Client.GetTransactions().TxStart())
+                using (GetClient().GetTransactions().TxStart())
+                {
+                    // No-op.

Review comment:
       We should check that operations can be performed and that one client does not use the TX started by the other.
   I think this is not the case, since we only have one thread local which is shared by all clients.




----------------------------------------------------------------
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.

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



[GitHub] [ignite] ptupitsyn commented on a change in pull request #7992: IGNITE-7369 .NET: Thin Client Transactions

Posted by GitBox <gi...@apache.org>.
ptupitsyn commented on a change in pull request #7992:
URL: https://github.com/apache/ignite/pull/7992#discussion_r477362042



##########
File path: modules/platforms/dotnet/Apache.Ignite.Core/Impl/Client/Transactions/TransactionsClient.cs
##########
@@ -0,0 +1,257 @@
+/*
+ * 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.
+ */
+
+namespace Apache.Ignite.Core.Impl.Client.Transactions
+{
+    using System;
+    using System.Diagnostics.CodeAnalysis;
+    using System.Threading;
+    using Apache.Ignite.Core.Client;
+    using Apache.Ignite.Core.Client.Transactions;
+    using Apache.Ignite.Core.Impl.Binary;
+    using Apache.Ignite.Core.Impl.Common;
+    using Apache.Ignite.Core.Transactions;
+
+    /// <summary>
+    /// Ignite Thin Client transactions facade.
+    /// </summary>
+    internal class TransactionsClient : ITransactionsClient, IDisposable
+    {
+        /** Default transaction configuration. */
+        private readonly TransactionClientConfiguration _cfg;
+
+        /** Transaction for this thread and client. */
+        private readonly ThreadLocal<TransactionClient> _currentTx = new ThreadLocal<TransactionClient>();
+
+        /** Ignite. */
+        private readonly IgniteClient _ignite;
+
+        /** Transaction manager. */
+        private readonly ClientCacheTransactionManager _txManager;
+
+        /// <summary>
+        /// Constructor.
+        /// </summary>
+        /// <param name="ignite">Ignite.</param>
+        /// <param name="cfg"></param>
+        public TransactionsClient(IgniteClient ignite, TransactionClientConfiguration cfg)
+        {
+            _ignite = ignite;
+            _cfg = cfg ?? new TransactionClientConfiguration();
+            _txManager = new ClientCacheTransactionManager(this);
+        }
+
+        /** <inheritDoc /> */
+        [SuppressMessage("Microsoft.Usage", "CA1816:CallGCSuppressFinalizeCorrectly",
+            Justification = "There is no finalizer.")]
+        public void Dispose()
+        {
+            _currentTx.Dispose();
+            _txManager.Dispose();
+        }
+
+        /// <summary>
+        /// Starts ambient transaction if needed.
+        /// </summary>
+        internal void StartTxIfNeeded()
+        {
+            _txManager.StartTxIfNeeded();
+        }
+
+        /** <inheritDoc /> */
+        ITransactionClient ITransactionsClient.Tx
+        {
+            get { return Tx; }
+        }
+
+        /// <summary>
+        /// Gets transaction started by this thread or null if this thread does not have a transaction.
+        /// </summary>
+        internal TransactionClient Tx
+        {
+            get
+            {
+                var tx = _currentTx.Value;
+
+                if (tx == null)
+                    return null;
+
+                if (tx.Closed)

Review comment:
       Also, when socket is disconnected, we should clear all related thread locals to avoid leading memory.




----------------------------------------------------------------
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.

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



[GitHub] [ignite] gurustron commented on a change in pull request #7992: IGNITE-7369 .NET: Thin Client Transactions

Posted by GitBox <gi...@apache.org>.
gurustron commented on a change in pull request #7992:
URL: https://github.com/apache/ignite/pull/7992#discussion_r458400450



##########
File path: modules/platforms/dotnet/Apache.Ignite.Core/Client/Transactions/TransactionClientConfiguration.cs
##########
@@ -0,0 +1,81 @@
+/*
+ * 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.
+ */
+
+namespace Apache.Ignite.Core.Client.Transactions
+{
+    using System;
+    using System.ComponentModel;
+    using Apache.Ignite.Core.Impl.Common;
+    using Apache.Ignite.Core.Transactions;
+
+    /// <summary>
+    /// Transactions configuration.
+    /// </summary>
+    public class TransactionClientConfiguration
+    {
+        /// <summary> The default value for <see cref="DefaultTransactionConcurrency"/> property. </summary>
+        public const TransactionConcurrency DefaultDefaultTransactionConcurrency = TransactionConcurrency.Pessimistic;
+
+        /// <summary> The default value for <see cref="DefaultTransactionIsolation"/> property. </summary>
+        public const TransactionIsolation DefaultDefaultTransactionIsolation = TransactionIsolation.RepeatableRead;
+
+        /// <summary> The default value for <see cref="DefaultTransactionIsolation"/> property. </summary>
+        public static readonly TimeSpan DefaultDefaultTimeout = TimeSpan.Zero;
+
+        /// <summary>
+        /// Gets or sets the cache transaction concurrency to use when one is not explicitly specified.
+        /// </summary>
+        [DefaultValue(DefaultDefaultTransactionConcurrency)]
+        public TransactionConcurrency DefaultTransactionConcurrency { get; set; }
+
+        /// <summary>
+        /// Gets or sets the cache transaction isolation to use when one is not explicitly specified.
+        /// </summary>
+        [DefaultValue(DefaultDefaultTransactionIsolation)]
+        public TransactionIsolation DefaultTransactionIsolation { get; set; }
+
+        /// <summary>
+        /// Gets or sets the cache transaction timeout to use when one is not explicitly specified.
+        /// <see cref="TimeSpan.Zero"/> for infinite timeout.
+        /// </summary>
+        [DefaultValue(typeof(TimeSpan), "00:00:00")]
+        public TimeSpan DefaultTimeout { get; set; }
+
+        /// <summary>
+        /// Initializes a new instance of the <see cref="TransactionClientConfiguration" /> class.
+        /// </summary>
+        public TransactionClientConfiguration()

Review comment:
       1.  IgniteConfigurationTest.TestDefaultValueAttributes:103 for default ctor. Will add one for non-default one
   2.  CacheClientAbstractTxTest.TestClientTransactionConfiguration would not be sufficient?




----------------------------------------------------------------
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.

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



[GitHub] [ignite] ptupitsyn merged pull request #7992: IGNITE-7369 .NET: Thin Client Transactions

Posted by GitBox <gi...@apache.org>.
ptupitsyn merged pull request #7992:
URL: https://github.com/apache/ignite/pull/7992


   


----------------------------------------------------------------
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.

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



[GitHub] [ignite] gurustron commented on a change in pull request #7992: IGNITE-7369 Thin Client Transactions [DRAFT]

Posted by GitBox <gi...@apache.org>.
gurustron commented on a change in pull request #7992:
URL: https://github.com/apache/ignite/pull/7992#discussion_r451151424



##########
File path: modules/platforms/dotnet/Apache.Ignite.Core/Client/Transactions/IClientTransactions.cs
##########
@@ -0,0 +1,58 @@
+/*
+ * 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.
+ */
+
+namespace Apache.Ignite.Core.Client.Transactions
+{
+    using System;
+    using Apache.Ignite.Core.Transactions;
+
+    /// <summary>
+    /// Ignite Thin Client transactions facade.
+    /// </summary>
+    public interface IClientTransactions

Review comment:
       I think it can be better compared to `IClientCluster` , `IClientClusterNode` and so on. To me `ITransactionsClient` sounds a little bit misleading and clumsy. 




----------------------------------------------------------------
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.

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



[GitHub] [ignite] gurustron commented on a change in pull request #7992: IGNITE-7369 .NET: Thin Client Transactions

Posted by GitBox <gi...@apache.org>.
gurustron commented on a change in pull request #7992:
URL: https://github.com/apache/ignite/pull/7992#discussion_r464068406



##########
File path: modules/platforms/dotnet/Apache.Ignite.Core/Client/Transactions/ITransactionClient.cs
##########
@@ -0,0 +1,46 @@
+/*
+ * 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.
+ */
+
+namespace Apache.Ignite.Core.Client.Transactions
+{
+    using System;
+    using Apache.Ignite.Core.Transactions;
+
+    /// <summary>
+    /// Thin client transaction.
+    /// </summary>
+    public interface ITransactionClient : IDisposable
+    {
+        /// <summary>
+        /// Commits this transaction.
+        /// </summary>
+        void Commit();
+
+        /// <summary>
+        /// Rolls back this transaction.
+        /// </summary>
+        void Rollback();
+        
+        TransactionConcurrency Concurrency { get; } 
+        
+        TransactionIsolation Isolation { get; }
+
+        TimeSpan Timeout { get; }
+
+        string Label { get; }

Review comment:
       Added to `TestWithLabel`




----------------------------------------------------------------
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.

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



[GitHub] [ignite] ptupitsyn commented on a change in pull request #7992: IGNITE-7369 Thin Client Transactions [DRAFT]

Posted by GitBox <gi...@apache.org>.
ptupitsyn commented on a change in pull request #7992:
URL: https://github.com/apache/ignite/pull/7992#discussion_r451856987



##########
File path: modules/platforms/dotnet/Apache.Ignite.Core/Client/Transactions/IClientTransactions.cs
##########
@@ -0,0 +1,58 @@
+/*
+ * 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.
+ */
+
+namespace Apache.Ignite.Core.Client.Transactions
+{
+    using System;
+    using Apache.Ignite.Core.Transactions;
+
+    /// <summary>
+    /// Ignite Thin Client transactions facade.
+    /// </summary>
+    public interface IClientTransactions

Review comment:
       Actually, `IClientCluster` was missed during the review, so we have an inconsistency now, but I'd like to avoid further inconsistencies.




----------------------------------------------------------------
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.

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



[GitHub] [ignite] ptupitsyn commented on a change in pull request #7992: IGNITE-7369 Thin Client Transactions [DRAFT]

Posted by GitBox <gi...@apache.org>.
ptupitsyn commented on a change in pull request #7992:
URL: https://github.com/apache/ignite/pull/7992#discussion_r450973219



##########
File path: modules/platforms/dotnet/Apache.Ignite.Core.Tests/Apache.Ignite.Core.Tests.csproj
##########
@@ -143,6 +143,9 @@
     <Compile Include="Cache\Query\Linq\CacheLinqTest.Contains.cs" />
     <Compile Include="Cache\Store\CacheStoreSessionTestCodeConfig.cs" />
     <Compile Include="Cache\Store\CacheStoreSessionTestSharedFactory.cs" />
+    <Compile Include="Client\Cache\CacheClientAbstractTxTest.cs" />

Review comment:
       Please add those tests to `.DotNetCore` project as well.

##########
File path: modules/platforms/dotnet/Apache.Ignite.Core.Tests/Client/Cache/CacheClientLocalTxTest.cs
##########
@@ -0,0 +1,38 @@
+/*
+ * 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.
+ */
+
+namespace Apache.Ignite.Core.Tests.Client.Cache
+{
+    /// <summary>
+    /// Tests client transactions for single node.
+    /// </summary>
+    public class CacheClientLocalTxTest : CacheClientAbstractTxTest
+    {
+        /// <summary>
+        ///  Initializes a new instance of the <see cref="CacheClientLocalTxTest"/> class.
+        /// </summary>
+        public CacheClientLocalTxTest() : base(1, false)
+        {
+            // No-op.
+        }
+
+        protected override string GetCacheName()

Review comment:
       Missing `inheritdoc`

##########
File path: modules/platforms/dotnet/Apache.Ignite.Core/Client/Transactions/IClientTransactions.cs
##########
@@ -0,0 +1,58 @@
+/*
+ * 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.
+ */
+
+namespace Apache.Ignite.Core.Client.Transactions
+{
+    using System;
+    using Apache.Ignite.Core.Transactions;
+
+    /// <summary>
+    /// Ignite Thin Client transactions facade.
+    /// </summary>
+    public interface IClientTransactions
+    {
+        /// <summary>
+        /// Starts a new transaction with the default isolation level, concurrency and timeout. 
+        /// </summary>
+        /// <returns>New transaction.</returns>
+        IClientTransaction TxStart();
+
+        /// <summary>
+        /// Starts new transaction with the specified concurrency and isolation.
+        /// </summary>
+        /// <param name="concurrency">Concurrency.</param>
+        /// <param name="isolation">Isolation.</param>
+        /// <returns>New transaction.</returns>
+        IClientTransaction TxStart(TransactionConcurrency concurrency, TransactionIsolation isolation);
+
+        /// <summary>
+        /// Starts new transaction with the specified concurrency, isolation and timeout.
+        /// </summary>
+        /// <param name="concurrency">Concurrency.</param>
+        /// <param name="isolation">Isolation.</param>
+        /// <param name="timeout">Timeout. TimeSpan. Zero for indefinite timeout.</param>
+        /// <returns>New transaction.</returns>
+        IClientTransaction TxStart(TransactionConcurrency concurrency, TransactionIsolation isolation, TimeSpan timeout);
+
+        /// <summary>
+        /// Returns instance of <see cref="IClientTransactions"/>> to mark a transaction with a special label.

Review comment:
       Please add more explanation - why is this useful, how to retrieve this label

##########
File path: modules/platforms/dotnet/Apache.Ignite.Core/Client/Transactions/IClientTransactions.cs
##########
@@ -0,0 +1,58 @@
+/*
+ * 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.
+ */
+
+namespace Apache.Ignite.Core.Client.Transactions
+{
+    using System;
+    using Apache.Ignite.Core.Transactions;
+
+    /// <summary>
+    /// Ignite Thin Client transactions facade.
+    /// </summary>
+    public interface IClientTransactions

Review comment:
       Let's rename to `ITransactionsClient` to be consistent with `ICacheClient`, `IComputeClient`.

##########
File path: modules/platforms/dotnet/Apache.Ignite.Core/Client/Transactions/IClientTransaction.cs
##########
@@ -0,0 +1,37 @@
+/*
+ * 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.
+ */
+
+namespace Apache.Ignite.Core.Client.Transactions
+{
+    using System;
+
+    /// <summary>
+    /// Thin client transaction.
+    /// </summary>
+    public interface IClientTransaction : IDisposable

Review comment:
       Let's rename to `ITransactionClient` to be consistent with `ICacheClient`, `IComputeClient`.

##########
File path: modules/platforms/dotnet/Apache.Ignite.Core/Impl/Client/Cache/CacheClient.cs
##########
@@ -754,14 +797,26 @@ private void WriteRequest(Action<ClientRequestContext> writeAction, ClientReques
         {
             ctx.Stream.WriteInt(_id);
 
+            var flags = ClientCacheRequestFlag.None;
             if (_expiryPolicy != null)
             {
                 ctx.Features.ValidateWithExpiryPolicyFlag();
-                ctx.Stream.WriteByte((byte) ClientCacheRequestFlag.WithExpiryPolicy);
-                ExpiryPolicySerializer.WritePolicy(ctx.Writer, _expiryPolicy);
+                flags = flags | ClientCacheRequestFlag.WithExpiryPolicy;
             }
-            else
-                ctx.Stream.WriteByte((byte) ClientCacheRequestFlag.None); // Flags (skipStore, etc).
+
+            var tx = _ignite.Transactions.CurrentTx;
+            if (tx != null)
+            {
+                flags |= ClientCacheRequestFlag.WithTransactional;
+            }
+
+            ctx.Stream.WriteByte((byte) flags);
+
+            if (flags.HasFlag(ClientCacheRequestFlag.WithExpiryPolicy))

Review comment:
       `HasFlag` is [slow on older frameworks](https://www.code4it.dev/blog/hasflag-performance-benchmarkdotnet), let's avoid it here.

##########
File path: modules/platforms/dotnet/Apache.Ignite.Core/Client/IIgniteClient.cs
##########
@@ -111,6 +112,11 @@ public interface IIgniteClient : IDisposable
         [SuppressMessage("Microsoft.Design", "CA1024:UsePropertiesWhereAppropriate", Justification = "Semantics.")]
         IBinary GetBinary();
 
+        /// <summary>
+        /// Gets Ignite transactions facade.
+        /// </summary>
+        IClientTransactions Transactions { get; }

Review comment:
       Please convert to a method for consistency with `IIgnite.GetTransactions`.
   Method is also a more future-proof option (see https://docs.microsoft.com/en-us/previous-versions/dotnet/netframework-4.0/ms229054(v=vs.100)).

##########
File path: modules/platforms/dotnet/Apache.Ignite.Core/Impl/Client/Transactions/ClientTransaction.cs
##########
@@ -0,0 +1,125 @@
+/*
+ * 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.
+ */
+
+namespace Apache.Ignite.Core.Impl.Client.Transactions
+{
+    using System;
+    using System.Threading;
+
+    /// <summary>
+    /// Ignite Thin Client transaction facade.
+    /// </summary>
+    internal class ClientTransaction: IClientTransactionInternal
+    {
+        /** Unique  transaction ID.*/
+        private readonly int _id;
+
+        /** Ignite. */
+        private readonly IgniteClient _ignite;
+
+        /* Transactions. */
+        private readonly ClientTransactions _transactions;
+
+        /** Transaction is closed. */
+        private volatile bool _closed; 

Review comment:
       Do we need `volatile`? Transactions are tied to a specific thread, right?

##########
File path: modules/platforms/dotnet/Apache.Ignite.Core/Impl/Client/ClientFailoverSocket.cs
##########
@@ -269,6 +278,12 @@ private ClientSocket GetAffinitySocket<TKey>(int cacheId, TKey key)
                 return null;
             }
 
+            // Transactional operation should be executed on node started the transaction.
+            if (_transactions.CurrentTx != null)

Review comment:
       To be precise: transactional operation must be executed using the same connection (same `ClientSocket` instance) that started the transaction.
   
   And the check here does not guarantee this at all - reconnect could have happened, for example.
   A proper approach is to store get the socket from `ClientContextBase.Socket` property when executing `ClientOp.TxStart`, and use that socket directly for transactional operations.




----------------------------------------------------------------
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.

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



[GitHub] [ignite] ptupitsyn commented on a change in pull request #7992: IGNITE-7369 Thin Client Transactions [DRAFT]

Posted by GitBox <gi...@apache.org>.
ptupitsyn commented on a change in pull request #7992:
URL: https://github.com/apache/ignite/pull/7992#discussion_r456610186



##########
File path: modules/platforms/dotnet/Apache.Ignite.Core/Impl/Client/Transactions/ClientTransaction.cs
##########
@@ -0,0 +1,125 @@
+/*
+ * 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.
+ */
+
+namespace Apache.Ignite.Core.Impl.Client.Transactions
+{
+    using System;
+    using System.Threading;
+
+    /// <summary>
+    /// Ignite Thin Client transaction facade.
+    /// </summary>
+    internal class ClientTransaction: IClientTransactionInternal
+    {
+        /** Unique  transaction ID.*/
+        private readonly int _id;
+
+        /** Ignite. */
+        private readonly IgniteClient _ignite;
+
+        /* Transactions. */
+        private readonly ClientTransactions _transactions;
+
+        /** Transaction is closed. */
+        private volatile bool _closed; 

Review comment:
       Usually, types and APIs are not thread safe, unless stated otherwise. E.g. we don't expect `List<T>` to be thread safe and have volatile fields. We explicitly state that Ignite transactions are per-thread, so we don't need to guarantee thread safety.




----------------------------------------------------------------
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.

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



[GitHub] [ignite] gurustron commented on a change in pull request #7992: IGNITE-7369 .NET: Thin Client Transactions

Posted by GitBox <gi...@apache.org>.
gurustron commented on a change in pull request #7992:
URL: https://github.com/apache/ignite/pull/7992#discussion_r464069966



##########
File path: modules/platforms/dotnet/Apache.Ignite.Core/Impl/Client/Transactions/TransactionClient.cs
##########
@@ -0,0 +1,172 @@
+/*
+ * 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.
+ */
+
+namespace Apache.Ignite.Core.Impl.Client.Transactions
+{
+    using System;
+    using System.Globalization;
+    using Apache.Ignite.Core.Client.Transactions;
+    using Apache.Ignite.Core.Transactions;
+
+    /// <summary>
+    /// Ignite Thin Client transaction facade.
+    /// </summary>
+    internal class TransactionClient : ITransactionClient
+    {
+        /** Unique  transaction ID.*/
+        private readonly int _id;
+
+        /** Ignite. */
+        private readonly IgniteClient _ignite;
+
+        /** Socket. */
+        private readonly ClientSocket _socket;
+
+        /** Transaction is closed. */
+        private bool _closed;
+
+        /// <summary>
+        /// Constructor.
+        /// </summary>
+        /// <param name="id">ID.</param>
+        /// <param name="ignite">Ignite.</param>
+        /// <param name="socket">Socket.</param>
+        /// <param name="concurrency">Concurrency.</param>
+        /// <param name="isolation">Isolation.</param>
+        /// <param name="timeout">Timeout.</param>
+        /// <param name="label">Label.</param>
+        public TransactionClient(int id,
+            IgniteClient ignite,
+            ClientSocket socket,
+            TransactionConcurrency concurrency,
+            TransactionIsolation isolation,
+            TimeSpan timeout,
+            string label)
+        {
+            _id = id;
+            _ignite = ignite;
+            _socket = socket;
+            Concurrency = concurrency;
+            Isolation = isolation;
+            Timeout = timeout;
+            Label = label;
+        }
+
+        /** <inheritdoc /> */
+        public void Commit()
+        {
+            ThrowIfClosed();
+            Close(true);
+        }
+
+        /** <inheritdoc /> */
+        public void Rollback()
+        {
+            ThrowIfClosed();
+            Close(false);
+        }
+
+        /** <inheritdoc /> */
+        public TransactionConcurrency Concurrency { get; private set; }
+
+        /** <inheritdoc /> */
+        public TransactionIsolation Isolation { get; private set; }
+
+        /** <inheritdoc /> */
+        public TimeSpan Timeout { get; private set; }
+
+        /** <inheritdoc /> */
+        public string Label { get; private set; }
+
+        /** <inheritdoc /> */
+        public void Dispose()
+        {
+            try
+            {
+                Close(false);
+            }
+            finally
+            {
+                GC.SuppressFinalize(this);
+            }
+        }
+
+        /// <summary>
+        /// Transaction Id.
+        /// </summary>
+        public int Id
+        {
+            get { return _id; }
+        }
+
+        public ClientSocket Socket
+        {
+            get { return _socket; }
+        }
+
+        /// <summary>
+        /// Returns if transaction is closed.
+        /// </summary>
+        internal bool Closed
+        {
+            get { return _closed; }
+        }
+
+        /// <summary>
+        /// Closes the transaction. 
+        /// </summary>
+        private void Close(bool commit)
+        {
+            if (!_closed)
+            {
+                try
+                {
+                    _ignite.Socket.DoOutInOp<object>(ClientOp.TxEnd,

Review comment:
       1. Fixed.
   2. What exceptions should we cover?




----------------------------------------------------------------
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.

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



[GitHub] [ignite] ptupitsyn commented on a change in pull request #7992: IGNITE-7369 .NET: Thin Client Transactions

Posted by GitBox <gi...@apache.org>.
ptupitsyn commented on a change in pull request #7992:
URL: https://github.com/apache/ignite/pull/7992#discussion_r477356355



##########
File path: modules/platforms/dotnet/Apache.Ignite.Core/Impl/Client/Transactions/TransactionsClient.cs
##########
@@ -0,0 +1,257 @@
+/*
+ * 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.
+ */
+
+namespace Apache.Ignite.Core.Impl.Client.Transactions
+{
+    using System;
+    using System.Diagnostics.CodeAnalysis;
+    using System.Threading;
+    using Apache.Ignite.Core.Client;
+    using Apache.Ignite.Core.Client.Transactions;
+    using Apache.Ignite.Core.Impl.Binary;
+    using Apache.Ignite.Core.Impl.Common;
+    using Apache.Ignite.Core.Transactions;
+
+    /// <summary>
+    /// Ignite Thin Client transactions facade.
+    /// </summary>
+    internal class TransactionsClient : ITransactionsClient, IDisposable
+    {
+        /** Default transaction configuration. */
+        private readonly TransactionClientConfiguration _cfg;
+
+        /** Transaction for this thread and client. */
+        private readonly ThreadLocal<TransactionClient> _currentTx = new ThreadLocal<TransactionClient>();
+
+        /** Ignite. */
+        private readonly IgniteClient _ignite;
+
+        /** Transaction manager. */
+        private readonly ClientCacheTransactionManager _txManager;
+
+        /// <summary>
+        /// Constructor.
+        /// </summary>
+        /// <param name="ignite">Ignite.</param>
+        /// <param name="cfg"></param>
+        public TransactionsClient(IgniteClient ignite, TransactionClientConfiguration cfg)
+        {
+            _ignite = ignite;
+            _cfg = cfg ?? new TransactionClientConfiguration();
+            _txManager = new ClientCacheTransactionManager(this);
+        }
+
+        /** <inheritDoc /> */
+        [SuppressMessage("Microsoft.Usage", "CA1816:CallGCSuppressFinalizeCorrectly",
+            Justification = "There is no finalizer.")]
+        public void Dispose()
+        {
+            _currentTx.Dispose();
+            _txManager.Dispose();
+        }
+
+        /// <summary>
+        /// Starts ambient transaction if needed.
+        /// </summary>
+        internal void StartTxIfNeeded()
+        {
+            _txManager.StartTxIfNeeded();
+        }
+
+        /** <inheritDoc /> */
+        ITransactionClient ITransactionsClient.Tx
+        {
+            get { return Tx; }
+        }
+
+        /// <summary>
+        /// Gets transaction started by this thread or null if this thread does not have a transaction.
+        /// </summary>
+        internal TransactionClient Tx
+        {
+            get
+            {
+                var tx = _currentTx.Value;
+
+                if (tx == null)
+                    return null;
+
+                if (tx.Closed)

Review comment:
       We should handle an edge case when multiple clients are used from the same thread:
   check that `tx` actually belongs to the current `IgniteClient` and has the same `ClientSocket`.
   
   If there is an active transaction for another client or socket, I think we should throw  `NotSupportedException`.




----------------------------------------------------------------
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.

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



[GitHub] [ignite] gurustron commented on a change in pull request #7992: IGNITE-7369 .NET: Thin Client Transactions

Posted by GitBox <gi...@apache.org>.
gurustron commented on a change in pull request #7992:
URL: https://github.com/apache/ignite/pull/7992#discussion_r458400450



##########
File path: modules/platforms/dotnet/Apache.Ignite.Core/Client/Transactions/TransactionClientConfiguration.cs
##########
@@ -0,0 +1,81 @@
+/*
+ * 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.
+ */
+
+namespace Apache.Ignite.Core.Client.Transactions
+{
+    using System;
+    using System.ComponentModel;
+    using Apache.Ignite.Core.Impl.Common;
+    using Apache.Ignite.Core.Transactions;
+
+    /// <summary>
+    /// Transactions configuration.
+    /// </summary>
+    public class TransactionClientConfiguration
+    {
+        /// <summary> The default value for <see cref="DefaultTransactionConcurrency"/> property. </summary>
+        public const TransactionConcurrency DefaultDefaultTransactionConcurrency = TransactionConcurrency.Pessimistic;
+
+        /// <summary> The default value for <see cref="DefaultTransactionIsolation"/> property. </summary>
+        public const TransactionIsolation DefaultDefaultTransactionIsolation = TransactionIsolation.RepeatableRead;
+
+        /// <summary> The default value for <see cref="DefaultTransactionIsolation"/> property. </summary>
+        public static readonly TimeSpan DefaultDefaultTimeout = TimeSpan.Zero;
+
+        /// <summary>
+        /// Gets or sets the cache transaction concurrency to use when one is not explicitly specified.
+        /// </summary>
+        [DefaultValue(DefaultDefaultTransactionConcurrency)]
+        public TransactionConcurrency DefaultTransactionConcurrency { get; set; }
+
+        /// <summary>
+        /// Gets or sets the cache transaction isolation to use when one is not explicitly specified.
+        /// </summary>
+        [DefaultValue(DefaultDefaultTransactionIsolation)]
+        public TransactionIsolation DefaultTransactionIsolation { get; set; }
+
+        /// <summary>
+        /// Gets or sets the cache transaction timeout to use when one is not explicitly specified.
+        /// <see cref="TimeSpan.Zero"/> for infinite timeout.
+        /// </summary>
+        [DefaultValue(typeof(TimeSpan), "00:00:00")]
+        public TimeSpan DefaultTimeout { get; set; }
+
+        /// <summary>
+        /// Initializes a new instance of the <see cref="TransactionClientConfiguration" /> class.
+        /// </summary>
+        public TransactionClientConfiguration()

Review comment:
       1.  There are already tests for it - IgniteConfigurationTest.TestDefaultValueAttributes:103 for default ctor And IgniteClientConfigurationTest.TestTransactionConfigurationCopyCtor for copy one.
   2.  CacheClientAbstractTxTest.TestClientTransactionConfiguration would not be sufficient?




----------------------------------------------------------------
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.

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



[GitHub] [ignite] gurustron commented on a change in pull request #7992: IGNITE-7369 .NET: Thin Client Transactions

Posted by GitBox <gi...@apache.org>.
gurustron commented on a change in pull request #7992:
URL: https://github.com/apache/ignite/pull/7992#discussion_r464069966



##########
File path: modules/platforms/dotnet/Apache.Ignite.Core/Impl/Client/Transactions/TransactionClient.cs
##########
@@ -0,0 +1,172 @@
+/*
+ * 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.
+ */
+
+namespace Apache.Ignite.Core.Impl.Client.Transactions
+{
+    using System;
+    using System.Globalization;
+    using Apache.Ignite.Core.Client.Transactions;
+    using Apache.Ignite.Core.Transactions;
+
+    /// <summary>
+    /// Ignite Thin Client transaction facade.
+    /// </summary>
+    internal class TransactionClient : ITransactionClient
+    {
+        /** Unique  transaction ID.*/
+        private readonly int _id;
+
+        /** Ignite. */
+        private readonly IgniteClient _ignite;
+
+        /** Socket. */
+        private readonly ClientSocket _socket;
+
+        /** Transaction is closed. */
+        private bool _closed;
+
+        /// <summary>
+        /// Constructor.
+        /// </summary>
+        /// <param name="id">ID.</param>
+        /// <param name="ignite">Ignite.</param>
+        /// <param name="socket">Socket.</param>
+        /// <param name="concurrency">Concurrency.</param>
+        /// <param name="isolation">Isolation.</param>
+        /// <param name="timeout">Timeout.</param>
+        /// <param name="label">Label.</param>
+        public TransactionClient(int id,
+            IgniteClient ignite,
+            ClientSocket socket,
+            TransactionConcurrency concurrency,
+            TransactionIsolation isolation,
+            TimeSpan timeout,
+            string label)
+        {
+            _id = id;
+            _ignite = ignite;
+            _socket = socket;
+            Concurrency = concurrency;
+            Isolation = isolation;
+            Timeout = timeout;
+            Label = label;
+        }
+
+        /** <inheritdoc /> */
+        public void Commit()
+        {
+            ThrowIfClosed();
+            Close(true);
+        }
+
+        /** <inheritdoc /> */
+        public void Rollback()
+        {
+            ThrowIfClosed();
+            Close(false);
+        }
+
+        /** <inheritdoc /> */
+        public TransactionConcurrency Concurrency { get; private set; }
+
+        /** <inheritdoc /> */
+        public TransactionIsolation Isolation { get; private set; }
+
+        /** <inheritdoc /> */
+        public TimeSpan Timeout { get; private set; }
+
+        /** <inheritdoc /> */
+        public string Label { get; private set; }
+
+        /** <inheritdoc /> */
+        public void Dispose()
+        {
+            try
+            {
+                Close(false);
+            }
+            finally
+            {
+                GC.SuppressFinalize(this);
+            }
+        }
+
+        /// <summary>
+        /// Transaction Id.
+        /// </summary>
+        public int Id
+        {
+            get { return _id; }
+        }
+
+        public ClientSocket Socket
+        {
+            get { return _socket; }
+        }
+
+        /// <summary>
+        /// Returns if transaction is closed.
+        /// </summary>
+        internal bool Closed
+        {
+            get { return _closed; }
+        }
+
+        /// <summary>
+        /// Closes the transaction. 
+        /// </summary>
+        private void Close(bool commit)
+        {
+            if (!_closed)
+            {
+                try
+                {
+                    _ignite.Socket.DoOutInOp<object>(ClientOp.TxEnd,

Review comment:
       1. Fixed.
   2. What exceptions we shoud cover?




----------------------------------------------------------------
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.

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



[GitHub] [ignite] gurustron commented on a change in pull request #7992: IGNITE-7369 .NET: Thin Client Transactions

Posted by GitBox <gi...@apache.org>.
gurustron commented on a change in pull request #7992:
URL: https://github.com/apache/ignite/pull/7992#discussion_r487460081



##########
File path: modules/platforms/dotnet/Apache.Ignite.Core.Tests/Client/Cache/CacheClientAbstractTxTest.cs
##########
@@ -0,0 +1,822 @@
+/*
+ * 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.
+ */
+
+namespace Apache.Ignite.Core.Tests.Client.Cache
+{
+    using System;
+    using System.Collections.Generic;
+    using System.Linq;
+    using System.Threading;
+    using System.Threading.Tasks;
+    using System.Transactions;
+    using Apache.Ignite.Core.Cache.Configuration;
+    using Apache.Ignite.Core.Client;
+    using Apache.Ignite.Core.Client.Cache;
+    using Apache.Ignite.Core.Client.Transactions;
+    using Apache.Ignite.Core.Impl.Client.Transactions;
+    using Apache.Ignite.Core.Transactions;
+    using NUnit.Framework;
+    using NUnit.Framework.Constraints;
+
+    /// <summary>
+    /// Transactional cache client tests.
+    /// </summary>
+    public abstract class CacheClientAbstractTxTest : ClientTestBase
+    {
+        /** All concurrency controls. */
+        private static readonly TransactionConcurrency[] AllConcurrencyControls =
+        {
+            TransactionConcurrency.Optimistic,
+            TransactionConcurrency.Pessimistic
+        };
+
+        /** All isolation levels*/
+        private static readonly TransactionIsolation[] AllIsolationLevels =
+        {
+            TransactionIsolation.Serializable,
+            TransactionIsolation.ReadCommitted,
+            TransactionIsolation.RepeatableRead
+        };
+
+        /// <summary>
+        /// Initializes a new instance of the <see cref="CacheClientAbstractTxTest"/> class.
+        /// </summary>
+        protected CacheClientAbstractTxTest(int serverCount, bool enablePartitionAwareness) : base(serverCount,
+            enablePartitionAwareness: enablePartitionAwareness)
+        {
+            // No-op.
+        }
+
+        /// <summary>
+        /// Tests that custom client transactions configuration is applied.
+        /// </summary>
+        [Test]
+        public void TestClientTransactionConfiguration()
+        {
+            var timeout = TransactionClientConfiguration.DefaultDefaultTimeout.Add(TimeSpan.FromMilliseconds(1000));
+            var cfg = GetClientConfiguration();
+            cfg.TransactionConfiguration = new TransactionClientConfiguration
+            {
+                DefaultTimeout = timeout
+            };
+
+            foreach (var concurrency in AllConcurrencyControls)
+            {
+                foreach (var isolation in AllIsolationLevels)
+                {
+                    cfg.TransactionConfiguration.DefaultTransactionConcurrency = concurrency;
+                    cfg.TransactionConfiguration.DefaultTransactionIsolation = isolation;
+                    using (var client = Ignition.StartClient(cfg))
+                    {
+                        var transactions = client.GetTransactions();
+                        Assert.AreEqual(concurrency, transactions.DefaultTransactionConcurrency);
+                        Assert.AreEqual(isolation, transactions.DefaultTransactionIsolation);
+                        Assert.AreEqual(timeout, transactions.DefaultTimeout);
+
+                        ITransaction igniteTx;
+                        using (var tx = transactions.TxStart())
+                        {
+                            Assert.AreEqual(tx, transactions.Tx);
+                            Assert.AreEqual(concurrency, tx.Concurrency);
+                            Assert.AreEqual(isolation, tx.Isolation);
+                            Assert.AreEqual(timeout, tx.Timeout);
+
+                            igniteTx = GetSingleLocalTransaction();
+                            Assert.AreEqual(concurrency, igniteTx.Concurrency);
+                            Assert.AreEqual(isolation, igniteTx.Isolation);
+                            Assert.AreEqual(timeout, igniteTx.Timeout);
+                        }
+
+                        igniteTx.Dispose();
+                    }
+                }
+            }
+        }
+
+        /// <summary>
+        /// Tests that parameters passed to TxStart are applied.
+        /// </summary>
+        [Test]
+        public void TestTxStartPassesParameters()
+        {
+            var timeout = TransactionClientConfiguration.DefaultDefaultTimeout.Add(TimeSpan.FromMilliseconds(1000));
+            var acts = new List<Func<ITransactionsClient>>
+            {
+                () => Client.GetTransactions(),
+                () => Client.GetTransactions().WithLabel("label"),
+            };
+            foreach (var concurrency in AllConcurrencyControls)
+            {
+                foreach (var isolation in AllIsolationLevels)
+                {
+                    foreach (var act in acts)
+                    {
+                        var client = act();
+
+                        ITransaction igniteTx;
+                        using (var tx = client.TxStart(concurrency, isolation))
+                        {
+                            Assert.AreEqual(tx, client.Tx);
+                            Assert.AreEqual(concurrency, tx.Concurrency);
+                            Assert.AreEqual(isolation, tx.Isolation);
+
+                            igniteTx = GetSingleLocalTransaction();
+                            Assert.AreEqual(concurrency, igniteTx.Concurrency);
+                            Assert.AreEqual(isolation, igniteTx.Isolation);
+                        }
+
+                        igniteTx.Dispose();
+                        using (var tx = client.TxStart(concurrency, isolation, timeout))
+                        {
+                            Assert.AreEqual(concurrency, tx.Concurrency);
+                            Assert.AreEqual(isolation, tx.Isolation);
+                            Assert.AreEqual(timeout, tx.Timeout);
+
+                            igniteTx = GetSingleLocalTransaction();
+                            Assert.AreEqual(concurrency, igniteTx.Concurrency);
+                            Assert.AreEqual(isolation, igniteTx.Isolation);
+                            Assert.AreEqual(timeout, igniteTx.Timeout);
+                        }
+
+                        igniteTx.Dispose();
+                    }
+                }
+            }
+        }
+
+        /// <summary>
+        /// Tests that transaction can't be committed/rollback after being already completed.
+        /// </summary>
+        [Test]
+        public void TestThrowsIfEndAlreadyCompletedTransaction()
+        {
+            var tx = Client.GetTransactions().TxStart();
+            tx.Commit();
+
+            var constraint = new ReusableConstraint(Is.TypeOf<InvalidOperationException>()
+                .And.Message.Contains("Transaction")
+                .And.Message.Contains("is closed"));
+
+            Assert.Throws(constraint, () => tx.Commit());
+            Assert.Throws(constraint, () => tx.Rollback());
+
+            using (tx = Client.GetTransactions().TxStart())
+            {
+            }
+
+            Assert.Throws(constraint, () => tx.Commit());
+            Assert.Throws(constraint, () => tx.Rollback());
+        }
+
+        /// <summary>
+        /// Tests that transaction throws if timeout elapsed.
+        /// </summary>
+        [Test]
+        public void TestTimeout()
+        {
+            var timeout = TimeSpan.FromMilliseconds(200);
+            var cache = GetTransactionalCache();
+            cache.Put(1, 1);
+            using (var tx = Client.GetTransactions().TxStart(TransactionConcurrency.Pessimistic,
+                TransactionIsolation.ReadCommitted,
+                timeout))
+            {
+                Thread.Sleep(TimeSpan.FromMilliseconds(300));
+                var constraint = new ReusableConstraint(Is.TypeOf<IgniteClientException>()
+                    .And.Message.Contains("Cache transaction timed out"));
+                Assert.Throws(constraint, () => cache.Put(1, 10));
+                Assert.Throws(constraint, () => tx.Commit());
+            }
+
+            Assert.AreEqual(1, cache.Get(1));
+        }
+
+        /// <summary>
+        /// Tests that commit applies cache changes.
+        /// </summary>
+        [Test]
+        public void TestTxCommit()
+        {
+            var cache = GetTransactionalCache();
+
+            cache.Put(1, 1);
+            cache.Put(2, 2);
+
+            using (var tx = Client.GetTransactions().TxStart())
+            {
+                cache.Put(1, 10);
+                cache.Put(2, 20);
+
+                tx.Commit();
+            }
+
+            Assert.AreEqual(10, cache.Get(1));
+            Assert.AreEqual(20, cache.Get(2));
+        }
+
+        /// <summary>
+        /// Tests that rollback reverts cache changes.
+        /// </summary>
+        [Test]
+        public void TestTxRollback()
+        {
+            var cache = GetTransactionalCache();
+            cache.Put(1, 1);
+            cache.Put(2, 2);
+
+            using (var tx = Client.GetTransactions().TxStart())
+            {
+                cache.Put(1, 10);
+                cache.Put(2, 20);
+
+                Assert.AreEqual(10, cache.Get(1));
+                Assert.AreEqual(20, cache.Get(2));
+                tx.Rollback();
+            }
+
+            Assert.AreEqual(1, cache.Get(1));
+            Assert.AreEqual(2, cache.Get(2));
+        }
+
+        /// <summary>
+        /// Tests that closing transaction without commit reverts cache changes.
+        /// </summary>
+        [Test]
+        public void TestTxClose()
+        {
+            var cache = GetTransactionalCache();
+
+            cache.Put(1, 1);
+            cache.Put(2, 2);
+
+            using (Client.GetTransactions().TxStart())
+            {
+                cache.Put(1, 10);
+                cache.Put(2, 20);
+            }
+
+            Assert.AreEqual(1, cache.Get(1));
+            Assert.AreEqual(2, cache.Get(2));
+        }
+
+        /// <summary>
+        /// Tests that client can't start multiple transactions in one thread.
+        /// </summary>
+        [Test]
+        public void TestThrowsIfMultipleStarted()
+        {
+            TestThrowsIfMultipleStarted(
+                () => Client.GetTransactions().TxStart(),
+                () => Client.GetTransactions().TxStart());
+        }
+
+        /// <summary>
+        /// Tests that different clients can start transactions in one thread.
+        /// </summary>
+        [Test]
+        public void TestDifferentClientsCanStartTransactions()
+        {
+            Assert.DoesNotThrow(() =>
+            {
+                using (Client.GetTransactions().TxStart())
+                using (GetClient().GetTransactions().TxStart())
+                {
+                    // No-op.

Review comment:
       Added test.




----------------------------------------------------------------
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.

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