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/21 18:05:12 UTC
[GitHub] [ignite] ptupitsyn commented on a change in pull request #7992: IGNITE-7369 .NET: Thin Client Transactions
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