From 4800554fb0cdab64277af271e4a6bcc48dbb3fca Mon Sep 17 00:00:00 2001 From: Roman Puchkovskiy Date: Fri, 13 Dec 2024 21:30:11 +0400 Subject: [PATCH 01/18] IGNITE-23747 Support timeouts for RO transactions * Timeouts for RO transactions are supported * Data availability time is used as a default RO transaction timeout * After the timeout expires, an RO transaction gets aborted; we rely on existing cleanup mechanism to close remote cursors of this transaction and unlock its LWM locks * RO transaction is guaranteed to not be aborted automatically until its timeout expires, but it might live longer than its timeout * TxManager API has been revamped to make distinction between implicit and explicit transaction initiation more visible --- .../apache/ignite/tx/TransactionOptions.java | 13 +- .../ignite/tx/TransactionOptionsTest.java | 91 ++++++++++ .../client/handler/ClientHandlerModule.java | 8 +- .../handler/ClientInboundMessageHandler.java | 6 + .../handler/JdbcQueryEventHandlerImpl.java | 7 +- .../requests/table/ClientTableCommon.java | 38 ++++- .../tx/ClientTransactionBeginRequest.java | 17 +- .../JdbcQueryEventHandlerImplTest.java | 23 ++- .../client/tx/ClientLazyTransaction.java | 5 +- .../client/tx/ClientTransactions.java | 5 +- .../ignite/client/fakes/FakeTxManager.java | 13 +- .../testframework/CommonTestScheduler.java | 42 +++++ .../rebalance/ItRebalanceDistributedTest.java | 10 +- .../replicator/ItReplicaLifecycleTest.java | 8 +- .../detail/transaction/transactions_impl.h | 1 + .../cpp/ignite/odbc/sql_connection.cpp | 1 + .../dotnet/Apache.Ignite.Tests/FakeServer.cs | 1 + .../Internal/Transactions/LazyTransaction.cs | 1 + .../Transactions/TransactionOptions.cs | 8 +- .../runner/app/ItIgniteNodeRestartTest.java | 8 +- .../ignite/internal/app/IgniteImpl.java | 19 ++- .../internal/sql/sqllogic/ItSqlLogicTest.java | 3 +- .../internal/sql/sqllogic/ScriptContext.java | 8 +- .../tx/QueryTransactionContextImpl.java | 8 +- .../QueryTransactionWrapperSelfTest.java | 9 +- .../exec/rel/TableScanNodeExecutionTest.java | 15 +- .../ItInternalTableReadOnlyScanTest.java | 3 +- .../ItInternalTableReadWriteScanTest.java | 3 +- .../ignite/distributed/ItLockTableTest.java | 16 +- .../ItTxDistributedCleanupRecoveryTest.java | 1 + ...ributedTestSingleNodeNoCleanupMessage.java | 10 +- .../distributed/ItTxStateLocalMapTest.java | 5 + .../internal/table/ItColocationTest.java | 15 +- .../storage/InternalTableImpl.java | 6 +- .../ignite/distributed/ItTxTestCluster.java | 13 +- .../ignite/internal/table/TxAbstractTest.java | 3 +- .../internal/table/TxInfrastructureTest.java | 13 +- .../table/impl/DummyInternalTableImpl.java | 12 +- modules/transactions/build.gradle | 3 + .../ItClientReadOnlyTxTimeoutOneNodeTest.java | 65 ++++++++ ...tEmbeddedReadOnlyTxTimeoutOneNodeTest.java | 35 ++++ .../ItReadOnlyTxAndLowWatermarkTest.java | 11 +- .../ItReadOnlyTxTimeoutOneNodeTest.java | 70 ++++++++ .../ignite/internal/tx/InternalTxOptions.java | 79 +++++++++ .../apache/ignite/internal/tx/TxManager.java | 42 ++--- .../tx/impl/IgniteTransactionsImpl.java | 26 ++- .../impl/TransactionExpirationRegistry.java | 124 ++++++++++++++ .../internal/tx/impl/TxManagerImpl.java | 111 +++++++++++-- .../ignite/internal/tx/TxManagerTest.java | 83 ++++++---- .../TransactionExpirationRegistryTest.java | 155 ++++++++++++++++++ 50 files changed, 1117 insertions(+), 155 deletions(-) create mode 100644 modules/api/src/test/java/org/apache/ignite/tx/TransactionOptionsTest.java create mode 100644 modules/core/src/testFixtures/java/org/apache/ignite/internal/testframework/CommonTestScheduler.java create mode 100644 modules/transactions/src/integrationTest/java/org/apache/ignite/internal/tx/readonly/ItClientReadOnlyTxTimeoutOneNodeTest.java create mode 100644 modules/transactions/src/integrationTest/java/org/apache/ignite/internal/tx/readonly/ItEmbeddedReadOnlyTxTimeoutOneNodeTest.java create mode 100644 modules/transactions/src/integrationTest/java/org/apache/ignite/internal/tx/readonly/ItReadOnlyTxTimeoutOneNodeTest.java create mode 100644 modules/transactions/src/main/java/org/apache/ignite/internal/tx/InternalTxOptions.java create mode 100644 modules/transactions/src/main/java/org/apache/ignite/internal/tx/impl/TransactionExpirationRegistry.java create mode 100644 modules/transactions/src/test/java/org/apache/ignite/internal/tx/impl/TransactionExpirationRegistryTest.java diff --git a/modules/api/src/main/java/org/apache/ignite/tx/TransactionOptions.java b/modules/api/src/main/java/org/apache/ignite/tx/TransactionOptions.java index 2b785ccd764..1404822e0e3 100644 --- a/modules/api/src/main/java/org/apache/ignite/tx/TransactionOptions.java +++ b/modules/api/src/main/java/org/apache/ignite/tx/TransactionOptions.java @@ -21,14 +21,14 @@ * Ignite transaction options. */ public class TransactionOptions { - /** Transaction timeout. */ + /** Transaction timeout. 0 means 'use default timeout'. */ private long timeoutMillis = 0; /** Read-only transaction. */ private boolean readOnly = false; /** - * Returns transaction timeout, in milliseconds. + * Returns transaction timeout, in milliseconds. 0 means 'use default timeout'. * * @return Transaction timeout, in milliseconds. */ @@ -39,10 +39,17 @@ public long timeoutMillis() { /** * Sets transaction timeout, in milliseconds. * - * @param timeoutMillis Transaction timeout, in milliseconds. + * @param timeoutMillis Transaction timeout, in milliseconds. Cannot be negative; 0 means 'use default timeout'. + * For RO transactions, the default timeout is data availability time configured via ignite.gc.lowWatermark.dataAvailabilityTime + * configuration setting. + * For RW transactions, timeouts are not supported yet. TODO: IGNITE-15936 * @return {@code this} for chaining. */ public TransactionOptions timeoutMillis(long timeoutMillis) { + if (timeoutMillis < 0) { + throw new IllegalArgumentException("Negative timeoutMillis: " + timeoutMillis); + } + this.timeoutMillis = timeoutMillis; return this; diff --git a/modules/api/src/test/java/org/apache/ignite/tx/TransactionOptionsTest.java b/modules/api/src/test/java/org/apache/ignite/tx/TransactionOptionsTest.java new file mode 100644 index 00000000000..3f305415329 --- /dev/null +++ b/modules/api/src/test/java/org/apache/ignite/tx/TransactionOptionsTest.java @@ -0,0 +1,91 @@ +/* + * 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. + */ + +package org.apache.ignite.tx; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.is; +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; +import static org.junit.jupiter.api.Assertions.assertSame; +import static org.junit.jupiter.api.Assertions.assertThrows; + +import org.junit.jupiter.api.Test; + +class TransactionOptionsTest { + @Test + void readOnlyIsFalseByDefault() { + assertThat(new TransactionOptions().readOnly(), is(false)); + } + + @Test + void readOnlyStatusIsSet() { + var options = new TransactionOptions(); + + options.readOnly(true); + + assertThat(options.readOnly(), is(true)); + } + + @Test + void readOnlySetterReturnsSameObject() { + var options = new TransactionOptions(); + + TransactionOptions afterSetting = options.readOnly(true); + + assertSame(options, afterSetting); + } + + @Test + void timeoutIsZeroByDefault() { + assertThat(new TransactionOptions().timeoutMillis(), is(0L)); + } + + @Test + void timeoutIsSet() { + var options = new TransactionOptions(); + + options.timeoutMillis(3333); + + assertThat(options.timeoutMillis(), is(3333L)); + } + + @Test + void timeoutSetterReturnsSameObject() { + var options = new TransactionOptions(); + + TransactionOptions afterSetting = options.timeoutMillis(3333); + + assertSame(options, afterSetting); + } + + @Test + void positiveTimeoutIsAllowed() { + assertDoesNotThrow(() -> new TransactionOptions().timeoutMillis(0)); + } + + @Test + void zeroTimeoutIsAllowed() { + assertDoesNotThrow(() -> new TransactionOptions().timeoutMillis(0)); + } + + @Test + void negativeTimeoutIsRejected() { + IllegalArgumentException ex = assertThrows(IllegalArgumentException.class, () -> new TransactionOptions().timeoutMillis(-1)); + + assertThat(ex.getMessage(), is("Negative timeoutMillis: -1")); + } +} diff --git a/modules/client-handler/src/main/java/org/apache/ignite/client/handler/ClientHandlerModule.java b/modules/client-handler/src/main/java/org/apache/ignite/client/handler/ClientHandlerModule.java index 18ec268eb94..6b3c71c5123 100644 --- a/modules/client-handler/src/main/java/org/apache/ignite/client/handler/ClientHandlerModule.java +++ b/modules/client-handler/src/main/java/org/apache/ignite/client/handler/ClientHandlerModule.java @@ -27,7 +27,6 @@ import io.netty.channel.Channel; import io.netty.channel.ChannelFuture; import io.netty.channel.ChannelFutureListener; -import io.netty.channel.ChannelHandler; import io.netty.channel.ChannelInitializer; import io.netty.channel.ChannelOption; import io.netty.handler.ssl.SslContext; @@ -130,7 +129,7 @@ public class ClientHandlerModule implements IgniteComponent { @TestOnly @SuppressWarnings("unused") - private volatile ChannelHandler handler; + private volatile ClientInboundMessageHandler handler; /** * Constructor. @@ -396,4 +395,9 @@ private ClientInboundMessageHandler createInboundMessageHandler(ClientConnectorV partitionOperationsExecutor ); } + + @TestOnly + public ClientInboundMessageHandler handler() { + return handler; + } } diff --git a/modules/client-handler/src/main/java/org/apache/ignite/client/handler/ClientInboundMessageHandler.java b/modules/client-handler/src/main/java/org/apache/ignite/client/handler/ClientInboundMessageHandler.java index c5dc979615f..c7e83e5580b 100644 --- a/modules/client-handler/src/main/java/org/apache/ignite/client/handler/ClientInboundMessageHandler.java +++ b/modules/client-handler/src/main/java/org/apache/ignite/client/handler/ClientInboundMessageHandler.java @@ -139,6 +139,7 @@ import org.apache.ignite.security.exception.UnsupportedAuthenticationTypeException; import org.apache.ignite.sql.SqlBatchException; import org.jetbrains.annotations.Nullable; +import org.jetbrains.annotations.TestOnly; /** * Handles messages from thin clients. @@ -1139,4 +1140,9 @@ private static Set authenticationEventsToSubscribe() { AuthenticationEvent.USER_REMOVED ); } + + @TestOnly + public ClientResourceRegistry resources() { + return resources; + } } diff --git a/modules/client-handler/src/main/java/org/apache/ignite/client/handler/JdbcQueryEventHandlerImpl.java b/modules/client-handler/src/main/java/org/apache/ignite/client/handler/JdbcQueryEventHandlerImpl.java index 1dde250be81..9b463f90173 100644 --- a/modules/client-handler/src/main/java/org/apache/ignite/client/handler/JdbcQueryEventHandlerImpl.java +++ b/modules/client-handler/src/main/java/org/apache/ignite/client/handler/JdbcQueryEventHandlerImpl.java @@ -67,6 +67,7 @@ import org.apache.ignite.internal.sql.engine.property.SqlPropertiesHelper; import org.apache.ignite.internal.tx.HybridTimestampTracker; import org.apache.ignite.internal.tx.InternalTransaction; +import org.apache.ignite.internal.tx.InternalTxOptions; import org.apache.ignite.internal.tx.TxManager; import org.apache.ignite.internal.util.AsyncCursor.BatchedResult; import org.apache.ignite.lang.CancelHandle; @@ -189,8 +190,8 @@ public HybridTimestampTracker getTimestampTracker() { } private static SqlProperties createProperties( - JdbcStatementType stmtType, - boolean multiStatement, + JdbcStatementType stmtType, + boolean multiStatement, ZoneId timeZoneId, long queryTimeoutMillis ) { @@ -455,7 +456,7 @@ ZoneId timeZoneId() { * @return Transaction associated with the current connection. */ InternalTransaction getOrStartTransaction(HybridTimestampTracker timestampProvider) { - return tx == null ? tx = txManager.begin(timestampProvider, false) : tx; + return tx == null ? tx = txManager.beginExplicitRw(timestampProvider, InternalTxOptions.defaults()) : tx; } /** diff --git a/modules/client-handler/src/main/java/org/apache/ignite/client/handler/requests/table/ClientTableCommon.java b/modules/client-handler/src/main/java/org/apache/ignite/client/handler/requests/table/ClientTableCommon.java index fdc1aa3b71c..e1d156a7d2d 100644 --- a/modules/client-handler/src/main/java/org/apache/ignite/client/handler/requests/table/ClientTableCommon.java +++ b/modules/client-handler/src/main/java/org/apache/ignite/client/handler/requests/table/ClientTableCommon.java @@ -44,6 +44,7 @@ import org.apache.ignite.internal.table.TableViewInternal; import org.apache.ignite.internal.tx.HybridTimestampTracker; import org.apache.ignite.internal.tx.InternalTransaction; +import org.apache.ignite.internal.tx.InternalTxOptions; import org.apache.ignite.internal.tx.TxManager; import org.apache.ignite.internal.type.DecimalNativeType; import org.apache.ignite.internal.type.NativeType; @@ -439,32 +440,53 @@ public static TableNotFoundException tableIdNotFoundException(Integer tableId) { if (tx == null) { // Implicit transactions do not use an observation timestamp because RW never depends on it, and implicit RO is always direct. // The direct transaction uses a current timestamp on the primary replica by definition. - tx = startTx(out, txManager, null, true, readOnly); + tx = startImplicitTx(out, txManager, null, readOnly); } return tx; } /** - * Start a transaction. + * Starts an explicit transaction. * * @param out Packer. * @param txManager Ignite transactions. * @param currentTs Current observation timestamp or {@code null} if it is not defined. - * @param implicit Implicit transaction flag. * @param readOnly Read only flag. + * @param options Transaction options. * @return Transaction. */ - public static InternalTransaction startTx( + public static InternalTransaction startExplicitTx( + ClientMessagePacker out, + TxManager txManager, + @Nullable HybridTimestamp currentTs, + boolean readOnly, + InternalTxOptions options + ) { + return txManager.beginExplicit( + HybridTimestampTracker.clientTracker(currentTs, ts -> {}), + readOnly, + options + ); + } + + /** + * Starts an implicit transaction. + * + * @param out Packer. + * @param txManager Ignite transactions. + * @param currentTs Current observation timestamp or {@code null} if it is not defined. + * @param readOnly Read only flag. + * @return Transaction. + */ + public static InternalTransaction startImplicitTx( ClientMessagePacker out, TxManager txManager, @Nullable HybridTimestamp currentTs, - boolean implicit, boolean readOnly ) { - return txManager.begin( - HybridTimestampTracker.clientTracker(currentTs, implicit ? out::meta : ts -> {}), - implicit, + return txManager.beginImplicit( + HybridTimestampTracker.clientTracker(currentTs, out::meta), readOnly ); } diff --git a/modules/client-handler/src/main/java/org/apache/ignite/client/handler/requests/tx/ClientTransactionBeginRequest.java b/modules/client-handler/src/main/java/org/apache/ignite/client/handler/requests/tx/ClientTransactionBeginRequest.java index 5237f6bf91e..6f804045b05 100644 --- a/modules/client-handler/src/main/java/org/apache/ignite/client/handler/requests/tx/ClientTransactionBeginRequest.java +++ b/modules/client-handler/src/main/java/org/apache/ignite/client/handler/requests/tx/ClientTransactionBeginRequest.java @@ -17,7 +17,7 @@ package org.apache.ignite.client.handler.requests.tx; -import static org.apache.ignite.client.handler.requests.table.ClientTableCommon.startTx; +import static org.apache.ignite.client.handler.requests.table.ClientTableCommon.startExplicitTx; import java.util.concurrent.CompletableFuture; import org.apache.ignite.client.handler.ClientHandlerMetricSource; @@ -27,6 +27,7 @@ import org.apache.ignite.internal.client.proto.ClientMessageUnpacker; import org.apache.ignite.internal.hlc.HybridTimestamp; import org.apache.ignite.internal.lang.IgniteInternalCheckedException; +import org.apache.ignite.internal.tx.InternalTxOptions; import org.apache.ignite.internal.tx.TxManager; import org.jetbrains.annotations.Nullable; @@ -52,12 +53,20 @@ public class ClientTransactionBeginRequest { ClientHandlerMetricSource metrics ) throws IgniteInternalCheckedException { boolean readOnly = in.unpackBoolean(); + long timeoutMillis = in.unpackLong(); - // Timestamp makes sense only for read-only transactions. - HybridTimestamp observableTs = readOnly ? HybridTimestamp.nullableHybridTimestamp(in.unpackLong()) : null; + HybridTimestamp observableTs = null; + if (readOnly) { + // Timestamp makes sense only for read-only transactions. + observableTs = HybridTimestamp.nullableHybridTimestamp(in.unpackLong()); + } + + InternalTxOptions txOptions = InternalTxOptions.builder() + .timeoutMillis(timeoutMillis) + .build(); // NOTE: we don't use beginAsync here because it is synchronous anyway. - var tx = startTx(out, txManager, observableTs, false, readOnly); + var tx = startExplicitTx(out, txManager, observableTs, readOnly, txOptions); if (readOnly) { // For read-only tx, override observable timestamp that we send to the client: diff --git a/modules/client-handler/src/test/java/org/apache/ignite/client/handler/JdbcQueryEventHandlerImplTest.java b/modules/client-handler/src/test/java/org/apache/ignite/client/handler/JdbcQueryEventHandlerImplTest.java index 69f7e6df60a..4df09296b48 100644 --- a/modules/client-handler/src/test/java/org/apache/ignite/client/handler/JdbcQueryEventHandlerImplTest.java +++ b/modules/client-handler/src/test/java/org/apache/ignite/client/handler/JdbcQueryEventHandlerImplTest.java @@ -29,7 +29,6 @@ import static org.hamcrest.Matchers.notNullValue; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.mockito.ArgumentMatchers.any; -import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; @@ -133,7 +132,7 @@ public void contextClosedDuringBatchQuery() throws Exception { CountDownLatch registryCloseLatch = new CountDownLatch(1); long connectionId = acquireConnectionId(); - when(txManager.begin(any(), eq(false))).thenAnswer(v -> { + when(txManager.beginExplicitRw(any(), any())).thenAnswer(v -> { registryCloseLatch.countDown(); assertThat(startTxLatch.await(timeout, TimeUnit.SECONDS), is(true)); @@ -161,13 +160,13 @@ public void explicitTxRollbackOnCloseRegistry() { InternalTransaction tx = mock(InternalTransaction.class); when(tx.rollbackAsync()).thenReturn(nullCompletedFuture()); - when(txManager.begin(any(), eq(false))).thenReturn(tx); + when(txManager.beginExplicitRw(any(), any())).thenReturn(tx); long connectionId = acquireConnectionId(); await(eventHandler.batchAsync(connectionId, createExecuteBatchRequest("x", "UPDATE 1"))); - verify(txManager).begin(any(), eq(false)); + verify(txManager).beginExplicitRw(any(), any()); verify(tx, times(0)).rollbackAsync(); resourceRegistry.close(); @@ -183,10 +182,10 @@ public void singleTxUsedForMultipleOperations() { InternalTransaction tx = mock(InternalTransaction.class); when(tx.commitAsync()).thenReturn(nullCompletedFuture()); when(tx.rollbackAsync()).thenReturn(nullCompletedFuture()); - when(txManager.begin(any(), eq(false))).thenReturn(tx); + when(txManager.beginExplicitRw(any(), any())).thenReturn(tx); long connectionId = acquireConnectionId(); - verify(txManager, times(0)).begin(any(), eq(false)); + verify(txManager, times(0)).beginExplicitRw(any(), any()); String schema = "schema"; JdbcStatementType type = JdbcStatementType.SELECT_STATEMENT_TYPE; @@ -194,21 +193,21 @@ public void singleTxUsedForMultipleOperations() { await(eventHandler.queryAsync( connectionId, createExecuteRequest(schema, "SELECT 1", type) )); - verify(txManager, times(1)).begin(any(), eq(false)); + verify(txManager, times(1)).beginExplicitRw(any(), any()); await(eventHandler.batchAsync(connectionId, createExecuteBatchRequest("schema", "UPDATE 1", "UPDATE 2"))); - verify(txManager, times(1)).begin(any(), eq(false)); + verify(txManager, times(1)).beginExplicitRw(any(), any()); await(eventHandler.finishTxAsync(connectionId, false)); verify(tx).rollbackAsync(); await(eventHandler.batchAsync(connectionId, createExecuteBatchRequest("schema", "UPDATE 1", "UPDATE 2"))); - verify(txManager, times(2)).begin(any(), eq(false)); + verify(txManager, times(2)).beginExplicitRw(any(), any()); await(eventHandler.queryAsync( connectionId, createExecuteRequest(schema, "SELECT 2", type) )); - verify(txManager, times(2)).begin(any(), eq(false)); + verify(txManager, times(2)).beginExplicitRw(any(), any()); await(eventHandler.batchAsync(connectionId, createExecuteBatchRequest("schema", "UPDATE 3", "UPDATE 4"))); - verify(txManager, times(2)).begin(any(), eq(false)); + verify(txManager, times(2)).beginExplicitRw(any(), any()); await(eventHandler.finishTxAsync(connectionId, true)); verify(tx).commitAsync(); @@ -223,7 +222,7 @@ void simpleQueryCancellation() { long connectionId = acquireConnectionId(); - JdbcQueryExecuteRequest executeRequest = createExecuteRequest("schema", "SELECT 1", JdbcStatementType.SELECT_STATEMENT_TYPE); + JdbcQueryExecuteRequest executeRequest = createExecuteRequest("schema", "SELECT 1", JdbcStatementType.SELECT_STATEMENT_TYPE); CompletableFuture resultFuture = eventHandler.queryAsync(connectionId, executeRequest); diff --git a/modules/client/src/main/java/org/apache/ignite/internal/client/tx/ClientLazyTransaction.java b/modules/client/src/main/java/org/apache/ignite/internal/client/tx/ClientLazyTransaction.java index 133ec00f7b2..57530d6a365 100644 --- a/modules/client/src/main/java/org/apache/ignite/internal/client/tx/ClientLazyTransaction.java +++ b/modules/client/src/main/java/org/apache/ignite/internal/client/tx/ClientLazyTransaction.java @@ -165,7 +165,10 @@ private synchronized CompletableFuture ensureStarted( return tx0; } - ClientTransaction startedTx() { + /** + * Returns actual {@link ClientTransaction} started by this transaction or throws an exception if no transaction was started yet. + */ + public ClientTransaction startedTx() { var tx0 = tx; assert tx0 != null : "Transaction is not started"; diff --git a/modules/client/src/main/java/org/apache/ignite/internal/client/tx/ClientTransactions.java b/modules/client/src/main/java/org/apache/ignite/internal/client/tx/ClientTransactions.java index 325ed680a7c..ece4c13b584 100644 --- a/modules/client/src/main/java/org/apache/ignite/internal/client/tx/ClientTransactions.java +++ b/modules/client/src/main/java/org/apache/ignite/internal/client/tx/ClientTransactions.java @@ -62,9 +62,9 @@ static CompletableFuture beginAsync( @Nullable String preferredNodeName, @Nullable TransactionOptions options, long observableTimestamp) { - if (options != null && options.timeoutMillis() != 0) { + if (options != null && options.timeoutMillis() != 0 && !options.readOnly()) { // TODO: IGNITE-16193 - throw new UnsupportedOperationException("Timeouts are not supported yet"); + throw new UnsupportedOperationException("Timeouts are not supported yet for RW transactions"); } boolean readOnly = options != null && options.readOnly(); @@ -73,6 +73,7 @@ static CompletableFuture beginAsync( ClientOp.TX_BEGIN, w -> { w.out().packBoolean(readOnly); + w.out().packLong(options == null ? 0 : options.timeoutMillis()); w.out().packLong(observableTimestamp); }, r -> readTx(r, readOnly), diff --git a/modules/client/src/test/java/org/apache/ignite/client/fakes/FakeTxManager.java b/modules/client/src/test/java/org/apache/ignite/client/fakes/FakeTxManager.java index eb5dbeb74e7..8cefbecb722 100644 --- a/modules/client/src/test/java/org/apache/ignite/client/fakes/FakeTxManager.java +++ b/modules/client/src/test/java/org/apache/ignite/client/fakes/FakeTxManager.java @@ -31,9 +31,9 @@ import org.apache.ignite.internal.replicator.TablePartitionId; import org.apache.ignite.internal.tx.HybridTimestampTracker; import org.apache.ignite.internal.tx.InternalTransaction; +import org.apache.ignite.internal.tx.InternalTxOptions; import org.apache.ignite.internal.tx.LockManager; import org.apache.ignite.internal.tx.TxManager; -import org.apache.ignite.internal.tx.TxPriority; import org.apache.ignite.internal.tx.TxState; import org.apache.ignite.internal.tx.TxStateMeta; import org.apache.ignite.network.ClusterNode; @@ -63,17 +63,16 @@ public CompletableFuture stopAsync(ComponentContext componentContext) { } @Override - public InternalTransaction begin(HybridTimestampTracker tracker, boolean implicit) { - return begin(tracker, implicit, false); + public InternalTransaction beginImplicit(HybridTimestampTracker timestampTracker, boolean readOnly) { + return begin(timestampTracker, true, readOnly, InternalTxOptions.defaults()); } @Override - public InternalTransaction begin(HybridTimestampTracker timestampTracker, boolean implicit, boolean readOnly) { - return begin(timestampTracker, implicit, readOnly, TxPriority.NORMAL); + public InternalTransaction beginExplicit(HybridTimestampTracker timestampTracker, boolean readOnly, InternalTxOptions txOptions) { + return begin(timestampTracker, false, readOnly, txOptions); } - @Override - public InternalTransaction begin(HybridTimestampTracker tracker, boolean implicit, boolean readOnly, TxPriority priority) { + private InternalTransaction begin(HybridTimestampTracker tracker, boolean implicit, boolean readOnly, InternalTxOptions options) { return new InternalTransaction() { private final UUID id = UUID.randomUUID(); diff --git a/modules/core/src/testFixtures/java/org/apache/ignite/internal/testframework/CommonTestScheduler.java b/modules/core/src/testFixtures/java/org/apache/ignite/internal/testframework/CommonTestScheduler.java new file mode 100644 index 00000000000..a65067cc1f3 --- /dev/null +++ b/modules/core/src/testFixtures/java/org/apache/ignite/internal/testframework/CommonTestScheduler.java @@ -0,0 +1,42 @@ +/* + * 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. + */ + +package org.apache.ignite.internal.testframework; + +import java.util.concurrent.Executors; +import java.util.concurrent.ScheduledExecutorService; +import org.apache.ignite.internal.logger.IgniteLogger; +import org.apache.ignite.internal.logger.Loggers; +import org.apache.ignite.internal.thread.NamedThreadFactory; + +/** + * Provides access to a single-thread scheduler for light-weight non-blocking operations. It doesn't need to be stopped. + */ +public class CommonTestScheduler { + private static final IgniteLogger LOG = Loggers.forClass(CommonTestScheduler.class); + + private static final ScheduledExecutorService INSTANCE = Executors.newSingleThreadScheduledExecutor( + new NamedThreadFactory("test-common-scheduler-", true, LOG) + ); + + /** + * Returns a single-thread scheduler for light-weight non-blocking operations. It doesn't need to be shut down. + */ + public static ScheduledExecutorService instance() { + return INSTANCE; + } +} diff --git a/modules/distribution-zones/src/integrationTest/java/org/apache/ignite/internal/rebalance/ItRebalanceDistributedTest.java b/modules/distribution-zones/src/integrationTest/java/org/apache/ignite/internal/rebalance/ItRebalanceDistributedTest.java index 5745dfd4215..c27368ba4bd 100644 --- a/modules/distribution-zones/src/integrationTest/java/org/apache/ignite/internal/rebalance/ItRebalanceDistributedTest.java +++ b/modules/distribution-zones/src/integrationTest/java/org/apache/ignite/internal/rebalance/ItRebalanceDistributedTest.java @@ -203,6 +203,7 @@ import org.apache.ignite.internal.schema.configuration.GcConfiguration; import org.apache.ignite.internal.schema.configuration.GcExtensionConfiguration; import org.apache.ignite.internal.schema.configuration.GcExtensionConfigurationSchema; +import org.apache.ignite.internal.schema.configuration.LowWatermarkConfiguration; import org.apache.ignite.internal.schema.configuration.StorageUpdateConfiguration; import org.apache.ignite.internal.schema.configuration.StorageUpdateExtensionConfiguration; import org.apache.ignite.internal.schema.configuration.StorageUpdateExtensionConfigurationSchema; @@ -299,6 +300,9 @@ public class ItRebalanceDistributedTest extends BaseIgniteAbstractTest { @InjectConfiguration private TransactionConfiguration txConfiguration; + @InjectConfiguration + private LowWatermarkConfiguration lowWatermarkConfiguration; + @InjectConfiguration private RaftConfiguration raftConfiguration; @@ -331,7 +335,7 @@ public class ItRebalanceDistributedTest extends BaseIgniteAbstractTest { private Path workDir; @InjectExecutorService - private static ScheduledExecutorService commonScheduledExecutorService; + private ScheduledExecutorService commonScheduledExecutorService; private StaticNodeFinder finder; @@ -1401,6 +1405,7 @@ private class Node { txManager = new TxManagerImpl( txConfiguration, + lowWatermarkConfiguration, clusterService, replicaSvc, lockManager, @@ -1411,7 +1416,8 @@ private class Node { new TestLocalRwTxCounter(), resourcesRegistry, transactionInflights, - lowWatermark + lowWatermark, + commonScheduledExecutorService ); rebalanceScheduler = new ScheduledThreadPoolExecutor(REBALANCE_SCHEDULER_POOL_SIZE, diff --git a/modules/partition-replicator/src/integrationTest/java/org/apache/ignite/internal/partition/replicator/ItReplicaLifecycleTest.java b/modules/partition-replicator/src/integrationTest/java/org/apache/ignite/internal/partition/replicator/ItReplicaLifecycleTest.java index 6280a5e0f6c..65a02b42899 100644 --- a/modules/partition-replicator/src/integrationTest/java/org/apache/ignite/internal/partition/replicator/ItReplicaLifecycleTest.java +++ b/modules/partition-replicator/src/integrationTest/java/org/apache/ignite/internal/partition/replicator/ItReplicaLifecycleTest.java @@ -170,6 +170,7 @@ import org.apache.ignite.internal.schema.configuration.GcConfiguration; import org.apache.ignite.internal.schema.configuration.GcExtensionConfiguration; import org.apache.ignite.internal.schema.configuration.GcExtensionConfigurationSchema; +import org.apache.ignite.internal.schema.configuration.LowWatermarkConfiguration; import org.apache.ignite.internal.schema.configuration.StorageUpdateConfiguration; import org.apache.ignite.internal.schema.configuration.StorageUpdateExtensionConfiguration; import org.apache.ignite.internal.schema.configuration.StorageUpdateExtensionConfigurationSchema; @@ -250,6 +251,9 @@ public class ItReplicaLifecycleTest extends BaseIgniteAbstractTest { @InjectConfiguration private static TransactionConfiguration txConfiguration; + @InjectConfiguration + private static LowWatermarkConfiguration lowWatermarkConfiguration; + @InjectConfiguration private static RaftConfiguration raftConfiguration; @@ -1227,6 +1231,7 @@ public CompletableFuture invoke( txManager = new TxManagerImpl( txConfiguration, + lowWatermarkConfiguration, clusterService, replicaSvc, lockManager, @@ -1237,7 +1242,8 @@ public CompletableFuture invoke( new TestLocalRwTxCounter(), resourcesRegistry, transactionInflights, - lowWatermark + lowWatermark, + threadPoolsManager.commonScheduler() ); replicaManager = new ReplicaManager( diff --git a/modules/platforms/cpp/ignite/client/detail/transaction/transactions_impl.h b/modules/platforms/cpp/ignite/client/detail/transaction/transactions_impl.h index cbaedbb0597..7c00c610a5c 100644 --- a/modules/platforms/cpp/ignite/client/detail/transaction/transactions_impl.h +++ b/modules/platforms/cpp/ignite/client/detail/transaction/transactions_impl.h @@ -57,6 +57,7 @@ class transactions_impl { IGNITE_API void begin_async(ignite_callback callback) { auto writer_func = [this](protocol::writer &writer) { writer.write_bool(false); // readOnly. + writer.write(std::int64_t(0)); // timeoutMillis. writer.write(m_connection->get_observable_timestamp()); }; diff --git a/modules/platforms/cpp/ignite/odbc/sql_connection.cpp b/modules/platforms/cpp/ignite/odbc/sql_connection.cpp index d043b3b536a..ea707d48799 100644 --- a/modules/platforms/cpp/ignite/odbc/sql_connection.cpp +++ b/modules/platforms/cpp/ignite/odbc/sql_connection.cpp @@ -433,6 +433,7 @@ void sql_connection::transaction_start() { network::data_buffer_owning response = sync_request(protocol::client_operation::TX_BEGIN, [&](protocol::writer &writer) { writer.write_bool(false); // read_only. + writer.write(std::int64_t(0)); // timeoutMillis. }); protocol::reader reader(response.get_bytes_view()); diff --git a/modules/platforms/dotnet/Apache.Ignite.Tests/FakeServer.cs b/modules/platforms/dotnet/Apache.Ignite.Tests/FakeServer.cs index c0bdd678ac2..550e178434a 100644 --- a/modules/platforms/dotnet/Apache.Ignite.Tests/FakeServer.cs +++ b/modules/platforms/dotnet/Apache.Ignite.Tests/FakeServer.cs @@ -309,6 +309,7 @@ protected override void Handle(Socket handler, CancellationToken cancellationTok case ClientOp.TxBegin: reader.Skip(); // Read only. + reader.Skip(); // TimeoutMillis. LastClientObservableTimestamp = reader.ReadInt64(); Send(handler, requestId, new byte[] { 0 }.AsMemory()); diff --git a/modules/platforms/dotnet/Apache.Ignite/Internal/Transactions/LazyTransaction.cs b/modules/platforms/dotnet/Apache.Ignite/Internal/Transactions/LazyTransaction.cs index c2976cf5fa5..933ad33b336 100644 --- a/modules/platforms/dotnet/Apache.Ignite/Internal/Transactions/LazyTransaction.cs +++ b/modules/platforms/dotnet/Apache.Ignite/Internal/Transactions/LazyTransaction.cs @@ -197,6 +197,7 @@ void Write() { var w = writer.MessageWriter; w.Write(_options.ReadOnly); + w.Write(_options.TimeoutMillis); w.Write(failoverSocket.ObservableTimestamp); } } diff --git a/modules/platforms/dotnet/Apache.Ignite/Transactions/TransactionOptions.cs b/modules/platforms/dotnet/Apache.Ignite/Transactions/TransactionOptions.cs index acc005bdccc..8a94718c2a4 100644 --- a/modules/platforms/dotnet/Apache.Ignite/Transactions/TransactionOptions.cs +++ b/modules/platforms/dotnet/Apache.Ignite/Transactions/TransactionOptions.cs @@ -25,4 +25,10 @@ namespace Apache.Ignite.Transactions; /// Read-only transactions provide a snapshot view of data at a certain point in time. /// They are lock-free and perform better than normal transactions, but do not permit data modifications. /// -public readonly record struct TransactionOptions(bool ReadOnly); +/// +/// Transaction timeout. 0 means 'use default timeout'. +/// For RO transactions, the default timeout is data availability time configured via ignite.gc.lowWatermark.dataAvailabilityTime +/// configuration setting. +/// For RW transactions, timeouts are not supported yet. TODO: IGNITE-15936. +/// +public readonly record struct TransactionOptions(bool ReadOnly, long TimeoutMillis = 0); diff --git a/modules/runner/src/integrationTest/java/org/apache/ignite/internal/runner/app/ItIgniteNodeRestartTest.java b/modules/runner/src/integrationTest/java/org/apache/ignite/internal/runner/app/ItIgniteNodeRestartTest.java index 3e86b870d6a..4aac956529b 100644 --- a/modules/runner/src/integrationTest/java/org/apache/ignite/internal/runner/app/ItIgniteNodeRestartTest.java +++ b/modules/runner/src/integrationTest/java/org/apache/ignite/internal/runner/app/ItIgniteNodeRestartTest.java @@ -189,6 +189,7 @@ import org.apache.ignite.internal.schema.SchemaManager; import org.apache.ignite.internal.schema.configuration.GcConfiguration; import org.apache.ignite.internal.schema.configuration.GcExtensionConfiguration; +import org.apache.ignite.internal.schema.configuration.LowWatermarkConfiguration; import org.apache.ignite.internal.schema.configuration.StorageUpdateConfiguration; import org.apache.ignite.internal.sql.api.IgniteSqlImpl; import org.apache.ignite.internal.sql.configuration.distributed.SqlClusterExtensionConfiguration; @@ -295,6 +296,9 @@ public class ItIgniteNodeRestartTest extends BaseIgniteRestartTest { @InjectConfiguration private static TransactionConfiguration txConfiguration; + @InjectConfiguration + private static LowWatermarkConfiguration lowWatermarkConfiguration; + @InjectConfiguration private static StorageUpdateConfiguration storageUpdateConfiguration; @@ -619,6 +623,7 @@ public CompletableFuture invoke(Condition condition, List su var txManager = new TxManagerImpl( name, txConfiguration, + lowWatermarkConfiguration, messagingServiceReturningToStorageOperationsPool, clusterSvc.topologyService(), replicaService, @@ -631,7 +636,8 @@ public CompletableFuture invoke(Condition condition, List su threadPoolsManager.partitionOperationsExecutor(), resourcesRegistry, transactionInflights, - lowWatermark + lowWatermark, + threadPoolsManager.commonScheduler() ); ResourceVacuumManager resourceVacuumManager = new ResourceVacuumManager( diff --git a/modules/runner/src/main/java/org/apache/ignite/internal/app/IgniteImpl.java b/modules/runner/src/main/java/org/apache/ignite/internal/app/IgniteImpl.java index c109175b9d2..9ea0d075490 100644 --- a/modules/runner/src/main/java/org/apache/ignite/internal/app/IgniteImpl.java +++ b/modules/runner/src/main/java/org/apache/ignite/internal/app/IgniteImpl.java @@ -60,6 +60,7 @@ import org.apache.ignite.catalog.IgniteCatalog; import org.apache.ignite.client.handler.ClientHandlerMetricSource; import org.apache.ignite.client.handler.ClientHandlerModule; +import org.apache.ignite.client.handler.ClientInboundMessageHandler; import org.apache.ignite.client.handler.ClusterInfo; import org.apache.ignite.client.handler.configuration.ClientConnectorConfiguration; import org.apache.ignite.client.handler.configuration.ClientConnectorExtensionConfiguration; @@ -358,7 +359,7 @@ public class IgniteImpl implements Ignite { private final ReplicaManager replicaMgr; /** Transactions manager. */ - private final TxManager txManager; + private final TxManagerImpl txManager; /** Distributed table manager. */ private final TableManager distributedTblMgr; @@ -977,6 +978,7 @@ public class IgniteImpl implements Ignite { txManager = new TxManagerImpl( name, txConfig, + gcConfig.lowWatermark(), messagingServiceReturningToStorageOperationsPool, clusterSvc.topologyService(), replicaSvc, @@ -989,10 +991,11 @@ public class IgniteImpl implements Ignite { threadPoolsManager.partitionOperationsExecutor(), resourcesRegistry, transactionInflights, - lowWatermark + lowWatermark, + threadPoolsManager.commonScheduler() ); - systemViewManager.register((TxManagerImpl) txManager); + systemViewManager.register(txManager); resourceVacuumManager = new ResourceVacuumManager( name, @@ -1878,17 +1881,17 @@ public LogStorageFactory partitionsLogStorageFactory() { return partitionsLogStorageFactory; } - @TestOnly - public LogStorageFactory volatileLogStorageFactory() { - return volatileLogStorageFactoryCreator.factory(raftMgr.volatileRaft().logStorageBudget().value()); - } - /** Returns the node's transaction manager. */ @TestOnly public TxManager txManager() { return txManager; } + @TestOnly + public ClientInboundMessageHandler clientInboundMessageHandler() { + return clientHandlerModule.handler(); + } + /** Returns the node's placement driver service. */ @TestOnly public PlacementDriver placementDriver() { diff --git a/modules/sql-engine/src/integrationTest/java/org/apache/ignite/internal/sql/sqllogic/ItSqlLogicTest.java b/modules/sql-engine/src/integrationTest/java/org/apache/ignite/internal/sql/sqllogic/ItSqlLogicTest.java index bee186f5e3e..49df7bca717 100644 --- a/modules/sql-engine/src/integrationTest/java/org/apache/ignite/internal/sql/sqllogic/ItSqlLogicTest.java +++ b/modules/sql-engine/src/integrationTest/java/org/apache/ignite/internal/sql/sqllogic/ItSqlLogicTest.java @@ -351,7 +351,8 @@ private static void startNodes() { .clusterName("cluster") .clusterConfiguration("ignite {" + "metaStorage.idleSyncTimeInterval: " + METASTORAGE_IDLE_SYNC_TIME_INTERVAL_MS + ",\n" - + "gc.lowWatermark.dataAvailabilityTime: 5000,\n" + // TODO: Set dataAvailabilityTime to 5000 after IGNITE-24002 is fixed. + + "gc.lowWatermark.dataAvailabilityTime: 30000,\n" + "gc.lowWatermark.updateInterval: 1000,\n" + "metrics.exporters.logPush.exporterName: logPush,\n" + "metrics.exporters.logPush.period: 5000\n" diff --git a/modules/sql-engine/src/integrationTest/java/org/apache/ignite/internal/sql/sqllogic/ScriptContext.java b/modules/sql-engine/src/integrationTest/java/org/apache/ignite/internal/sql/sqllogic/ScriptContext.java index a8815808c68..641a83b4397 100644 --- a/modules/sql-engine/src/integrationTest/java/org/apache/ignite/internal/sql/sqllogic/ScriptContext.java +++ b/modules/sql-engine/src/integrationTest/java/org/apache/ignite/internal/sql/sqllogic/ScriptContext.java @@ -27,6 +27,7 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.concurrent.TimeUnit; import org.apache.ignite.internal.logger.IgniteLogger; import org.apache.ignite.internal.sql.sqllogic.SqlScriptRunner.RunnerRuntime; import org.apache.ignite.sql.IgniteSql; @@ -78,7 +79,9 @@ final class ScriptContext { List> executeQuery(String sql) { sql = replaceVars(sql); - log.info("Execute: " + sql); + log.info("Execute: {}", sql); + + long startNanos = System.nanoTime(); try (ResultSet rs = ignSql.execute(null, sql)) { if (rs.hasRowSet()) { @@ -100,6 +103,9 @@ List> executeQuery(String sql) { } else { return Collections.singletonList(Collections.singletonList(rs.wasApplied())); } + } finally { + long tookNanos = System.nanoTime() - startNanos; + log.info("Execution took {} ms", TimeUnit.NANOSECONDS.toMillis(tookNanos)); } } diff --git a/modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/tx/QueryTransactionContextImpl.java b/modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/tx/QueryTransactionContextImpl.java index f1db775f409..ee5275787ee 100644 --- a/modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/tx/QueryTransactionContextImpl.java +++ b/modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/tx/QueryTransactionContextImpl.java @@ -24,6 +24,7 @@ import org.apache.ignite.internal.sql.engine.exec.TransactionTracker; import org.apache.ignite.internal.tx.HybridTimestampTracker; import org.apache.ignite.internal.tx.InternalTransaction; +import org.apache.ignite.internal.tx.InternalTxOptions; import org.apache.ignite.internal.tx.TxManager; import org.apache.ignite.tx.TransactionException; import org.jetbrains.annotations.Nullable; @@ -58,7 +59,12 @@ public QueryTransactionWrapper getOrStartSqlManaged(boolean readOnly, boolean im QueryTransactionWrapper result; if (tx == null) { - transaction = txManager.begin(observableTimeTracker, implicit, readOnly); + if (implicit) { + transaction = txManager.beginImplicit(observableTimeTracker, readOnly); + } else { + transaction = txManager.beginExplicit(observableTimeTracker, readOnly, InternalTxOptions.defaults()); + } + result = new QueryTransactionWrapperImpl(transaction, true, txTracker); } else { transaction = tx.unwrap(); diff --git a/modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/QueryTransactionWrapperSelfTest.java b/modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/QueryTransactionWrapperSelfTest.java index 6cc20636a42..394241d9d74 100644 --- a/modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/QueryTransactionWrapperSelfTest.java +++ b/modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/QueryTransactionWrapperSelfTest.java @@ -131,7 +131,7 @@ public void throwsExceptionForNestedScriptTransaction() { ); IgniteSqlStartTransaction txStartStmt = mock(IgniteSqlStartTransaction.class); - when(txManager.begin(any(), anyBoolean(), anyBoolean())).thenAnswer(inv -> { + when(txManager.beginExplicit(any(), anyBoolean(), any())).thenAnswer(inv -> { boolean implicit = inv.getArgument(1, Boolean.class); return NoOpTransaction.readWrite("test", implicit); @@ -222,12 +222,11 @@ public void testScriptTransactionWrapperTxInflightsInteraction() { } private void prepareTransactionsMocks() { - when(txManager.begin(any(), anyBoolean(), anyBoolean())).thenAnswer( + when(txManager.beginExplicit(any(), anyBoolean(), any())).thenAnswer( inv -> { - boolean implicit = inv.getArgument(1, Boolean.class); - boolean readOnly = inv.getArgument(2, Boolean.class); + boolean readOnly = inv.getArgument(1, Boolean.class); - return readOnly ? NoOpTransaction.readOnly("test-ro", implicit) : NoOpTransaction.readWrite("test-rw", implicit); + return readOnly ? NoOpTransaction.readOnly("test-ro", false) : NoOpTransaction.readWrite("test-rw", false); } ); } diff --git a/modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/exec/rel/TableScanNodeExecutionTest.java b/modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/exec/rel/TableScanNodeExecutionTest.java index d98e0cbbf14..5f112f79892 100644 --- a/modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/exec/rel/TableScanNodeExecutionTest.java +++ b/modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/exec/rel/TableScanNodeExecutionTest.java @@ -36,6 +36,7 @@ import java.util.concurrent.CountDownLatch; import java.util.concurrent.Flow.Publisher; import java.util.concurrent.Flow.Subscription; +import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.ThreadLocalRandom; import java.util.stream.Collectors; import java.util.stream.IntStream; @@ -60,6 +61,7 @@ import org.apache.ignite.internal.schema.BinaryRow; import org.apache.ignite.internal.schema.BinaryRowEx; import org.apache.ignite.internal.schema.BinaryTuplePrefix; +import org.apache.ignite.internal.schema.configuration.LowWatermarkConfiguration; import org.apache.ignite.internal.sql.engine.exec.ExecutionContext; import org.apache.ignite.internal.sql.engine.exec.PartitionProvider; import org.apache.ignite.internal.sql.engine.exec.PartitionWithConsistencyToken; @@ -75,6 +77,8 @@ import org.apache.ignite.internal.storage.engine.MvTableStorage; import org.apache.ignite.internal.table.StreamerReceiverRunner; import org.apache.ignite.internal.table.distributed.storage.InternalTableImpl; +import org.apache.ignite.internal.testframework.ExecutorServiceExtension; +import org.apache.ignite.internal.testframework.InjectExecutorService; import org.apache.ignite.internal.tx.HybridTimestampTracker; import org.apache.ignite.internal.tx.TxManager; import org.apache.ignite.internal.tx.configuration.TransactionConfiguration; @@ -97,12 +101,19 @@ * Tests execution flow of TableScanNode. */ @ExtendWith(ConfigurationExtension.class) +@ExtendWith(ExecutorServiceExtension.class) public class TableScanNodeExecutionTest extends AbstractExecutionTest { private final LinkedList closeables = new LinkedList<>(); @InjectConfiguration private TransactionConfiguration txConfiguration; + @InjectConfiguration + private LowWatermarkConfiguration lowWatermarkConfiguration; + + @InjectExecutorService + private ScheduledExecutorService commonExecutor; + // Ensures that all data from TableScanNode is being propagated correctly. @Test public void testScanNodeDataPropagation() throws InterruptedException { @@ -161,6 +172,7 @@ public void testScanNodeDataPropagation() throws InterruptedException { TxManagerImpl txManager = new TxManagerImpl( txConfiguration, + lowWatermarkConfiguration, clusterService, replicaSvc, HeapLockManager.smallInstance(), @@ -171,7 +183,8 @@ public void testScanNodeDataPropagation() throws InterruptedException { new TestLocalRwTxCounter(), resourcesRegistry, transactionInflights, - new TestLowWatermark() + new TestLowWatermark(), + commonExecutor ); assertThat(txManager.startAsync(new ComponentContext()), willCompleteSuccessfully()); diff --git a/modules/table/src/integrationTest/java/org/apache/ignite/distributed/ItInternalTableReadOnlyScanTest.java b/modules/table/src/integrationTest/java/org/apache/ignite/distributed/ItInternalTableReadOnlyScanTest.java index 14024a33c7f..200c22bf59a 100644 --- a/modules/table/src/integrationTest/java/org/apache/ignite/distributed/ItInternalTableReadOnlyScanTest.java +++ b/modules/table/src/integrationTest/java/org/apache/ignite/distributed/ItInternalTableReadOnlyScanTest.java @@ -26,6 +26,7 @@ import org.apache.ignite.internal.table.InternalTable; import org.apache.ignite.internal.tx.HybridTimestampTracker; import org.apache.ignite.internal.tx.InternalTransaction; +import org.apache.ignite.internal.tx.InternalTxOptions; import org.apache.ignite.network.ClusterNode; import org.jetbrains.annotations.Nullable; import org.junit.jupiter.api.Test; @@ -48,7 +49,7 @@ protected Publisher scan(int part, @Nullable InternalTransaction tx) @Override protected InternalTransaction startTx() { - return internalTbl.txManager().begin(HYBRID_TIMESTAMP_TRACKER, false, true); + return internalTbl.txManager().beginExplicit(HYBRID_TIMESTAMP_TRACKER, true, InternalTxOptions.defaults()); } @Override diff --git a/modules/table/src/integrationTest/java/org/apache/ignite/distributed/ItInternalTableReadWriteScanTest.java b/modules/table/src/integrationTest/java/org/apache/ignite/distributed/ItInternalTableReadWriteScanTest.java index 7f1c58925c4..13c3c221603 100644 --- a/modules/table/src/integrationTest/java/org/apache/ignite/distributed/ItInternalTableReadWriteScanTest.java +++ b/modules/table/src/integrationTest/java/org/apache/ignite/distributed/ItInternalTableReadWriteScanTest.java @@ -27,6 +27,7 @@ import org.apache.ignite.internal.table.RollbackTxOnErrorPublisher; import org.apache.ignite.internal.tx.HybridTimestampTracker; import org.apache.ignite.internal.tx.InternalTransaction; +import org.apache.ignite.internal.tx.InternalTxOptions; import org.apache.ignite.internal.utils.PrimaryReplica; import org.apache.ignite.network.ClusterNode; import org.jetbrains.annotations.Nullable; @@ -71,7 +72,7 @@ public void testInvalidPartitionParameterScan() { @Override protected InternalTransaction startTx() { - InternalTransaction tx = internalTbl.txManager().begin(HYBRID_TIMESTAMP_TRACKER, false); + InternalTransaction tx = internalTbl.txManager().beginExplicitRw(HYBRID_TIMESTAMP_TRACKER, InternalTxOptions.defaults()); TablePartitionId tblPartId = new TablePartitionId(internalTbl.tableId(), ((TablePartitionId) internalTbl.groupId()).partitionId()); diff --git a/modules/table/src/integrationTest/java/org/apache/ignite/distributed/ItLockTableTest.java b/modules/table/src/integrationTest/java/org/apache/ignite/distributed/ItLockTableTest.java index 793e3aae3ba..c4ecbbc7d37 100644 --- a/modules/table/src/integrationTest/java/org/apache/ignite/distributed/ItLockTableTest.java +++ b/modules/table/src/integrationTest/java/org/apache/ignite/distributed/ItLockTableTest.java @@ -23,6 +23,7 @@ import java.util.ArrayList; import java.util.List; import java.util.concurrent.CompletableFuture; +import java.util.concurrent.ScheduledExecutorService; import org.apache.ignite.internal.configuration.testframework.ConfigurationExtension; import org.apache.ignite.internal.configuration.testframework.InjectConfiguration; import org.apache.ignite.internal.hlc.ClockService; @@ -37,9 +38,12 @@ import org.apache.ignite.internal.schema.Column; import org.apache.ignite.internal.schema.SchemaDescriptor; import org.apache.ignite.internal.schema.configuration.GcConfiguration; +import org.apache.ignite.internal.schema.configuration.LowWatermarkConfiguration; import org.apache.ignite.internal.schema.configuration.StorageUpdateConfiguration; import org.apache.ignite.internal.table.TableViewInternal; +import org.apache.ignite.internal.testframework.ExecutorServiceExtension; import org.apache.ignite.internal.testframework.IgniteAbstractTest; +import org.apache.ignite.internal.testframework.InjectExecutorService; import org.apache.ignite.internal.tx.HybridTimestampTracker; import org.apache.ignite.internal.tx.configuration.TransactionConfiguration; import org.apache.ignite.internal.tx.impl.HeapLockManager; @@ -65,6 +69,7 @@ * Test lock table. */ @ExtendWith(ConfigurationExtension.class) +@ExtendWith(ExecutorServiceExtension.class) public class ItLockTableTest extends IgniteAbstractTest { private static final IgniteLogger LOG = Loggers.forClass(ItLockTableTest.class); @@ -95,12 +100,18 @@ public class ItLockTableTest extends IgniteAbstractTest { @InjectConfiguration("mock: { deadlockPreventionPolicy: { waitTimeout: -1, txIdComparator: NONE } }") protected static TransactionConfiguration txConfiguration; + @InjectConfiguration + protected LowWatermarkConfiguration lowWatermarkConfiguration; + @InjectConfiguration protected static ReplicationConfiguration replicationConfiguration; @InjectConfiguration protected static StorageUpdateConfiguration storageUpdateConfiguration; + @InjectExecutorService + protected ScheduledExecutorService commonExecutor; + private ItTxTestCluster txTestCluster; private HybridTimestampTracker timestampTracker = HybridTimestampTracker.atomicTracker(null); @@ -120,6 +131,7 @@ public void before() throws Exception { testInfo, raftConfiguration, txConfiguration, + lowWatermarkConfiguration, storageUpdateConfiguration, workDir, 1, @@ -142,6 +154,7 @@ protected TxManagerImpl newTxManager( ) { return new TxManagerImpl( txConfiguration, + lowWatermarkConfiguration, clusterService, replicaSvc, new HeapLockManager( @@ -154,7 +167,8 @@ protected TxManagerImpl newTxManager( new TestLocalRwTxCounter(), resourcesRegistry, transactionInflights, - lowWatermark + lowWatermark, + commonExecutor ); } }; diff --git a/modules/table/src/integrationTest/java/org/apache/ignite/distributed/ItTxDistributedCleanupRecoveryTest.java b/modules/table/src/integrationTest/java/org/apache/ignite/distributed/ItTxDistributedCleanupRecoveryTest.java index a1fa4e0e1dc..a981b630f50 100644 --- a/modules/table/src/integrationTest/java/org/apache/ignite/distributed/ItTxDistributedCleanupRecoveryTest.java +++ b/modules/table/src/integrationTest/java/org/apache/ignite/distributed/ItTxDistributedCleanupRecoveryTest.java @@ -54,6 +54,7 @@ public void before() throws Exception { testInfo, raftConfiguration, txConfiguration, + lowWatermarkConfiguration, storageUpdateConfiguration, workDir, nodes(), diff --git a/modules/table/src/integrationTest/java/org/apache/ignite/distributed/ItTxDistributedTestSingleNodeNoCleanupMessage.java b/modules/table/src/integrationTest/java/org/apache/ignite/distributed/ItTxDistributedTestSingleNodeNoCleanupMessage.java index bcce6ff3e18..f347a8d81af 100644 --- a/modules/table/src/integrationTest/java/org/apache/ignite/distributed/ItTxDistributedTestSingleNodeNoCleanupMessage.java +++ b/modules/table/src/integrationTest/java/org/apache/ignite/distributed/ItTxDistributedTestSingleNodeNoCleanupMessage.java @@ -34,7 +34,6 @@ import java.util.concurrent.Executor; import java.util.function.Supplier; import org.apache.ignite.internal.catalog.CatalogService; -import org.apache.ignite.internal.configuration.testframework.InjectConfiguration; import org.apache.ignite.internal.hlc.ClockService; import org.apache.ignite.internal.hlc.HybridTimestamp; import org.apache.ignite.internal.lowwatermark.LowWatermark; @@ -59,7 +58,6 @@ import org.apache.ignite.internal.tx.InternalTransaction; import org.apache.ignite.internal.tx.LockManager; import org.apache.ignite.internal.tx.TxManager; -import org.apache.ignite.internal.tx.configuration.TransactionConfiguration; import org.apache.ignite.internal.tx.impl.HeapLockManager; import org.apache.ignite.internal.tx.impl.RemotelyTriggeredResourceRegistry; import org.apache.ignite.internal.tx.impl.TransactionIdGenerator; @@ -84,9 +82,6 @@ public class ItTxDistributedTestSingleNodeNoCleanupMessage extends TxAbstractTes /** A list of background cleanup futures. */ private final List> cleanupFutures = new CopyOnWriteArrayList<>(); - @InjectConfiguration - private TransactionConfiguration txConfiguration; - /** * The constructor. * @@ -103,6 +98,7 @@ public void before() throws Exception { testInfo, raftConfiguration, txConfiguration, + lowWatermarkConfiguration, storageUpdateConfiguration, workDir, nodes(), @@ -125,6 +121,7 @@ protected TxManagerImpl newTxManager( ) { return new TxManagerImpl( txConfiguration, + lowWatermarkConfiguration, clusterService, replicaSvc, new HeapLockManager(), @@ -135,7 +132,8 @@ protected TxManagerImpl newTxManager( new TestLocalRwTxCounter(), resourcesRegistry, transactionInflights, - lowWatermark + lowWatermark, + commonExecutor ) { @Override public CompletableFuture executeWriteIntentSwitchAsync(Runnable runnable) { diff --git a/modules/table/src/integrationTest/java/org/apache/ignite/distributed/ItTxStateLocalMapTest.java b/modules/table/src/integrationTest/java/org/apache/ignite/distributed/ItTxStateLocalMapTest.java index 2b6ad33a8cf..04adec949c8 100644 --- a/modules/table/src/integrationTest/java/org/apache/ignite/distributed/ItTxStateLocalMapTest.java +++ b/modules/table/src/integrationTest/java/org/apache/ignite/distributed/ItTxStateLocalMapTest.java @@ -36,6 +36,7 @@ import org.apache.ignite.internal.replicator.configuration.ReplicationConfiguration; import org.apache.ignite.internal.schema.Column; import org.apache.ignite.internal.schema.SchemaDescriptor; +import org.apache.ignite.internal.schema.configuration.LowWatermarkConfiguration; import org.apache.ignite.internal.schema.configuration.StorageUpdateConfiguration; import org.apache.ignite.internal.table.TableViewInternal; import org.apache.ignite.internal.testframework.IgniteAbstractTest; @@ -69,6 +70,9 @@ public class ItTxStateLocalMapTest extends IgniteAbstractTest { @InjectConfiguration private TransactionConfiguration txConfiguration; + @InjectConfiguration + protected LowWatermarkConfiguration lowWatermarkConfiguration; + @InjectConfiguration private StorageUpdateConfiguration storageUpdateConfiguration; @@ -102,6 +106,7 @@ public void before() throws Exception { testInfo, raftConfig, txConfiguration, + lowWatermarkConfiguration, storageUpdateConfiguration, workDir, NODES, diff --git a/modules/table/src/integrationTest/java/org/apache/ignite/internal/table/ItColocationTest.java b/modules/table/src/integrationTest/java/org/apache/ignite/internal/table/ItColocationTest.java index 46eada69ea7..ee91ee9e5e8 100644 --- a/modules/table/src/integrationTest/java/org/apache/ignite/internal/table/ItColocationTest.java +++ b/modules/table/src/integrationTest/java/org/apache/ignite/internal/table/ItColocationTest.java @@ -49,6 +49,7 @@ import java.util.Set; import java.util.UUID; import java.util.concurrent.CompletableFuture; +import java.util.concurrent.ScheduledExecutorService; import java.util.stream.Collectors; import java.util.stream.IntStream; import java.util.stream.Stream; @@ -90,6 +91,7 @@ import org.apache.ignite.internal.schema.NullBinaryRow; import org.apache.ignite.internal.schema.SchemaDescriptor; import org.apache.ignite.internal.schema.SchemaRegistry; +import org.apache.ignite.internal.schema.configuration.LowWatermarkConfiguration; import org.apache.ignite.internal.schema.marshaller.TupleMarshallerImpl; import org.apache.ignite.internal.schema.row.Row; import org.apache.ignite.internal.sql.engine.util.SqlTestUtils; @@ -99,6 +101,8 @@ import org.apache.ignite.internal.table.impl.DummyInternalTableImpl; import org.apache.ignite.internal.table.impl.DummySchemaManagerImpl; import org.apache.ignite.internal.testframework.BaseIgniteAbstractTest; +import org.apache.ignite.internal.testframework.ExecutorServiceExtension; +import org.apache.ignite.internal.testframework.InjectExecutorService; import org.apache.ignite.internal.tx.HybridTimestampTracker; import org.apache.ignite.internal.tx.LockManager; import org.apache.ignite.internal.tx.TxManager; @@ -132,6 +136,7 @@ * Tests for data colocation. */ @ExtendWith(ConfigurationExtension.class) +@ExtendWith(ExecutorServiceExtension.class) public class ItColocationTest extends BaseIgniteAbstractTest { /** Partitions count. */ private static final int PARTS = 32; @@ -159,6 +164,12 @@ public class ItColocationTest extends BaseIgniteAbstractTest { @InjectConfiguration private static TransactionConfiguration txConfiguration; + @InjectConfiguration + private static LowWatermarkConfiguration lowWatermarkConfiguration; + + @InjectExecutorService + private static ScheduledExecutorService commonExecutor; + private SchemaDescriptor schema; private SchemaRegistry schemaRegistry; @@ -188,6 +199,7 @@ static void beforeAllTests() { txManager = new TxManagerImpl( txConfiguration, + lowWatermarkConfiguration, clusterService, replicaService, new HeapLockManager(), @@ -198,7 +210,8 @@ static void beforeAllTests() { new TestLocalRwTxCounter(), resourcesRegistry, transactionInflights, - new TestLowWatermark() + new TestLowWatermark(), + commonExecutor ) { @Override public CompletableFuture finish( diff --git a/modules/table/src/main/java/org/apache/ignite/internal/table/distributed/storage/InternalTableImpl.java b/modules/table/src/main/java/org/apache/ignite/internal/table/distributed/storage/InternalTableImpl.java index eaf214ee076..d516385639a 100644 --- a/modules/table/src/main/java/org/apache/ignite/internal/table/distributed/storage/InternalTableImpl.java +++ b/modules/table/src/main/java/org/apache/ignite/internal/table/distributed/storage/InternalTableImpl.java @@ -501,11 +501,11 @@ private CompletableFuture enlistInTx( } private InternalTransaction startImplicitRwTxIfNeeded(@Nullable InternalTransaction tx) { - return tx == null ? txManager.begin(observableTimestampTracker, true) : tx; + return tx == null ? txManager.beginImplicitRw(observableTimestampTracker) : tx; } private InternalTransaction startImplicitRoTxIfNeeded(@Nullable InternalTransaction tx) { - return tx == null ? txManager.begin(observableTimestampTracker, true, true) : tx; + return tx == null ? txManager.beginImplicit(observableTimestampTracker, true) : tx; } /** @@ -1169,7 +1169,7 @@ private CompletableFuture updateAllWithRetry( int partition, @Nullable Long txStartTs ) { - InternalTransaction tx = txManager.begin(observableTimestampTracker, true); + InternalTransaction tx = txManager.beginImplicitRw(observableTimestampTracker); TablePartitionId partGroupId = new TablePartitionId(tableId, partition); assert rows.stream().allMatch(row -> partitionId(row) == partition) : "Invalid batch for partition " + partition; diff --git a/modules/table/src/testFixtures/java/org/apache/ignite/distributed/ItTxTestCluster.java b/modules/table/src/testFixtures/java/org/apache/ignite/distributed/ItTxTestCluster.java index 1c052875c7f..aa5d92b618b 100644 --- a/modules/table/src/testFixtures/java/org/apache/ignite/distributed/ItTxTestCluster.java +++ b/modules/table/src/testFixtures/java/org/apache/ignite/distributed/ItTxTestCluster.java @@ -123,6 +123,7 @@ import org.apache.ignite.internal.schema.SchemaDescriptor; import org.apache.ignite.internal.schema.SchemaRegistry; import org.apache.ignite.internal.schema.SchemaSyncService; +import org.apache.ignite.internal.schema.configuration.LowWatermarkConfiguration; import org.apache.ignite.internal.schema.configuration.StorageUpdateConfiguration; import org.apache.ignite.internal.storage.MvPartitionStorage; import org.apache.ignite.internal.storage.engine.MvTableStorage; @@ -198,6 +199,8 @@ public class ItTxTestCluster { private final TransactionConfiguration txConfiguration; + private final LowWatermarkConfiguration lowWatermarkConfiguration; + private final StorageUpdateConfiguration storageUpdateConfiguration; private final Path workDir; @@ -320,6 +323,7 @@ public ItTxTestCluster( TestInfo testInfo, RaftConfiguration raftConfig, TransactionConfiguration txConfiguration, + LowWatermarkConfiguration lowWatermarkConfiguration, StorageUpdateConfiguration storageUpdateConfiguration, Path workDir, int nodes, @@ -330,6 +334,7 @@ public ItTxTestCluster( ) { this.raftConfig = raftConfig; this.txConfiguration = txConfiguration; + this.lowWatermarkConfiguration = lowWatermarkConfiguration; this.storageUpdateConfiguration = storageUpdateConfiguration; this.workDir = workDir; this.nodes = nodes; @@ -573,6 +578,7 @@ protected TxManagerImpl newTxManager( return new TxManagerImpl( node.name(), txConfiguration, + lowWatermarkConfiguration, clusterService.messagingService(), clusterService.topologyService(), replicaSvc, @@ -585,7 +591,8 @@ protected TxManagerImpl newTxManager( partitionOperationsExecutor, resourcesRegistry, transactionInflights, - lowWatermark + lowWatermark, + executor ); } @@ -1062,6 +1069,7 @@ private void initializeClientTxComponents() { clientTxManager = new TxManagerImpl( "client", txConfiguration, + lowWatermarkConfiguration, client.messagingService(), client.topologyService(), clientReplicaSvc, @@ -1074,7 +1082,8 @@ private void initializeClientTxComponents() { partitionOperationsExecutor, resourceRegistry, clientTransactionInflights, - lowWatermark + lowWatermark, + executor ); clientResourceVacuumManager = new ResourceVacuumManager( diff --git a/modules/table/src/testFixtures/java/org/apache/ignite/internal/table/TxAbstractTest.java b/modules/table/src/testFixtures/java/org/apache/ignite/internal/table/TxAbstractTest.java index 86c7e31ae19..8c3f41467a6 100644 --- a/modules/table/src/testFixtures/java/org/apache/ignite/internal/table/TxAbstractTest.java +++ b/modules/table/src/testFixtures/java/org/apache/ignite/internal/table/TxAbstractTest.java @@ -83,6 +83,7 @@ import org.apache.ignite.internal.table.distributed.replicator.PartitionReplicaListener; import org.apache.ignite.internal.testframework.IgniteTestUtils; import org.apache.ignite.internal.tx.InternalTransaction; +import org.apache.ignite.internal.tx.InternalTxOptions; import org.apache.ignite.internal.tx.Lock; import org.apache.ignite.internal.tx.LockException; import org.apache.ignite.internal.tx.LockManager; @@ -1735,7 +1736,7 @@ public void run() { } while (!stop.get() && firstErr.get() == null) { - InternalTransaction tx = clientTxManager().begin(timestampTracker, false, false); + InternalTransaction tx = clientTxManager().beginExplicitRw(timestampTracker, InternalTxOptions.defaults()); var table = accounts.recordView(); diff --git a/modules/table/src/testFixtures/java/org/apache/ignite/internal/table/TxInfrastructureTest.java b/modules/table/src/testFixtures/java/org/apache/ignite/internal/table/TxInfrastructureTest.java index 341fa32abbc..86ae76efd0a 100644 --- a/modules/table/src/testFixtures/java/org/apache/ignite/internal/table/TxInfrastructureTest.java +++ b/modules/table/src/testFixtures/java/org/apache/ignite/internal/table/TxInfrastructureTest.java @@ -25,6 +25,7 @@ import java.util.Collection; import java.util.Map; import java.util.concurrent.CompletableFuture; +import java.util.concurrent.ScheduledExecutorService; import org.apache.ignite.distributed.ItTxTestCluster; import org.apache.ignite.internal.configuration.testframework.ConfigurationExtension; import org.apache.ignite.internal.configuration.testframework.InjectConfiguration; @@ -46,11 +47,14 @@ import org.apache.ignite.internal.replicator.configuration.ReplicationConfiguration; import org.apache.ignite.internal.schema.Column; import org.apache.ignite.internal.schema.SchemaDescriptor; +import org.apache.ignite.internal.schema.configuration.LowWatermarkConfiguration; import org.apache.ignite.internal.schema.configuration.StorageUpdateConfiguration; import org.apache.ignite.internal.storage.MvPartitionStorage; import org.apache.ignite.internal.table.distributed.raft.PartitionListener; +import org.apache.ignite.internal.testframework.ExecutorServiceExtension; import org.apache.ignite.internal.testframework.IgniteAbstractTest; import org.apache.ignite.internal.testframework.IgniteTestUtils; +import org.apache.ignite.internal.testframework.InjectExecutorService; import org.apache.ignite.internal.tx.HybridTimestampTracker; import org.apache.ignite.internal.tx.TxManager; import org.apache.ignite.internal.tx.configuration.TransactionConfiguration; @@ -72,7 +76,7 @@ /** * Setup infrastructure for tx related test scenarios. */ -@ExtendWith({MockitoExtension.class, ConfigurationExtension.class}) +@ExtendWith({MockitoExtension.class, ConfigurationExtension.class, ExecutorServiceExtension.class}) @MockitoSettings(strictness = Strictness.LENIENT) public abstract class TxInfrastructureTest extends IgniteAbstractTest { protected static final double BALANCE_1 = 500; @@ -114,12 +118,18 @@ public abstract class TxInfrastructureTest extends IgniteAbstractTest { @InjectConfiguration protected TransactionConfiguration txConfiguration; + @InjectConfiguration + protected LowWatermarkConfiguration lowWatermarkConfiguration; + @InjectConfiguration protected StorageUpdateConfiguration storageUpdateConfiguration; @InjectConfiguration protected ReplicationConfiguration replicationConfiguration; + @InjectExecutorService + protected ScheduledExecutorService commonExecutor; + protected final TestInfo testInfo; protected ItTxTestCluster txTestCluster; @@ -167,6 +177,7 @@ public void before() throws Exception { testInfo, raftConfiguration, txConfiguration, + lowWatermarkConfiguration, storageUpdateConfiguration, workDir, nodes(), diff --git a/modules/table/src/testFixtures/java/org/apache/ignite/internal/table/impl/DummyInternalTableImpl.java b/modules/table/src/testFixtures/java/org/apache/ignite/internal/table/impl/DummyInternalTableImpl.java index 570282eef8d..f977f6db406 100644 --- a/modules/table/src/testFixtures/java/org/apache/ignite/internal/table/impl/DummyInternalTableImpl.java +++ b/modules/table/src/testFixtures/java/org/apache/ignite/internal/table/impl/DummyInternalTableImpl.java @@ -39,6 +39,7 @@ import java.util.concurrent.CompletableFuture; import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicLong; +import org.apache.ignite.configuration.ConfigurationValue; import org.apache.ignite.distributed.TestPartitionDataStorage; import org.apache.ignite.internal.TestHybridClock; import org.apache.ignite.internal.catalog.CatalogService; @@ -84,6 +85,7 @@ import org.apache.ignite.internal.schema.BinaryRowEx; import org.apache.ignite.internal.schema.ColumnsExtractor; import org.apache.ignite.internal.schema.SchemaDescriptor; +import org.apache.ignite.internal.schema.configuration.LowWatermarkConfiguration; import org.apache.ignite.internal.schema.configuration.StorageUpdateConfiguration; import org.apache.ignite.internal.storage.MvPartitionStorage; import org.apache.ignite.internal.storage.engine.MvTableStorage; @@ -106,6 +108,7 @@ import org.apache.ignite.internal.table.distributed.replicator.TransactionStateResolver; import org.apache.ignite.internal.table.distributed.schema.AlwaysSyncedSchemaSyncService; import org.apache.ignite.internal.table.distributed.storage.InternalTableImpl; +import org.apache.ignite.internal.testframework.CommonTestScheduler; import org.apache.ignite.internal.tx.HybridTimestampTracker; import org.apache.ignite.internal.tx.InternalTransaction; import org.apache.ignite.internal.tx.TxManager; @@ -558,8 +561,14 @@ public static TxManagerImpl txManager( TransactionInflights transactionInflights = new TransactionInflights(placementDriver, CLOCK_SERVICE); + LowWatermarkConfiguration lowWatermarkConfiguration = mock(LowWatermarkConfiguration.class); + ConfigurationValue dataAvailabilityTime = mock(ConfigurationValue.class); + lenient().when(dataAvailabilityTime.value()).thenReturn(15L * 60 * 1000); + lenient().when(lowWatermarkConfiguration.dataAvailabilityTime()).thenReturn(dataAvailabilityTime); + var txManager = new TxManagerImpl( txConfiguration, + lowWatermarkConfiguration, clusterService, replicaSvc, HeapLockManager.smallInstance(), @@ -570,7 +579,8 @@ public static TxManagerImpl txManager( new TestLocalRwTxCounter(), resourcesRegistry, transactionInflights, - new TestLowWatermark() + new TestLowWatermark(), + CommonTestScheduler.instance() ); assertThat(txManager.startAsync(new ComponentContext()), willCompleteSuccessfully()); diff --git a/modules/transactions/build.gradle b/modules/transactions/build.gradle index 5ba2d181a8b..8191110c837 100644 --- a/modules/transactions/build.gradle +++ b/modules/transactions/build.gradle @@ -79,11 +79,14 @@ dependencies { integrationTestImplementation project(':ignite-schema') integrationTestImplementation project(':ignite-low-watermark') integrationTestImplementation project(':ignite-configuration-system') + integrationTestImplementation project(':ignite-client') + integrationTestImplementation project(':ignite-client-handler') integrationTestImplementation libs.jetbrains.annotations integrationTestImplementation(testFixtures(project(':ignite-core'))) integrationTestImplementation(testFixtures(project(':ignite-transactions'))) integrationTestImplementation(testFixtures(project(':ignite-sql-engine'))) integrationTestImplementation(testFixtures(project(':ignite-runner'))) + integrationTestImplementation libs.netty.transport testFixturesImplementation project(':ignite-configuration') testFixturesImplementation project(':ignite-core') diff --git a/modules/transactions/src/integrationTest/java/org/apache/ignite/internal/tx/readonly/ItClientReadOnlyTxTimeoutOneNodeTest.java b/modules/transactions/src/integrationTest/java/org/apache/ignite/internal/tx/readonly/ItClientReadOnlyTxTimeoutOneNodeTest.java new file mode 100644 index 00000000000..b7ca1c75964 --- /dev/null +++ b/modules/transactions/src/integrationTest/java/org/apache/ignite/internal/tx/readonly/ItClientReadOnlyTxTimeoutOneNodeTest.java @@ -0,0 +1,65 @@ +/* + * 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. + */ + +package org.apache.ignite.internal.tx.readonly; + +import static org.apache.ignite.internal.TestWrappers.unwrapIgniteImpl; + +import org.apache.ignite.Ignite; +import org.apache.ignite.client.IgniteClient; +import org.apache.ignite.client.handler.ClientResourceRegistry; +import org.apache.ignite.internal.client.tx.ClientLazyTransaction; +import org.apache.ignite.internal.lang.IgniteInternalCheckedException; +import org.apache.ignite.internal.tx.impl.ReadOnlyTransactionImpl; +import org.apache.ignite.tx.Transaction; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; + +class ItClientReadOnlyTxTimeoutOneNodeTest extends ItReadOnlyTxTimeoutOneNodeTest { + private IgniteClient client; + + @BeforeEach + void startClient() { + client = IgniteClient.builder() + .addresses("localhost:" + unwrapIgniteImpl(cluster.aliveNode()).clientAddress().port()) + .build(); + } + + @AfterEach + void closeClient() { + if (client != null) { + client.close(); + } + } + + @Override + Ignite ignite() { + return client; + } + + @Override + ReadOnlyTransactionImpl transactionImpl(Transaction tx) { + long txId = ClientLazyTransaction.get(tx).startedTx().id(); + + ClientResourceRegistry resources = unwrapIgniteImpl(cluster.aliveNode()).clientInboundMessageHandler().resources(); + try { + return resources.get(txId).get(ReadOnlyTransactionImpl.class); + } catch (IgniteInternalCheckedException e) { + throw new RuntimeException(e); + } + } +} diff --git a/modules/transactions/src/integrationTest/java/org/apache/ignite/internal/tx/readonly/ItEmbeddedReadOnlyTxTimeoutOneNodeTest.java b/modules/transactions/src/integrationTest/java/org/apache/ignite/internal/tx/readonly/ItEmbeddedReadOnlyTxTimeoutOneNodeTest.java new file mode 100644 index 00000000000..9d174defc88 --- /dev/null +++ b/modules/transactions/src/integrationTest/java/org/apache/ignite/internal/tx/readonly/ItEmbeddedReadOnlyTxTimeoutOneNodeTest.java @@ -0,0 +1,35 @@ +/* + * 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. + */ + +package org.apache.ignite.internal.tx.readonly; + +import org.apache.ignite.Ignite; +import org.apache.ignite.internal.tx.impl.ReadOnlyTransactionImpl; +import org.apache.ignite.internal.wrapper.Wrappers; +import org.apache.ignite.tx.Transaction; + +class ItEmbeddedReadOnlyTxTimeoutOneNodeTest extends ItReadOnlyTxTimeoutOneNodeTest { + @Override + Ignite ignite() { + return cluster.aliveNode(); + } + + @Override + ReadOnlyTransactionImpl transactionImpl(Transaction tx) { + return Wrappers.unwrap(tx, ReadOnlyTransactionImpl.class); + } +} diff --git a/modules/transactions/src/integrationTest/java/org/apache/ignite/internal/tx/readonly/ItReadOnlyTxAndLowWatermarkTest.java b/modules/transactions/src/integrationTest/java/org/apache/ignite/internal/tx/readonly/ItReadOnlyTxAndLowWatermarkTest.java index df078a35b63..7ba4671d6c0 100644 --- a/modules/transactions/src/integrationTest/java/org/apache/ignite/internal/tx/readonly/ItReadOnlyTxAndLowWatermarkTest.java +++ b/modules/transactions/src/integrationTest/java/org/apache/ignite/internal/tx/readonly/ItReadOnlyTxAndLowWatermarkTest.java @@ -25,6 +25,7 @@ import static org.apache.ignite.internal.testframework.matchers.CompletableFutureMatcher.willCompleteSuccessfully; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.either; import static org.hamcrest.Matchers.hasSize; import static org.hamcrest.Matchers.hasToString; import static org.hamcrest.Matchers.is; @@ -120,8 +121,14 @@ void roTransactionNoticesTupleVersionsMissingDueToGcOnDataNodes(TransactionalRea IgniteException ex = assertThrows(IgniteException.class, () -> reader.read(coordinator, roTx)); assertThat(ex, isA(reader.sql() ? SqlException.class : TransactionException.class)); - assertThat(ex, hasToString(containsString("Read timestamp is not available anymore."))); - assertThat("Wrong error code: " + ex.codeAsString(), ex.code(), is(Transactions.TX_STALE_READ_ONLY_OPERATION_ERR)); + assertThat(ex, hasToString( + either(containsString("Read timestamp is not available anymore.")) + .or(containsString("Transaction is already finished")) + )); + assertThat("Wrong error code: " + ex.codeAsString(), ex.code(), + either(is(Transactions.TX_STALE_READ_ONLY_OPERATION_ERR)) + .or(is(Transactions.TX_ALREADY_FINISHED_ERR)) + ); } private void updateDataAvailabilityTimeToShortPeriod() { diff --git a/modules/transactions/src/integrationTest/java/org/apache/ignite/internal/tx/readonly/ItReadOnlyTxTimeoutOneNodeTest.java b/modules/transactions/src/integrationTest/java/org/apache/ignite/internal/tx/readonly/ItReadOnlyTxTimeoutOneNodeTest.java new file mode 100644 index 00000000000..0bb57262b7f --- /dev/null +++ b/modules/transactions/src/integrationTest/java/org/apache/ignite/internal/tx/readonly/ItReadOnlyTxTimeoutOneNodeTest.java @@ -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. + */ + +package org.apache.ignite.internal.tx.readonly; + +import static java.util.concurrent.TimeUnit.SECONDS; +import static org.apache.ignite.internal.testframework.IgniteTestUtils.waitForCondition; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import org.apache.ignite.Ignite; +import org.apache.ignite.internal.ClusterPerTestIntegrationTest; +import org.apache.ignite.internal.tx.impl.ReadOnlyTransactionImpl; +import org.apache.ignite.table.Table; +import org.apache.ignite.tx.Transaction; +import org.apache.ignite.tx.TransactionOptions; +import org.junit.jupiter.api.Test; + +abstract class ItReadOnlyTxTimeoutOneNodeTest extends ClusterPerTestIntegrationTest { + private static final String TABLE_NAME = "TEST"; + + @Override + protected int initialNodes() { + return 1; + } + + abstract Ignite ignite(); + + abstract ReadOnlyTransactionImpl transactionImpl(Transaction tx); + + @Test + void roTransactionTimesOut() throws Exception { + Ignite ignite = ignite(); + + ignite.sql().executeScript("CREATE TABLE " + TABLE_NAME + " (ID INT PRIMARY KEY, VAL VARCHAR)"); + + Table table = ignite.tables().table(TABLE_NAME); + + Transaction roTx = ignite.transactions().begin(new TransactionOptions().readOnly(true).timeoutMillis(100)); + + // Make sure the RO tx actually begins on the server (as thin client transactions are lazy). + doGetOn(table, roTx); + + assertTrue( + waitForCondition(() -> transactionImpl(roTx).isFinishingOrFinished(), SECONDS.toMillis(10)), + "Transaction should have been finished due to timeout" + ); + + // TODO: Uncomment the following lines after https://issues.apache.org/jira/browse/IGNITE-23980 is fixed. + // assertThrows(TransactionException.class, () -> doGetOn(table, roTx)); + // assertThrows(TransactionException.class, roTx::commit); + } + + private static void doGetOn(Table table, Transaction tx) { + table.keyValueView(Integer.class, String.class).get(tx, 1); + } +} diff --git a/modules/transactions/src/main/java/org/apache/ignite/internal/tx/InternalTxOptions.java b/modules/transactions/src/main/java/org/apache/ignite/internal/tx/InternalTxOptions.java new file mode 100644 index 00000000000..36d1ba62068 --- /dev/null +++ b/modules/transactions/src/main/java/org/apache/ignite/internal/tx/InternalTxOptions.java @@ -0,0 +1,79 @@ +/* + * 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. + */ + +package org.apache.ignite.internal.tx; + +/** + * Transaction options for internal use. + */ +public class InternalTxOptions { + private static final InternalTxOptions DEFAULT_OPTIONS = builder().build(); + + /** + * Transaction priority. The priority is used to resolve conflicts between transactions. The higher priority is + * the more likely the transaction will win the conflict. + */ + private final TxPriority priority; + + /** Transaction timeout. 0 means 'use default timeout'. */ + private final long timeoutMillis; + + private InternalTxOptions(TxPriority priority, long timeoutMillis) { + this.priority = priority; + this.timeoutMillis = timeoutMillis; + } + + public static Builder builder() { + return new Builder(); + } + + public static InternalTxOptions defaults() { + return DEFAULT_OPTIONS; + } + + public static InternalTxOptions defaultsWithPriority(TxPriority priority) { + return builder().priority(priority).build(); + } + + public TxPriority priority() { + return priority; + } + + public long timeoutMillis() { + return timeoutMillis; + } + + /** Builder for InternalTxOptions. */ + public static class Builder { + private TxPriority priority = TxPriority.NORMAL; + private long timeoutMillis = 0; + + public Builder priority(TxPriority priority) { + this.priority = priority; + return this; + } + + public Builder timeoutMillis(long timeoutMillis) { + this.timeoutMillis = timeoutMillis; + return this; + } + + public InternalTxOptions build() { + return new InternalTxOptions(priority, timeoutMillis); + } + } +} diff --git a/modules/transactions/src/main/java/org/apache/ignite/internal/tx/TxManager.java b/modules/transactions/src/main/java/org/apache/ignite/internal/tx/TxManager.java index d21f77d80e7..3f956c21927 100644 --- a/modules/transactions/src/main/java/org/apache/ignite/internal/tx/TxManager.java +++ b/modules/transactions/src/main/java/org/apache/ignite/internal/tx/TxManager.java @@ -24,10 +24,8 @@ import java.util.function.Function; import org.apache.ignite.internal.hlc.HybridTimestamp; import org.apache.ignite.internal.lang.IgniteBiTuple; -import org.apache.ignite.internal.lang.IgniteInternalException; import org.apache.ignite.internal.manager.IgniteComponent; import org.apache.ignite.internal.replicator.TablePartitionId; -import org.apache.ignite.lang.ErrorGroups.Transactions; import org.apache.ignite.network.ClusterNode; import org.jetbrains.annotations.Nullable; import org.jetbrains.annotations.TestOnly; @@ -37,45 +35,51 @@ */ public interface TxManager extends IgniteComponent { /** - * Starts a read-write transaction coordinated by a local node. + * Starts an implicit read-write transaction coordinated by a local node. * * @param timestampTracker Observable timestamp tracker is used to track a timestamp for either read-write or read-only * transaction execution. The tracker is also used to determine the read timestamp for read-only transactions. - * @param implicit Whether the transaction is implicit or not. * @return The transaction. */ - InternalTransaction begin(HybridTimestampTracker timestampTracker, boolean implicit); + default InternalTransaction beginImplicitRw(HybridTimestampTracker timestampTracker) { + return beginImplicit(timestampTracker, false); + } /** - * Starts either read-write or read-only transaction, depending on {@code readOnly} parameter value. The transaction has - * {@link TxPriority#NORMAL} priority. + * Starts an implicit read-write transaction coordinated by a local node. * * @param timestampTracker Observable timestamp tracker is used to track a timestamp for either read-write or read-only - * transaction execution. The tracker is also used to determine the read timestamp for read-only transactions. Each client - * should pass its own tracker to provide linearizability between read-write and read-only transactions started by this client. - * @param implicit Whether the transaction is implicit or not. + * transaction execution. The tracker is also used to determine the read timestamp for read-only transactions. * @param readOnly {@code true} in order to start a read-only transaction, {@code false} in order to start read-write one. * Calling begin with readOnly {@code false} is an equivalent of TxManager#begin(). - * @return The started transaction. - * @throws IgniteInternalException with {@link Transactions#TX_READ_ONLY_TOO_OLD_ERR} if transaction much older than the data - * available in the tables. + * @return The transaction. + */ + InternalTransaction beginImplicit(HybridTimestampTracker timestampTracker, boolean readOnly); + + /** + * Starts an explicit read-write transaction coordinated by a local node. + * + * @param timestampTracker Observable timestamp tracker is used to track a timestamp for either read-write or read-only + * transaction execution. The tracker is also used to determine the read timestamp for read-only transactions. + * @param options Transaction options. + * @return The transaction. */ - InternalTransaction begin(HybridTimestampTracker timestampTracker, boolean implicit, boolean readOnly); + default InternalTransaction beginExplicitRw(HybridTimestampTracker timestampTracker, InternalTxOptions options) { + return beginExplicit(timestampTracker, false, options); + } /** - * Starts either read-write or read-only transaction, depending on {@code readOnly} parameter value. + * Starts either read-write or read-only explicit transaction, depending on {@code readOnly} parameter value. * * @param timestampTracker Observable timestamp tracker is used to track a timestamp for either read-write or read-only * transaction execution. The tracker is also used to determine the read timestamp for read-only transactions. Each client * should pass its own tracker to provide linearizability between read-write and read-only transactions started by this client. - * @param implicit Whether the transaction is implicit or not. * @param readOnly {@code true} in order to start a read-only transaction, {@code false} in order to start read-write one. * Calling begin with readOnly {@code false} is an equivalent of TxManager#begin(). - * @param priority Transaction priority. The priority is used to resolve conflicts between transactions. The higher priority is - * the more likely the transaction will win the conflict. + * @param txOptions Options. * @return The started transaction. */ - InternalTransaction begin(HybridTimestampTracker timestampTracker, boolean implicit, boolean readOnly, TxPriority priority); + InternalTransaction beginExplicit(HybridTimestampTracker timestampTracker, boolean readOnly, InternalTxOptions txOptions); /** * Returns a transaction state meta. diff --git a/modules/transactions/src/main/java/org/apache/ignite/internal/tx/impl/IgniteTransactionsImpl.java b/modules/transactions/src/main/java/org/apache/ignite/internal/tx/impl/IgniteTransactionsImpl.java index f80cb12c9da..f29a5993009 100644 --- a/modules/transactions/src/main/java/org/apache/ignite/internal/tx/impl/IgniteTransactionsImpl.java +++ b/modules/transactions/src/main/java/org/apache/ignite/internal/tx/impl/IgniteTransactionsImpl.java @@ -19,6 +19,8 @@ import java.util.concurrent.CompletableFuture; import org.apache.ignite.internal.tx.HybridTimestampTracker; +import org.apache.ignite.internal.tx.InternalTransaction; +import org.apache.ignite.internal.tx.InternalTxOptions; import org.apache.ignite.internal.tx.TxManager; import org.apache.ignite.internal.tx.TxPriority; import org.apache.ignite.tx.IgniteTransactions; @@ -48,12 +50,18 @@ public IgniteTransactionsImpl(TxManager txManager, HybridTimestampTracker observ /** {@inheritDoc} */ @Override public Transaction begin(@Nullable TransactionOptions options) { - if (options != null && options.timeoutMillis() != 0) { + if (options != null && options.timeoutMillis() != 0 && !options.readOnly()) { // TODO: IGNITE-15936. - throw new UnsupportedOperationException("Timeouts are not supported yet"); + throw new UnsupportedOperationException("Timeouts are not supported yet for RW transactions."); } - return txManager.begin(observableTimestampTracker, false, options != null && options.readOnly()); + InternalTxOptions internalTxOptions = options == null + ? InternalTxOptions.defaults() + : InternalTxOptions.builder() + .timeoutMillis(options.timeoutMillis()) + .build(); + + return txManager.beginExplicit(observableTimestampTracker, options != null && options.readOnly(), internalTxOptions); } /** {@inheritDoc} */ @@ -62,8 +70,18 @@ public CompletableFuture beginAsync(@Nullable TransactionOptions op return CompletableFuture.completedFuture(begin(options)); } + /** + * Begins a transaction. + * + * @param readOnly {@code true} in order to start a read-only transaction, {@code false} in order to start read-write one. + * @return The started transaction. + */ + public InternalTransaction beginImplicit(boolean readOnly) { + return txManager.beginImplicit(observableTimestampTracker, readOnly); + } + @TestOnly public Transaction beginWithPriority(boolean readOnly, TxPriority priority) { - return txManager.begin(observableTimestampTracker, false, readOnly, priority); + return txManager.beginExplicit(observableTimestampTracker, readOnly, InternalTxOptions.defaultsWithPriority(priority)); } } diff --git a/modules/transactions/src/main/java/org/apache/ignite/internal/tx/impl/TransactionExpirationRegistry.java b/modules/transactions/src/main/java/org/apache/ignite/internal/tx/impl/TransactionExpirationRegistry.java new file mode 100644 index 00000000000..8e50f7d19de --- /dev/null +++ b/modules/transactions/src/main/java/org/apache/ignite/internal/tx/impl/TransactionExpirationRegistry.java @@ -0,0 +1,124 @@ +/* + * 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. + */ + +package org.apache.ignite.internal.tx.impl; + +import java.util.ArrayList; +import java.util.List; +import java.util.Map; +import java.util.NavigableMap; +import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.ConcurrentSkipListMap; +import java.util.concurrent.locks.ReadWriteLock; +import java.util.concurrent.locks.ReentrantReadWriteLock; +import org.apache.ignite.internal.hlc.HybridTimestamp; +import org.apache.ignite.internal.logger.IgniteLogger; +import org.apache.ignite.internal.logger.Loggers; +import org.apache.ignite.internal.tx.InternalTransaction; + +class TransactionExpirationRegistry { + private static final IgniteLogger LOG = Loggers.forClass(TransactionExpirationRegistry.class); + + private final NavigableMap> txsByExpirationTime = new ConcurrentSkipListMap<>(); + private final Map expirationTimeByTx = new ConcurrentHashMap<>(); + + private final ReadWriteLock watermarkLock = new ReentrantReadWriteLock(); + private volatile HybridTimestamp watermark = HybridTimestamp.MIN_VALUE; + + void register(InternalTransaction tx, HybridTimestamp txExpirationTime) { + if (isExpired(txExpirationTime)) { + abortTransaction(tx); + return; + } + + watermarkLock.readLock().lock(); + + try { + if (isExpired(txExpirationTime)) { + abortTransaction(tx); + return; + } + + Set txsExpiringAtTs = txsByExpirationTime.computeIfAbsent( + txExpirationTime, + k -> ConcurrentHashMap.newKeySet() + ); + txsExpiringAtTs.add(tx); + + expirationTimeByTx.put(tx, txExpirationTime); + } finally { + watermarkLock.readLock().unlock(); + } + } + + private boolean isExpired(HybridTimestamp expirationTime) { + return expirationTime.compareTo(watermark) <= 0; + } + + private static void abortTransaction(InternalTransaction tx) { + tx.rollbackAsync().whenComplete((res, ex) -> { + if (ex != null) { + LOG.error("Transaction abort due to timeout failed [txId={}]", ex, tx.id()); + } + }); + } + + void expireUpTo(HybridTimestamp expirationTime) { + List> transactionSetsToExpire; + + watermarkLock.writeLock().lock(); + + try { + NavigableMap> headMap = txsByExpirationTime.headMap(expirationTime, true); + transactionSetsToExpire = new ArrayList<>(headMap.values()); + headMap.clear(); + + watermark = expirationTime; + } finally { + watermarkLock.writeLock().unlock(); + } + + for (Set set : transactionSetsToExpire) { + for (InternalTransaction tx : set) { + expirationTimeByTx.remove(tx); + + abortTransaction(tx); + } + } + } + + void abortAllRegistered() { + expireUpTo(HybridTimestamp.MAX_VALUE); + } + + void unregister(InternalTransaction tx) { + HybridTimestamp expirationTime = expirationTimeByTx.remove(tx); + + if (expirationTime != null) { + txsByExpirationTime.compute(expirationTime, (k, set) -> { + if (set == null) { + return null; + } + + set.remove(tx); + + return set.isEmpty() ? null : set; + }); + } + } +} diff --git a/modules/transactions/src/main/java/org/apache/ignite/internal/tx/impl/TxManagerImpl.java b/modules/transactions/src/main/java/org/apache/ignite/internal/tx/impl/TxManagerImpl.java index abcd0a8e1d7..d50e937f316 100644 --- a/modules/transactions/src/main/java/org/apache/ignite/internal/tx/impl/TxManagerImpl.java +++ b/modules/transactions/src/main/java/org/apache/ignite/internal/tx/impl/TxManagerImpl.java @@ -21,6 +21,7 @@ import static java.util.concurrent.CompletableFuture.failedFuture; import static java.util.concurrent.CompletableFuture.runAsync; import static java.util.concurrent.CompletableFuture.supplyAsync; +import static java.util.concurrent.TimeUnit.MILLISECONDS; import static java.util.function.Function.identity; import static org.apache.ignite.internal.lang.IgniteStringFormatter.format; import static org.apache.ignite.internal.thread.ThreadOperation.STORAGE_READ; @@ -49,6 +50,8 @@ import java.util.concurrent.ExecutorService; import java.util.concurrent.ForkJoinPool; import java.util.concurrent.LinkedBlockingQueue; +import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.ScheduledFuture; import java.util.concurrent.ThreadPoolExecutor; import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeoutException; @@ -83,18 +86,19 @@ import org.apache.ignite.internal.replicator.message.ErrorReplicaResponse; import org.apache.ignite.internal.replicator.message.ReplicaMessageGroup; import org.apache.ignite.internal.replicator.message.ReplicaResponse; +import org.apache.ignite.internal.schema.configuration.LowWatermarkConfiguration; import org.apache.ignite.internal.systemview.api.SystemView; import org.apache.ignite.internal.systemview.api.SystemViewProvider; import org.apache.ignite.internal.thread.IgniteThreadFactory; import org.apache.ignite.internal.tx.HybridTimestampTracker; import org.apache.ignite.internal.tx.InternalTransaction; +import org.apache.ignite.internal.tx.InternalTxOptions; import org.apache.ignite.internal.tx.LocalRwTxCounter; import org.apache.ignite.internal.tx.LockManager; import org.apache.ignite.internal.tx.MismatchingTransactionOutcomeInternalException; import org.apache.ignite.internal.tx.TransactionMeta; import org.apache.ignite.internal.tx.TransactionResult; import org.apache.ignite.internal.tx.TxManager; -import org.apache.ignite.internal.tx.TxPriority; import org.apache.ignite.internal.tx.TxState; import org.apache.ignite.internal.tx.TxStateMeta; import org.apache.ignite.internal.tx.TxStateMetaFinishing; @@ -122,6 +126,8 @@ public class TxManagerImpl implements TxManager, NetworkMessageHandler, SystemVi /** Transaction configuration. */ private final TransactionConfiguration txConfig; + private final LowWatermarkConfiguration lowWatermarkConfig; + /** Lock manager. */ private final LockManager lockManager; @@ -197,14 +203,21 @@ public class TxManagerImpl implements TxManager, NetworkMessageHandler, SystemVi private final ReplicaService replicaService; + private final ScheduledExecutorService commonScheduler; + private final TransactionsViewProvider txViewProvider = new TransactionsViewProvider(); private volatile PersistentTxStateVacuumizer persistentTxStateVacuumizer; + private final TransactionExpirationRegistry transactionExpirationRegistry = new TransactionExpirationRegistry(); + + private volatile @Nullable ScheduledFuture transactionExpirationJobFuture; + /** * Test-only constructor. * * @param txConfig Transaction configuration. + * @param lowWatermarkConfig Low watermark configuration. * @param clusterService Cluster service. * @param replicaService Replica service. * @param lockManager Lock manager. @@ -220,6 +233,7 @@ public class TxManagerImpl implements TxManager, NetworkMessageHandler, SystemVi @TestOnly public TxManagerImpl( TransactionConfiguration txConfig, + LowWatermarkConfiguration lowWatermarkConfig, ClusterService clusterService, ReplicaService replicaService, LockManager lockManager, @@ -230,11 +244,13 @@ public TxManagerImpl( LocalRwTxCounter localRwTxCounter, RemotelyTriggeredResourceRegistry resourcesRegistry, TransactionInflights transactionInflights, - LowWatermark lowWatermark + LowWatermark lowWatermark, + ScheduledExecutorService commonScheduler ) { this( clusterService.nodeName(), txConfig, + lowWatermarkConfig, clusterService.messagingService(), clusterService.topologyService(), replicaService, @@ -247,7 +263,8 @@ public TxManagerImpl( ForkJoinPool.commonPool(), resourcesRegistry, transactionInflights, - lowWatermark + lowWatermark, + commonScheduler ); } @@ -255,6 +272,7 @@ public TxManagerImpl( * The constructor. * * @param txConfig Transaction configuration. + * @param lowWatermarkConfig Low watermark configuration. * @param messagingService Messaging service. * @param topologyService Topology service. * @param replicaService Replica service. @@ -272,6 +290,7 @@ public TxManagerImpl( public TxManagerImpl( String nodeName, TransactionConfiguration txConfig, + LowWatermarkConfiguration lowWatermarkConfig, MessagingService messagingService, TopologyService topologyService, ReplicaService replicaService, @@ -284,9 +303,11 @@ public TxManagerImpl( Executor partitionOperationsExecutor, RemotelyTriggeredResourceRegistry resourcesRegistry, TransactionInflights transactionInflights, - LowWatermark lowWatermark + LowWatermark lowWatermark, + ScheduledExecutorService commonScheduler ) { this.txConfig = txConfig; + this.lowWatermarkConfig = lowWatermarkConfig; this.lockManager = lockManager; this.clockService = clockService; this.transactionIdGenerator = transactionIdGenerator; @@ -301,6 +322,7 @@ public TxManagerImpl( this.transactionInflights = transactionInflights; this.lowWatermark = lowWatermark; this.replicaService = replicaService; + this.commonScheduler = commonScheduler; placementDriverHelper = new PlacementDriverHelper(placementDriver, clockService); @@ -310,7 +332,7 @@ public TxManagerImpl( cpus, cpus, 100, - TimeUnit.MILLISECONDS, + MILLISECONDS, new LinkedBlockingQueue<>(), IgniteThreadFactory.create(nodeName, "tx-async-write-intent", LOG, STORAGE_READ, STORAGE_WRITE) ); @@ -371,24 +393,32 @@ private CompletableFuture primaryReplicaExpiredListener(PrimaryReplicaE } @Override - public InternalTransaction begin(HybridTimestampTracker timestampTracker, boolean implicit) { - return begin(timestampTracker, implicit, false); + public InternalTransaction beginImplicit(HybridTimestampTracker timestampTracker, boolean readOnly) { + return begin(timestampTracker, true, readOnly, InternalTxOptions.defaults()); } @Override - public InternalTransaction begin(HybridTimestampTracker timestampTracker, boolean implicit, boolean readOnly) { - return begin(timestampTracker, implicit, readOnly, TxPriority.NORMAL); + public InternalTransaction beginExplicit(HybridTimestampTracker timestampTracker, boolean readOnly, InternalTxOptions txOptions) { + return begin(timestampTracker, false, readOnly, txOptions); } - @Override - public InternalTransaction begin( + private InternalTransaction begin( HybridTimestampTracker timestampTracker, boolean implicit, boolean readOnly, - TxPriority priority + InternalTxOptions options + ) { + return inBusyLock(busyLock, () -> beginBusy(timestampTracker, implicit, readOnly, options)); + } + + private InternalTransaction beginBusy( + HybridTimestampTracker timestampTracker, + boolean implicit, + boolean readOnly, + InternalTxOptions options ) { HybridTimestamp beginTimestamp = readOnly ? clockService.now() : createBeginTimestampWithIncrementRwTxCounter(); - UUID txId = transactionIdGenerator.transactionIdFor(beginTimestamp, priority); + UUID txId = transactionIdGenerator.transactionIdFor(beginTimestamp, options.priority()); startedTxs.add(1); @@ -396,8 +426,18 @@ public InternalTransaction begin( txStateVolatileStorage.initialize(txId, localNodeId); return new ReadWriteTransactionImpl(this, timestampTracker, txId, localNodeId, implicit); + } else { + return beginReadOnlyTransaction(timestampTracker, beginTimestamp, txId, implicit, options); } + } + private ReadOnlyTransactionImpl beginReadOnlyTransaction( + HybridTimestampTracker timestampTracker, + HybridTimestamp beginTimestamp, + UUID txId, + boolean implicit, + InternalTxOptions options + ) { HybridTimestamp observableTimestamp = timestampTracker.get(); HybridTimestamp readTimestamp = observableTimestamp != null @@ -415,17 +455,40 @@ public InternalTransaction begin( try { CompletableFuture txFuture = new CompletableFuture<>(); + + var transaction = new ReadOnlyTransactionImpl(this, timestampTracker, txId, localNodeId, implicit, readTimestamp, txFuture); + + // Implicit transactions are finished as soon as their operation/query is finished, they cannot be abandoned, so there is + // no need to register them. + if (!implicit) { + transactionExpirationRegistry.register(transaction, roExpirationTimeFor(beginTimestamp, options)); + } + txFuture.whenComplete((unused, throwable) -> { lowWatermark.unlock(txId); + + // We only register explicit transactions, so we only unregister them as well. + if (!implicit) { + transactionExpirationRegistry.unregister(transaction); + } }); - return new ReadOnlyTransactionImpl(this, timestampTracker, txId, localNodeId, implicit, readTimestamp, txFuture); + return transaction; } catch (Throwable t) { lowWatermark.unlock(txId); throw t; } } + private HybridTimestamp roExpirationTimeFor(HybridTimestamp beginTimestamp, InternalTxOptions options) { + long effectiveTimeoutMillis = options.timeoutMillis() == 0 ? defaultRoTransactionTimeout() : options.timeoutMillis(); + return beginTimestamp.addPhysicalTime(effectiveTimeoutMillis); + } + + private long defaultRoTransactionTimeout() { + return lowWatermarkConfig.dataAvailabilityTime().value(); + } + /** * Current read timestamp, for calculation of read timestamp of read-only transactions. * @@ -787,10 +850,23 @@ public CompletableFuture startAsync(ComponentContext componentContext) { placementDriver.listen(PrimaryReplicaEvent.PRIMARY_REPLICA_ELECTED, primaryReplicaElectedListener); + transactionExpirationJobFuture = commonScheduler.scheduleAtFixedRate(this::expireTransactionsUpToNow, 1000, 1000, MILLISECONDS); + return nullCompletedFuture(); }); } + private void expireTransactionsUpToNow() { + HybridTimestamp expirationTime = null; + + try { + expirationTime = clockService.current(); + transactionExpirationRegistry.expireUpTo(expirationTime); + } catch (Throwable t) { + LOG.error("Could not expire transactions up to {}", t, expirationTime); + } + } + @Override public void beforeNodeStop() { orphanDetector.stop(); @@ -812,6 +888,13 @@ public CompletableFuture stopAsync(ComponentContext componentContext) { placementDriver.removeListener(PrimaryReplicaEvent.PRIMARY_REPLICA_ELECTED, primaryReplicaElectedListener); + ScheduledFuture expirationJobFuture = transactionExpirationJobFuture; + if (expirationJobFuture != null) { + expirationJobFuture.cancel(false); + } + + transactionExpirationRegistry.abortAllRegistered(); + shutdownAndAwaitTermination(writeIntentSwitchPool, 10, TimeUnit.SECONDS); return nullCompletedFuture(); diff --git a/modules/transactions/src/test/java/org/apache/ignite/internal/tx/TxManagerTest.java b/modules/transactions/src/test/java/org/apache/ignite/internal/tx/TxManagerTest.java index 221bf446e46..8f7ed20b178 100644 --- a/modules/transactions/src/test/java/org/apache/ignite/internal/tx/TxManagerTest.java +++ b/modules/transactions/src/test/java/org/apache/ignite/internal/tx/TxManagerTest.java @@ -55,6 +55,7 @@ import java.util.UUID; import java.util.concurrent.CompletableFuture; +import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.TimeUnit; import java.util.function.LongSupplier; import java.util.stream.Stream; @@ -77,7 +78,10 @@ import org.apache.ignite.internal.replicator.ReplicaService; import org.apache.ignite.internal.replicator.TablePartitionId; import org.apache.ignite.internal.replicator.exception.PrimaryReplicaMissException; +import org.apache.ignite.internal.schema.configuration.LowWatermarkConfiguration; +import org.apache.ignite.internal.testframework.ExecutorServiceExtension; import org.apache.ignite.internal.testframework.IgniteAbstractTest; +import org.apache.ignite.internal.testframework.InjectExecutorService; import org.apache.ignite.internal.tx.configuration.TransactionConfiguration; import org.apache.ignite.internal.tx.impl.HeapLockManager; import org.apache.ignite.internal.tx.impl.PrimaryReplicaExpiredException; @@ -109,6 +113,7 @@ * Basic tests for a transaction manager. */ @ExtendWith(ConfigurationExtension.class) +@ExtendWith(ExecutorServiceExtension.class) public class TxManagerTest extends IgniteAbstractTest { private static final ClusterNode LOCAL_NODE = new ClusterNodeImpl(randomUUID(), "local", new NetworkAddress("127.0.0.1", 2004), null); @@ -134,6 +139,12 @@ public class TxManagerTest extends IgniteAbstractTest { @InjectConfiguration private TransactionConfiguration txConfiguration; + @InjectConfiguration + private LowWatermarkConfiguration lowWatermarkConfiguration; + + @InjectExecutorService + private ScheduledExecutorService commonScheduler; + private final LocalRwTxCounter localRwTxCounter = spy(new TestLocalRwTxCounter()); private final TestLowWatermark lowWatermark = spy(new TestLowWatermark()); @@ -156,6 +167,7 @@ public void setup() { txManager = new TxManagerImpl( txConfiguration, + lowWatermarkConfiguration, clusterService, replicaService, lockManager(), @@ -166,7 +178,8 @@ public void setup() { localRwTxCounter, resourceRegistry, transactionInflights, - lowWatermark + lowWatermark, + commonScheduler ); assertThat(txManager.startAsync(new ComponentContext()), willCompleteSuccessfully()); @@ -191,10 +204,10 @@ public void tearDown() { @Test public void testBegin() { - InternalTransaction tx0 = txManager.begin(hybridTimestampTracker, false); - InternalTransaction tx1 = txManager.begin(hybridTimestampTracker, false); - InternalTransaction tx2 = txManager.begin(hybridTimestampTracker, false, true); - InternalTransaction tx3 = txManager.begin(hybridTimestampTracker, false, true, TxPriority.NORMAL); + InternalTransaction tx0 = txManager.beginExplicitRw(hybridTimestampTracker, InternalTxOptions.defaults()); + InternalTransaction tx1 = txManager.beginExplicit(hybridTimestampTracker, true, InternalTxOptions.defaults()); + InternalTransaction tx2 = txManager.beginImplicitRw(hybridTimestampTracker); + InternalTransaction tx3 = txManager.beginImplicit(hybridTimestampTracker, true); assertNotNull(tx0.id()); assertNotNull(tx1.id()); @@ -202,8 +215,8 @@ public void testBegin() { assertNotNull(tx3.id()); assertFalse(tx0.isReadOnly()); - assertFalse(tx1.isReadOnly()); - assertTrue(tx2.isReadOnly()); + assertTrue(tx1.isReadOnly()); + assertFalse(tx2.isReadOnly()); assertTrue(tx3.isReadOnly()); } @@ -213,7 +226,7 @@ public void testEnlist() { assertEquals(LOCAL_NODE.address(), addr); - InternalTransaction tx = txManager.begin(hybridTimestampTracker, false); + InternalTransaction tx = txManager.beginExplicitRw(hybridTimestampTracker, InternalTxOptions.defaults()); TablePartitionId tablePartitionId = new TablePartitionId(1, 0); @@ -243,8 +256,10 @@ void testCreateNewRoTxAfterUpdateLowerWatermark() { assertThat(lowWatermark.updateAndNotify(new HybridTimestamp(10_000, 11)), willSucceedFast()); - IgniteInternalException exception = - assertThrows(IgniteInternalException.class, () -> txManager.begin(hybridTimestampTracker, false, true)); + IgniteInternalException exception = assertThrows( + IgniteInternalException.class, + () -> txManager.beginExplicit(hybridTimestampTracker, true, InternalTxOptions.defaults()) + ); assertEquals(Transactions.TX_READ_ONLY_TOO_OLD_ERR, exception.code()); } @@ -254,12 +269,12 @@ void testUpdateLowerWatermark() { // Let's check the absence of transactions. assertThat(lowWatermark.updateAndNotify(clockService.now()), willSucceedFast()); - InternalTransaction rwTx0 = txManager.begin(hybridTimestampTracker, false); + InternalTransaction rwTx0 = txManager.beginExplicitRw(hybridTimestampTracker, InternalTxOptions.defaults()); hybridTimestampTracker.update(clockService.now()); - InternalTransaction roTx0 = txManager.begin(hybridTimestampTracker, false, true); - InternalTransaction roTx1 = txManager.begin(hybridTimestampTracker, false, true); + InternalTransaction roTx0 = txManager.beginExplicit(hybridTimestampTracker, true, InternalTxOptions.defaults()); + InternalTransaction roTx1 = txManager.beginExplicit(hybridTimestampTracker, true, InternalTxOptions.defaults()); CompletableFuture readOnlyTxsFuture = lowWatermark.updateAndNotify(roTx1.readTimestamp()); assertFalse(readOnlyTxsFuture.isDone()); @@ -274,8 +289,8 @@ void testUpdateLowerWatermark() { assertTrue(readOnlyTxsFuture.isDone()); // Let's check only RW transactions. - txManager.begin(hybridTimestampTracker, false); - txManager.begin(hybridTimestampTracker, false); + txManager.beginExplicitRw(hybridTimestampTracker, InternalTxOptions.defaults()); + txManager.beginExplicitRw(hybridTimestampTracker, InternalTxOptions.defaults()); assertThat(lowWatermark.updateAndNotify(clockService.now()), willSucceedFast()); } @@ -291,7 +306,7 @@ public void testRepeatedCommitRollbackAfterCommit() throws Exception { when(replicaService.invoke(anyString(), any(TxFinishReplicaRequest.class))) .thenReturn(completedFuture(new TransactionResult(TxState.COMMITTED, commitTimestamp))); - InternalTransaction tx = txManager.begin(hybridTimestampTracker, false); + InternalTransaction tx = txManager.beginExplicitRw(hybridTimestampTracker, InternalTxOptions.defaults()); TablePartitionId tablePartitionId1 = new TablePartitionId(1, 0); @@ -312,7 +327,7 @@ public void testRepeatedCommitRollbackAfterRollback() throws Exception { when(replicaService.invoke(anyString(), any(TxFinishReplicaRequest.class))) .thenReturn(completedFuture(new TransactionResult(TxState.ABORTED, null))); - InternalTransaction tx = txManager.begin(hybridTimestampTracker, false); + InternalTransaction tx = txManager.beginExplicitRw(hybridTimestampTracker, InternalTxOptions.defaults()); TablePartitionId tablePartitionId1 = new TablePartitionId(1, 0); @@ -340,7 +355,7 @@ void testRepeatedCommitRollbackAfterCommitWithException() throws Exception { ) ))); - InternalTransaction tx = txManager.begin(hybridTimestampTracker, false); + InternalTransaction tx = txManager.beginExplicitRw(hybridTimestampTracker, InternalTxOptions.defaults()); TablePartitionId tablePartitionId1 = new TablePartitionId(1, 0); @@ -368,7 +383,7 @@ public void testRepeatedCommitRollbackAfterRollbackWithException() throws Except ) ))); - InternalTransaction tx = txManager.begin(hybridTimestampTracker, false); + InternalTransaction tx = txManager.beginExplicitRw(hybridTimestampTracker, InternalTxOptions.defaults()); TablePartitionId tablePartitionId1 = new TablePartitionId(1, 0); @@ -385,12 +400,12 @@ public void testRepeatedCommitRollbackAfterRollbackWithException() throws Except @ParameterizedTest @ValueSource(booleans = {true, false}) - public void testTestOnlyPendingCommit(boolean startReadOnlyTransaction) { + public void testOnlyPendingCommit(boolean startReadOnlyTransaction) { assertEquals(0, txManager.pending()); assertEquals(0, txManager.finished()); // Start transaction. - InternalTransaction tx = txManager.begin(hybridTimestampTracker, false, true); + InternalTransaction tx = txManager.beginExplicit(hybridTimestampTracker, true, InternalTxOptions.defaults()); assertEquals(1, txManager.pending()); assertEquals(0, txManager.finished()); @@ -412,14 +427,14 @@ public void testTestOnlyPendingCommit(boolean startReadOnlyTransaction) { @ParameterizedTest @ValueSource(booleans = {true, false}) - public void testTestOnlyPendingRollback(boolean startReadOnlyTransaction) { + public void testOnlyPendingRollback(boolean startReadOnlyTransaction) { assertEquals(0, txManager.pending()); assertEquals(0, txManager.finished()); // Start transaction. InternalTransaction tx = - startReadOnlyTransaction ? txManager.begin(hybridTimestampTracker, false, true) - : txManager.begin(hybridTimestampTracker, false); + startReadOnlyTransaction ? txManager.beginExplicit(hybridTimestampTracker, true, InternalTxOptions.defaults()) + : txManager.beginExplicitRw(hybridTimestampTracker, InternalTxOptions.defaults()); assertEquals(1, txManager.pending()); assertEquals(0, txManager.finished()); @@ -447,14 +462,14 @@ public void testObservableTimestamp() { HybridTimestamp now = clockService.now(); - InternalTransaction tx = txManager.begin(hybridTimestampTracker, false, true); + InternalTransaction tx = txManager.beginExplicit(hybridTimestampTracker, true, InternalTxOptions.defaults()); assertTrue(abs(now.getPhysical() - tx.readTimestamp().getPhysical()) > compareThreshold); tx.commit(); hybridTimestampTracker.update(now); - tx = txManager.begin(hybridTimestampTracker, false, true); + tx = txManager.beginExplicit(hybridTimestampTracker, true, InternalTxOptions.defaults()); assertTrue(abs(now.getPhysical() - tx.readTimestamp().getPhysical()) < compareThreshold); tx.commit(); @@ -468,7 +483,7 @@ public void testObservableTimestamp() { hybridTimestampTracker.update(timestampInPast); - tx = txManager.begin(hybridTimestampTracker, false, true); + tx = txManager.beginExplicit(hybridTimestampTracker, true, InternalTxOptions.defaults()); long readTime = now.getPhysical() - idleSafeTimePropagationPeriodMsSupplier.getAsLong() - clockService.maxClockSkewMillis(); @@ -485,7 +500,7 @@ public void testObservableTimestampLocally() { HybridTimestamp now = clockService.now(); - InternalTransaction tx = txManager.begin(hybridTimestampTracker, false, true); + InternalTransaction tx = txManager.beginExplicit(hybridTimestampTracker, true, InternalTxOptions.defaults()); HybridTimestamp firstReadTs = tx.readTimestamp(); @@ -495,7 +510,7 @@ public void testObservableTimestampLocally() { + idleSafeTimePropagationPeriodMsSupplier.getAsLong() + clockService.maxClockSkewMillis()); tx.commit(); - tx = txManager.begin(hybridTimestampTracker, false, true); + tx = txManager.beginExplicit(hybridTimestampTracker, true, InternalTxOptions.defaults()); assertTrue(firstReadTs.compareTo(tx.readTimestamp()) <= 0); @@ -607,7 +622,7 @@ public void testExpiredExceptionDoesNotShadeResponseExceptions() { @Test public void testOnlyPrimaryExpirationAffectsTransaction() { // Prepare transaction. - InternalTransaction tx = txManager.begin(hybridTimestampTracker, false); + InternalTransaction tx = txManager.beginExplicitRw(hybridTimestampTracker, InternalTxOptions.defaults()); ClusterNode node = mock(ClusterNode.class); @@ -687,7 +702,7 @@ public void testFinishExpiredWithDifferentEnlistmentConsistencyToken() { @ParameterizedTest(name = "readOnly = {0}") @ValueSource(booleans = {true, false}) void testIncrementLocalRwTxCounterOnBeginTransaction(boolean readOnly) { - InternalTransaction tx = txManager.begin(hybridTimestampTracker, false, readOnly); + InternalTransaction tx = txManager.beginExplicit(hybridTimestampTracker, readOnly, InternalTxOptions.defaults()); VerificationMode verificationMode = readOnly ? never() : times(1); @@ -698,7 +713,7 @@ void testIncrementLocalRwTxCounterOnBeginTransaction(boolean readOnly) { @ParameterizedTest(name = "readOnly = {0}, commit = {1}") @MethodSource("txTypeAndWayCompleteTx") void testDecrementLocalRwTxCounterOnCompleteTransaction(boolean readOnly, boolean commit) { - InternalTransaction tx = txManager.begin(hybridTimestampTracker, false, readOnly); + InternalTransaction tx = txManager.beginExplicit(hybridTimestampTracker, readOnly, InternalTxOptions.defaults()); clearInvocations(localRwTxCounter); @@ -745,11 +760,11 @@ void testCreateBeginTsInsideInUpdateRwTxCount() { return result; }).when(localRwTxCounter).inUpdateRwTxCountLock(any()); - txManager.begin(hybridTimestampTracker, false, false); + txManager.beginExplicit(hybridTimestampTracker, false, InternalTxOptions.defaults()); } private InternalTransaction prepareTransaction() { - InternalTransaction tx = txManager.begin(hybridTimestampTracker, false); + InternalTransaction tx = txManager.beginExplicitRw(hybridTimestampTracker, InternalTxOptions.defaults()); TablePartitionId tablePartitionId1 = new TablePartitionId(1, 0); diff --git a/modules/transactions/src/test/java/org/apache/ignite/internal/tx/impl/TransactionExpirationRegistryTest.java b/modules/transactions/src/test/java/org/apache/ignite/internal/tx/impl/TransactionExpirationRegistryTest.java new file mode 100644 index 00000000000..6cdb0e2e227 --- /dev/null +++ b/modules/transactions/src/test/java/org/apache/ignite/internal/tx/impl/TransactionExpirationRegistryTest.java @@ -0,0 +1,155 @@ +/* + * 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. + */ + +package org.apache.ignite.internal.tx.impl; + +import static org.apache.ignite.internal.util.CompletableFutures.nullCompletedFuture; +import static org.mockito.Mockito.lenient; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; + +import org.apache.ignite.internal.hlc.HybridTimestamp; +import org.apache.ignite.internal.testframework.BaseIgniteAbstractTest; +import org.apache.ignite.internal.tx.InternalTransaction; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; + +@ExtendWith(MockitoExtension.class) +class TransactionExpirationRegistryTest extends BaseIgniteAbstractTest { + private final TransactionExpirationRegistry registry = new TransactionExpirationRegistry(); + + @Mock + private InternalTransaction tx1; + + @Mock + private InternalTransaction tx2; + + @BeforeEach + void configureMocks() { + lenient().when(tx1.rollbackAsync()).thenReturn(nullCompletedFuture()); + lenient().when(tx2.rollbackAsync()).thenReturn(nullCompletedFuture()); + } + + @Test + void abortsTransactionsBeforeExpirationTime() { + registry.register(tx1, new HybridTimestamp(1000, 0)); + registry.register(tx2, new HybridTimestamp(2000, 0)); + + registry.expireUpTo(new HybridTimestamp(3000, 0)); + + verify(tx1).rollbackAsync(); + verify(tx2).rollbackAsync(); + } + + @Test + void abortsTransactionsExactlyOnExpirationTime() { + registry.register(tx1, new HybridTimestamp(1000, 0)); + + registry.expireUpTo(new HybridTimestamp(1000, 0)); + + verify(tx1).rollbackAsync(); + } + + @Test + void doesNotAbortTransactionsAfterExpirationTime() { + registry.register(tx1, new HybridTimestamp(1000, 1)); + + registry.expireUpTo(new HybridTimestamp(1000, 0)); + + verify(tx1, never()).rollbackAsync(); + } + + @Test + void abortsTransactionsExpiredAfterFewExpirations() { + registry.register(tx1, new HybridTimestamp(1000, 1)); + + registry.expireUpTo(new HybridTimestamp(1000, 0)); + registry.expireUpTo(new HybridTimestamp(2000, 0)); + + verify(tx1).rollbackAsync(); + } + + @Test + void abortsAlreadyExpiredTransactionOnRegistration() { + registry.expireUpTo(new HybridTimestamp(2000, 0)); + + registry.register(tx1, new HybridTimestamp(1000, 0)); + registry.register(tx2, new HybridTimestamp(2000, 0)); + + verify(tx1).rollbackAsync(); + verify(tx2).rollbackAsync(); + } + + @Test + void abortsAlreadyExpiredTransactionJustOnce() { + registry.expireUpTo(new HybridTimestamp(2000, 0)); + + registry.register(tx1, new HybridTimestamp(1000, 0)); + registry.register(tx2, new HybridTimestamp(2000, 0)); + + registry.expireUpTo(new HybridTimestamp(2000, 0)); + + verify(tx1, times(1)).rollbackAsync(); + verify(tx2, times(1)).rollbackAsync(); + } + + @Test + void abortsAllRegistered() { + registry.register(tx1, new HybridTimestamp(1000, 0)); + registry.register(tx2, HybridTimestamp.MAX_VALUE); + + registry.abortAllRegistered(); + + verify(tx1).rollbackAsync(); + verify(tx2).rollbackAsync(); + } + + @Test + void abortsOnRegistrationAfterAbortingAllRegistered() { + registry.abortAllRegistered(); + + registry.register(tx1, new HybridTimestamp(1000, 0)); + registry.register(tx2, HybridTimestamp.MAX_VALUE); + + verify(tx1).rollbackAsync(); + verify(tx2).rollbackAsync(); + } + + @Test + void removesTransactionOnUnregister() { + registry.register(tx1, new HybridTimestamp(1000, 0)); + + registry.unregister(tx1); + + registry.expireUpTo(new HybridTimestamp(2000, 0)); + + // Should not be aborted due to expiration as we removed the transaction. + verify(tx1, never()).rollbackAsync(); + } + + @Test + void unregisterIsIdempotent() { + registry.register(tx1, new HybridTimestamp(1000, 0)); + + registry.unregister(tx1); + registry.unregister(tx1); + } +} From 63e52f99ef46559f87382eeace0be37dcbccdbf8 Mon Sep 17 00:00:00 2001 From: Roman Puchkovskiy Date: Thu, 16 Jan 2025 14:45:19 +0400 Subject: [PATCH 02/18] IGNITE-23747 / use transaction.timeout --- .../org/apache/ignite/tx/TransactionOptions.java | 3 +-- .../rebalance/ItRebalanceDistributedTest.java | 5 ----- .../replicator/ItReplicaLifecycleTest.java | 5 ----- .../runner/app/ItIgniteNodeRestartTest.java | 5 ----- .../apache/ignite/internal/app/IgniteImpl.java | 1 - .../exec/rel/TableScanNodeExecutionTest.java | 5 ----- .../ignite/distributed/ItLockTableTest.java | 6 ------ .../ItTxDistributedCleanupRecoveryTest.java | 1 - ...DistributedTestSingleNodeNoCleanupMessage.java | 2 -- .../ignite/distributed/ItTxStateLocalMapTest.java | 5 ----- .../ignite/internal/table/ItColocationTest.java | 5 ----- .../ignite/distributed/ItTxTestCluster.java | 7 ------- .../internal/table/TxInfrastructureTest.java | 5 ----- .../table/impl/DummyInternalTableImpl.java | 8 -------- .../TransactionConfigurationSchema.java | 11 ++++++++--- .../ignite/internal/tx/impl/TxManagerImpl.java | 15 +++------------ .../apache/ignite/internal/tx/TxManagerTest.java | 5 ----- 17 files changed, 12 insertions(+), 82 deletions(-) diff --git a/modules/api/src/main/java/org/apache/ignite/tx/TransactionOptions.java b/modules/api/src/main/java/org/apache/ignite/tx/TransactionOptions.java index 1404822e0e3..5f745572062 100644 --- a/modules/api/src/main/java/org/apache/ignite/tx/TransactionOptions.java +++ b/modules/api/src/main/java/org/apache/ignite/tx/TransactionOptions.java @@ -40,8 +40,7 @@ public long timeoutMillis() { * Sets transaction timeout, in milliseconds. * * @param timeoutMillis Transaction timeout, in milliseconds. Cannot be negative; 0 means 'use default timeout'. - * For RO transactions, the default timeout is data availability time configured via ignite.gc.lowWatermark.dataAvailabilityTime - * configuration setting. + * For RO transactions, the default timeout is configured via ignite.transaction.timeout configuration property. * For RW transactions, timeouts are not supported yet. TODO: IGNITE-15936 * @return {@code this} for chaining. */ diff --git a/modules/distribution-zones/src/integrationTest/java/org/apache/ignite/internal/rebalance/ItRebalanceDistributedTest.java b/modules/distribution-zones/src/integrationTest/java/org/apache/ignite/internal/rebalance/ItRebalanceDistributedTest.java index c27368ba4bd..f73d7f4db81 100644 --- a/modules/distribution-zones/src/integrationTest/java/org/apache/ignite/internal/rebalance/ItRebalanceDistributedTest.java +++ b/modules/distribution-zones/src/integrationTest/java/org/apache/ignite/internal/rebalance/ItRebalanceDistributedTest.java @@ -203,7 +203,6 @@ import org.apache.ignite.internal.schema.configuration.GcConfiguration; import org.apache.ignite.internal.schema.configuration.GcExtensionConfiguration; import org.apache.ignite.internal.schema.configuration.GcExtensionConfigurationSchema; -import org.apache.ignite.internal.schema.configuration.LowWatermarkConfiguration; import org.apache.ignite.internal.schema.configuration.StorageUpdateConfiguration; import org.apache.ignite.internal.schema.configuration.StorageUpdateExtensionConfiguration; import org.apache.ignite.internal.schema.configuration.StorageUpdateExtensionConfigurationSchema; @@ -300,9 +299,6 @@ public class ItRebalanceDistributedTest extends BaseIgniteAbstractTest { @InjectConfiguration private TransactionConfiguration txConfiguration; - @InjectConfiguration - private LowWatermarkConfiguration lowWatermarkConfiguration; - @InjectConfiguration private RaftConfiguration raftConfiguration; @@ -1405,7 +1401,6 @@ private class Node { txManager = new TxManagerImpl( txConfiguration, - lowWatermarkConfiguration, clusterService, replicaSvc, lockManager, diff --git a/modules/partition-replicator/src/integrationTest/java/org/apache/ignite/internal/partition/replicator/ItReplicaLifecycleTest.java b/modules/partition-replicator/src/integrationTest/java/org/apache/ignite/internal/partition/replicator/ItReplicaLifecycleTest.java index 65a02b42899..972dd217c59 100644 --- a/modules/partition-replicator/src/integrationTest/java/org/apache/ignite/internal/partition/replicator/ItReplicaLifecycleTest.java +++ b/modules/partition-replicator/src/integrationTest/java/org/apache/ignite/internal/partition/replicator/ItReplicaLifecycleTest.java @@ -170,7 +170,6 @@ import org.apache.ignite.internal.schema.configuration.GcConfiguration; import org.apache.ignite.internal.schema.configuration.GcExtensionConfiguration; import org.apache.ignite.internal.schema.configuration.GcExtensionConfigurationSchema; -import org.apache.ignite.internal.schema.configuration.LowWatermarkConfiguration; import org.apache.ignite.internal.schema.configuration.StorageUpdateConfiguration; import org.apache.ignite.internal.schema.configuration.StorageUpdateExtensionConfiguration; import org.apache.ignite.internal.schema.configuration.StorageUpdateExtensionConfigurationSchema; @@ -251,9 +250,6 @@ public class ItReplicaLifecycleTest extends BaseIgniteAbstractTest { @InjectConfiguration private static TransactionConfiguration txConfiguration; - @InjectConfiguration - private static LowWatermarkConfiguration lowWatermarkConfiguration; - @InjectConfiguration private static RaftConfiguration raftConfiguration; @@ -1231,7 +1227,6 @@ public CompletableFuture invoke( txManager = new TxManagerImpl( txConfiguration, - lowWatermarkConfiguration, clusterService, replicaSvc, lockManager, diff --git a/modules/runner/src/integrationTest/java/org/apache/ignite/internal/runner/app/ItIgniteNodeRestartTest.java b/modules/runner/src/integrationTest/java/org/apache/ignite/internal/runner/app/ItIgniteNodeRestartTest.java index 4aac956529b..8b4e1c15986 100644 --- a/modules/runner/src/integrationTest/java/org/apache/ignite/internal/runner/app/ItIgniteNodeRestartTest.java +++ b/modules/runner/src/integrationTest/java/org/apache/ignite/internal/runner/app/ItIgniteNodeRestartTest.java @@ -189,7 +189,6 @@ import org.apache.ignite.internal.schema.SchemaManager; import org.apache.ignite.internal.schema.configuration.GcConfiguration; import org.apache.ignite.internal.schema.configuration.GcExtensionConfiguration; -import org.apache.ignite.internal.schema.configuration.LowWatermarkConfiguration; import org.apache.ignite.internal.schema.configuration.StorageUpdateConfiguration; import org.apache.ignite.internal.sql.api.IgniteSqlImpl; import org.apache.ignite.internal.sql.configuration.distributed.SqlClusterExtensionConfiguration; @@ -296,9 +295,6 @@ public class ItIgniteNodeRestartTest extends BaseIgniteRestartTest { @InjectConfiguration private static TransactionConfiguration txConfiguration; - @InjectConfiguration - private static LowWatermarkConfiguration lowWatermarkConfiguration; - @InjectConfiguration private static StorageUpdateConfiguration storageUpdateConfiguration; @@ -623,7 +619,6 @@ public CompletableFuture invoke(Condition condition, List su var txManager = new TxManagerImpl( name, txConfiguration, - lowWatermarkConfiguration, messagingServiceReturningToStorageOperationsPool, clusterSvc.topologyService(), replicaService, diff --git a/modules/runner/src/main/java/org/apache/ignite/internal/app/IgniteImpl.java b/modules/runner/src/main/java/org/apache/ignite/internal/app/IgniteImpl.java index 9ea0d075490..843f9f14647 100644 --- a/modules/runner/src/main/java/org/apache/ignite/internal/app/IgniteImpl.java +++ b/modules/runner/src/main/java/org/apache/ignite/internal/app/IgniteImpl.java @@ -978,7 +978,6 @@ public class IgniteImpl implements Ignite { txManager = new TxManagerImpl( name, txConfig, - gcConfig.lowWatermark(), messagingServiceReturningToStorageOperationsPool, clusterSvc.topologyService(), replicaSvc, diff --git a/modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/exec/rel/TableScanNodeExecutionTest.java b/modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/exec/rel/TableScanNodeExecutionTest.java index 5f112f79892..c7b780f271b 100644 --- a/modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/exec/rel/TableScanNodeExecutionTest.java +++ b/modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/exec/rel/TableScanNodeExecutionTest.java @@ -61,7 +61,6 @@ import org.apache.ignite.internal.schema.BinaryRow; import org.apache.ignite.internal.schema.BinaryRowEx; import org.apache.ignite.internal.schema.BinaryTuplePrefix; -import org.apache.ignite.internal.schema.configuration.LowWatermarkConfiguration; import org.apache.ignite.internal.sql.engine.exec.ExecutionContext; import org.apache.ignite.internal.sql.engine.exec.PartitionProvider; import org.apache.ignite.internal.sql.engine.exec.PartitionWithConsistencyToken; @@ -108,9 +107,6 @@ public class TableScanNodeExecutionTest extends AbstractExecutionTest @InjectConfiguration private TransactionConfiguration txConfiguration; - @InjectConfiguration - private LowWatermarkConfiguration lowWatermarkConfiguration; - @InjectExecutorService private ScheduledExecutorService commonExecutor; @@ -172,7 +168,6 @@ public void testScanNodeDataPropagation() throws InterruptedException { TxManagerImpl txManager = new TxManagerImpl( txConfiguration, - lowWatermarkConfiguration, clusterService, replicaSvc, HeapLockManager.smallInstance(), diff --git a/modules/table/src/integrationTest/java/org/apache/ignite/distributed/ItLockTableTest.java b/modules/table/src/integrationTest/java/org/apache/ignite/distributed/ItLockTableTest.java index c4ecbbc7d37..547bcaf9b67 100644 --- a/modules/table/src/integrationTest/java/org/apache/ignite/distributed/ItLockTableTest.java +++ b/modules/table/src/integrationTest/java/org/apache/ignite/distributed/ItLockTableTest.java @@ -38,7 +38,6 @@ import org.apache.ignite.internal.schema.Column; import org.apache.ignite.internal.schema.SchemaDescriptor; import org.apache.ignite.internal.schema.configuration.GcConfiguration; -import org.apache.ignite.internal.schema.configuration.LowWatermarkConfiguration; import org.apache.ignite.internal.schema.configuration.StorageUpdateConfiguration; import org.apache.ignite.internal.table.TableViewInternal; import org.apache.ignite.internal.testframework.ExecutorServiceExtension; @@ -100,9 +99,6 @@ public class ItLockTableTest extends IgniteAbstractTest { @InjectConfiguration("mock: { deadlockPreventionPolicy: { waitTimeout: -1, txIdComparator: NONE } }") protected static TransactionConfiguration txConfiguration; - @InjectConfiguration - protected LowWatermarkConfiguration lowWatermarkConfiguration; - @InjectConfiguration protected static ReplicationConfiguration replicationConfiguration; @@ -131,7 +127,6 @@ public void before() throws Exception { testInfo, raftConfiguration, txConfiguration, - lowWatermarkConfiguration, storageUpdateConfiguration, workDir, 1, @@ -154,7 +149,6 @@ protected TxManagerImpl newTxManager( ) { return new TxManagerImpl( txConfiguration, - lowWatermarkConfiguration, clusterService, replicaSvc, new HeapLockManager( diff --git a/modules/table/src/integrationTest/java/org/apache/ignite/distributed/ItTxDistributedCleanupRecoveryTest.java b/modules/table/src/integrationTest/java/org/apache/ignite/distributed/ItTxDistributedCleanupRecoveryTest.java index a981b630f50..a1fa4e0e1dc 100644 --- a/modules/table/src/integrationTest/java/org/apache/ignite/distributed/ItTxDistributedCleanupRecoveryTest.java +++ b/modules/table/src/integrationTest/java/org/apache/ignite/distributed/ItTxDistributedCleanupRecoveryTest.java @@ -54,7 +54,6 @@ public void before() throws Exception { testInfo, raftConfiguration, txConfiguration, - lowWatermarkConfiguration, storageUpdateConfiguration, workDir, nodes(), diff --git a/modules/table/src/integrationTest/java/org/apache/ignite/distributed/ItTxDistributedTestSingleNodeNoCleanupMessage.java b/modules/table/src/integrationTest/java/org/apache/ignite/distributed/ItTxDistributedTestSingleNodeNoCleanupMessage.java index f347a8d81af..ec145a04a75 100644 --- a/modules/table/src/integrationTest/java/org/apache/ignite/distributed/ItTxDistributedTestSingleNodeNoCleanupMessage.java +++ b/modules/table/src/integrationTest/java/org/apache/ignite/distributed/ItTxDistributedTestSingleNodeNoCleanupMessage.java @@ -98,7 +98,6 @@ public void before() throws Exception { testInfo, raftConfiguration, txConfiguration, - lowWatermarkConfiguration, storageUpdateConfiguration, workDir, nodes(), @@ -121,7 +120,6 @@ protected TxManagerImpl newTxManager( ) { return new TxManagerImpl( txConfiguration, - lowWatermarkConfiguration, clusterService, replicaSvc, new HeapLockManager(), diff --git a/modules/table/src/integrationTest/java/org/apache/ignite/distributed/ItTxStateLocalMapTest.java b/modules/table/src/integrationTest/java/org/apache/ignite/distributed/ItTxStateLocalMapTest.java index 04adec949c8..2b6ad33a8cf 100644 --- a/modules/table/src/integrationTest/java/org/apache/ignite/distributed/ItTxStateLocalMapTest.java +++ b/modules/table/src/integrationTest/java/org/apache/ignite/distributed/ItTxStateLocalMapTest.java @@ -36,7 +36,6 @@ import org.apache.ignite.internal.replicator.configuration.ReplicationConfiguration; import org.apache.ignite.internal.schema.Column; import org.apache.ignite.internal.schema.SchemaDescriptor; -import org.apache.ignite.internal.schema.configuration.LowWatermarkConfiguration; import org.apache.ignite.internal.schema.configuration.StorageUpdateConfiguration; import org.apache.ignite.internal.table.TableViewInternal; import org.apache.ignite.internal.testframework.IgniteAbstractTest; @@ -70,9 +69,6 @@ public class ItTxStateLocalMapTest extends IgniteAbstractTest { @InjectConfiguration private TransactionConfiguration txConfiguration; - @InjectConfiguration - protected LowWatermarkConfiguration lowWatermarkConfiguration; - @InjectConfiguration private StorageUpdateConfiguration storageUpdateConfiguration; @@ -106,7 +102,6 @@ public void before() throws Exception { testInfo, raftConfig, txConfiguration, - lowWatermarkConfiguration, storageUpdateConfiguration, workDir, NODES, diff --git a/modules/table/src/integrationTest/java/org/apache/ignite/internal/table/ItColocationTest.java b/modules/table/src/integrationTest/java/org/apache/ignite/internal/table/ItColocationTest.java index ee91ee9e5e8..c6a8f5f6625 100644 --- a/modules/table/src/integrationTest/java/org/apache/ignite/internal/table/ItColocationTest.java +++ b/modules/table/src/integrationTest/java/org/apache/ignite/internal/table/ItColocationTest.java @@ -91,7 +91,6 @@ import org.apache.ignite.internal.schema.NullBinaryRow; import org.apache.ignite.internal.schema.SchemaDescriptor; import org.apache.ignite.internal.schema.SchemaRegistry; -import org.apache.ignite.internal.schema.configuration.LowWatermarkConfiguration; import org.apache.ignite.internal.schema.marshaller.TupleMarshallerImpl; import org.apache.ignite.internal.schema.row.Row; import org.apache.ignite.internal.sql.engine.util.SqlTestUtils; @@ -164,9 +163,6 @@ public class ItColocationTest extends BaseIgniteAbstractTest { @InjectConfiguration private static TransactionConfiguration txConfiguration; - @InjectConfiguration - private static LowWatermarkConfiguration lowWatermarkConfiguration; - @InjectExecutorService private static ScheduledExecutorService commonExecutor; @@ -199,7 +195,6 @@ static void beforeAllTests() { txManager = new TxManagerImpl( txConfiguration, - lowWatermarkConfiguration, clusterService, replicaService, new HeapLockManager(), diff --git a/modules/table/src/testFixtures/java/org/apache/ignite/distributed/ItTxTestCluster.java b/modules/table/src/testFixtures/java/org/apache/ignite/distributed/ItTxTestCluster.java index aa5d92b618b..49ab15b1577 100644 --- a/modules/table/src/testFixtures/java/org/apache/ignite/distributed/ItTxTestCluster.java +++ b/modules/table/src/testFixtures/java/org/apache/ignite/distributed/ItTxTestCluster.java @@ -123,7 +123,6 @@ import org.apache.ignite.internal.schema.SchemaDescriptor; import org.apache.ignite.internal.schema.SchemaRegistry; import org.apache.ignite.internal.schema.SchemaSyncService; -import org.apache.ignite.internal.schema.configuration.LowWatermarkConfiguration; import org.apache.ignite.internal.schema.configuration.StorageUpdateConfiguration; import org.apache.ignite.internal.storage.MvPartitionStorage; import org.apache.ignite.internal.storage.engine.MvTableStorage; @@ -199,8 +198,6 @@ public class ItTxTestCluster { private final TransactionConfiguration txConfiguration; - private final LowWatermarkConfiguration lowWatermarkConfiguration; - private final StorageUpdateConfiguration storageUpdateConfiguration; private final Path workDir; @@ -323,7 +320,6 @@ public ItTxTestCluster( TestInfo testInfo, RaftConfiguration raftConfig, TransactionConfiguration txConfiguration, - LowWatermarkConfiguration lowWatermarkConfiguration, StorageUpdateConfiguration storageUpdateConfiguration, Path workDir, int nodes, @@ -334,7 +330,6 @@ public ItTxTestCluster( ) { this.raftConfig = raftConfig; this.txConfiguration = txConfiguration; - this.lowWatermarkConfiguration = lowWatermarkConfiguration; this.storageUpdateConfiguration = storageUpdateConfiguration; this.workDir = workDir; this.nodes = nodes; @@ -578,7 +573,6 @@ protected TxManagerImpl newTxManager( return new TxManagerImpl( node.name(), txConfiguration, - lowWatermarkConfiguration, clusterService.messagingService(), clusterService.topologyService(), replicaSvc, @@ -1069,7 +1063,6 @@ private void initializeClientTxComponents() { clientTxManager = new TxManagerImpl( "client", txConfiguration, - lowWatermarkConfiguration, client.messagingService(), client.topologyService(), clientReplicaSvc, diff --git a/modules/table/src/testFixtures/java/org/apache/ignite/internal/table/TxInfrastructureTest.java b/modules/table/src/testFixtures/java/org/apache/ignite/internal/table/TxInfrastructureTest.java index 86ae76efd0a..106c798e9eb 100644 --- a/modules/table/src/testFixtures/java/org/apache/ignite/internal/table/TxInfrastructureTest.java +++ b/modules/table/src/testFixtures/java/org/apache/ignite/internal/table/TxInfrastructureTest.java @@ -47,7 +47,6 @@ import org.apache.ignite.internal.replicator.configuration.ReplicationConfiguration; import org.apache.ignite.internal.schema.Column; import org.apache.ignite.internal.schema.SchemaDescriptor; -import org.apache.ignite.internal.schema.configuration.LowWatermarkConfiguration; import org.apache.ignite.internal.schema.configuration.StorageUpdateConfiguration; import org.apache.ignite.internal.storage.MvPartitionStorage; import org.apache.ignite.internal.table.distributed.raft.PartitionListener; @@ -118,9 +117,6 @@ public abstract class TxInfrastructureTest extends IgniteAbstractTest { @InjectConfiguration protected TransactionConfiguration txConfiguration; - @InjectConfiguration - protected LowWatermarkConfiguration lowWatermarkConfiguration; - @InjectConfiguration protected StorageUpdateConfiguration storageUpdateConfiguration; @@ -177,7 +173,6 @@ public void before() throws Exception { testInfo, raftConfiguration, txConfiguration, - lowWatermarkConfiguration, storageUpdateConfiguration, workDir, nodes(), diff --git a/modules/table/src/testFixtures/java/org/apache/ignite/internal/table/impl/DummyInternalTableImpl.java b/modules/table/src/testFixtures/java/org/apache/ignite/internal/table/impl/DummyInternalTableImpl.java index f977f6db406..7047fe6f9c8 100644 --- a/modules/table/src/testFixtures/java/org/apache/ignite/internal/table/impl/DummyInternalTableImpl.java +++ b/modules/table/src/testFixtures/java/org/apache/ignite/internal/table/impl/DummyInternalTableImpl.java @@ -39,7 +39,6 @@ import java.util.concurrent.CompletableFuture; import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicLong; -import org.apache.ignite.configuration.ConfigurationValue; import org.apache.ignite.distributed.TestPartitionDataStorage; import org.apache.ignite.internal.TestHybridClock; import org.apache.ignite.internal.catalog.CatalogService; @@ -85,7 +84,6 @@ import org.apache.ignite.internal.schema.BinaryRowEx; import org.apache.ignite.internal.schema.ColumnsExtractor; import org.apache.ignite.internal.schema.SchemaDescriptor; -import org.apache.ignite.internal.schema.configuration.LowWatermarkConfiguration; import org.apache.ignite.internal.schema.configuration.StorageUpdateConfiguration; import org.apache.ignite.internal.storage.MvPartitionStorage; import org.apache.ignite.internal.storage.engine.MvTableStorage; @@ -561,14 +559,8 @@ public static TxManagerImpl txManager( TransactionInflights transactionInflights = new TransactionInflights(placementDriver, CLOCK_SERVICE); - LowWatermarkConfiguration lowWatermarkConfiguration = mock(LowWatermarkConfiguration.class); - ConfigurationValue dataAvailabilityTime = mock(ConfigurationValue.class); - lenient().when(dataAvailabilityTime.value()).thenReturn(15L * 60 * 1000); - lenient().when(lowWatermarkConfiguration.dataAvailabilityTime()).thenReturn(dataAvailabilityTime); - var txManager = new TxManagerImpl( txConfiguration, - lowWatermarkConfiguration, clusterService, replicaSvc, HeapLockManager.smallInstance(), diff --git a/modules/transactions/src/main/java/org/apache/ignite/internal/tx/configuration/TransactionConfigurationSchema.java b/modules/transactions/src/main/java/org/apache/ignite/internal/tx/configuration/TransactionConfigurationSchema.java index 0a619c7b905..1ec9d1ae74a 100644 --- a/modules/transactions/src/main/java/org/apache/ignite/internal/tx/configuration/TransactionConfigurationSchema.java +++ b/modules/transactions/src/main/java/org/apache/ignite/internal/tx/configuration/TransactionConfigurationSchema.java @@ -31,13 +31,18 @@ public class TransactionConfigurationSchema { /** Default checking transaction interval. */ public static final long DEFAULT_ABANDONED_CHECK_TS = 5_000; - /** Checking transaction interval. */ + /** Checking transaction interval (milliseconds). */ @Range(min = 0) @Value(hasDefault = true) public final long abandonedCheckTs = DEFAULT_ABANDONED_CHECK_TS; - /** Timeout for implicit transactions. */ - @Range(min = 0) + /** Default transaction timeout (milliseconds). */ + @Range(min = 1) + @Value(hasDefault = true) + public final long timeout = 3_000; + + /** Timeout for implicit transactions (milliseconds). */ + @Range(min = 1) @Value(hasDefault = true) public final long implicitTransactionTimeout = 3_000; diff --git a/modules/transactions/src/main/java/org/apache/ignite/internal/tx/impl/TxManagerImpl.java b/modules/transactions/src/main/java/org/apache/ignite/internal/tx/impl/TxManagerImpl.java index d50e937f316..b7e7fbb2368 100644 --- a/modules/transactions/src/main/java/org/apache/ignite/internal/tx/impl/TxManagerImpl.java +++ b/modules/transactions/src/main/java/org/apache/ignite/internal/tx/impl/TxManagerImpl.java @@ -86,7 +86,6 @@ import org.apache.ignite.internal.replicator.message.ErrorReplicaResponse; import org.apache.ignite.internal.replicator.message.ReplicaMessageGroup; import org.apache.ignite.internal.replicator.message.ReplicaResponse; -import org.apache.ignite.internal.schema.configuration.LowWatermarkConfiguration; import org.apache.ignite.internal.systemview.api.SystemView; import org.apache.ignite.internal.systemview.api.SystemViewProvider; import org.apache.ignite.internal.thread.IgniteThreadFactory; @@ -126,8 +125,6 @@ public class TxManagerImpl implements TxManager, NetworkMessageHandler, SystemVi /** Transaction configuration. */ private final TransactionConfiguration txConfig; - private final LowWatermarkConfiguration lowWatermarkConfig; - /** Lock manager. */ private final LockManager lockManager; @@ -217,7 +214,6 @@ public class TxManagerImpl implements TxManager, NetworkMessageHandler, SystemVi * Test-only constructor. * * @param txConfig Transaction configuration. - * @param lowWatermarkConfig Low watermark configuration. * @param clusterService Cluster service. * @param replicaService Replica service. * @param lockManager Lock manager. @@ -233,7 +229,6 @@ public class TxManagerImpl implements TxManager, NetworkMessageHandler, SystemVi @TestOnly public TxManagerImpl( TransactionConfiguration txConfig, - LowWatermarkConfiguration lowWatermarkConfig, ClusterService clusterService, ReplicaService replicaService, LockManager lockManager, @@ -250,7 +245,6 @@ public TxManagerImpl( this( clusterService.nodeName(), txConfig, - lowWatermarkConfig, clusterService.messagingService(), clusterService.topologyService(), replicaService, @@ -272,7 +266,6 @@ public TxManagerImpl( * The constructor. * * @param txConfig Transaction configuration. - * @param lowWatermarkConfig Low watermark configuration. * @param messagingService Messaging service. * @param topologyService Topology service. * @param replicaService Replica service. @@ -290,7 +283,6 @@ public TxManagerImpl( public TxManagerImpl( String nodeName, TransactionConfiguration txConfig, - LowWatermarkConfiguration lowWatermarkConfig, MessagingService messagingService, TopologyService topologyService, ReplicaService replicaService, @@ -307,7 +299,6 @@ public TxManagerImpl( ScheduledExecutorService commonScheduler ) { this.txConfig = txConfig; - this.lowWatermarkConfig = lowWatermarkConfig; this.lockManager = lockManager; this.clockService = clockService; this.transactionIdGenerator = transactionIdGenerator; @@ -481,12 +472,12 @@ private ReadOnlyTransactionImpl beginReadOnlyTransaction( } private HybridTimestamp roExpirationTimeFor(HybridTimestamp beginTimestamp, InternalTxOptions options) { - long effectiveTimeoutMillis = options.timeoutMillis() == 0 ? defaultRoTransactionTimeout() : options.timeoutMillis(); + long effectiveTimeoutMillis = options.timeoutMillis() == 0 ? defaultTransactionTimeout() : options.timeoutMillis(); return beginTimestamp.addPhysicalTime(effectiveTimeoutMillis); } - private long defaultRoTransactionTimeout() { - return lowWatermarkConfig.dataAvailabilityTime().value(); + private long defaultTransactionTimeout() { + return txConfig.timeout().value(); } /** diff --git a/modules/transactions/src/test/java/org/apache/ignite/internal/tx/TxManagerTest.java b/modules/transactions/src/test/java/org/apache/ignite/internal/tx/TxManagerTest.java index 8f7ed20b178..da02914944f 100644 --- a/modules/transactions/src/test/java/org/apache/ignite/internal/tx/TxManagerTest.java +++ b/modules/transactions/src/test/java/org/apache/ignite/internal/tx/TxManagerTest.java @@ -78,7 +78,6 @@ import org.apache.ignite.internal.replicator.ReplicaService; import org.apache.ignite.internal.replicator.TablePartitionId; import org.apache.ignite.internal.replicator.exception.PrimaryReplicaMissException; -import org.apache.ignite.internal.schema.configuration.LowWatermarkConfiguration; import org.apache.ignite.internal.testframework.ExecutorServiceExtension; import org.apache.ignite.internal.testframework.IgniteAbstractTest; import org.apache.ignite.internal.testframework.InjectExecutorService; @@ -139,9 +138,6 @@ public class TxManagerTest extends IgniteAbstractTest { @InjectConfiguration private TransactionConfiguration txConfiguration; - @InjectConfiguration - private LowWatermarkConfiguration lowWatermarkConfiguration; - @InjectExecutorService private ScheduledExecutorService commonScheduler; @@ -167,7 +163,6 @@ public void setup() { txManager = new TxManagerImpl( txConfiguration, - lowWatermarkConfiguration, clusterService, replicaService, lockManager(), From c143840d21092cec7114d2aa2697aa3ab84161e4 Mon Sep 17 00:00:00 2001 From: Roman Puchkovskiy Date: Thu, 16 Jan 2025 14:52:01 +0400 Subject: [PATCH 03/18] IGNITE-23747 / remove CommonTestScheduler --- .../testframework/CommonTestScheduler.java | 42 ------------------- .../table/impl/DummyInternalTableImpl.java | 10 ++++- 2 files changed, 8 insertions(+), 44 deletions(-) delete mode 100644 modules/core/src/testFixtures/java/org/apache/ignite/internal/testframework/CommonTestScheduler.java diff --git a/modules/core/src/testFixtures/java/org/apache/ignite/internal/testframework/CommonTestScheduler.java b/modules/core/src/testFixtures/java/org/apache/ignite/internal/testframework/CommonTestScheduler.java deleted file mode 100644 index a65067cc1f3..00000000000 --- a/modules/core/src/testFixtures/java/org/apache/ignite/internal/testframework/CommonTestScheduler.java +++ /dev/null @@ -1,42 +0,0 @@ -/* - * 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. - */ - -package org.apache.ignite.internal.testframework; - -import java.util.concurrent.Executors; -import java.util.concurrent.ScheduledExecutorService; -import org.apache.ignite.internal.logger.IgniteLogger; -import org.apache.ignite.internal.logger.Loggers; -import org.apache.ignite.internal.thread.NamedThreadFactory; - -/** - * Provides access to a single-thread scheduler for light-weight non-blocking operations. It doesn't need to be stopped. - */ -public class CommonTestScheduler { - private static final IgniteLogger LOG = Loggers.forClass(CommonTestScheduler.class); - - private static final ScheduledExecutorService INSTANCE = Executors.newSingleThreadScheduledExecutor( - new NamedThreadFactory("test-common-scheduler-", true, LOG) - ); - - /** - * Returns a single-thread scheduler for light-weight non-blocking operations. It doesn't need to be shut down. - */ - public static ScheduledExecutorService instance() { - return INSTANCE; - } -} diff --git a/modules/table/src/testFixtures/java/org/apache/ignite/internal/table/impl/DummyInternalTableImpl.java b/modules/table/src/testFixtures/java/org/apache/ignite/internal/table/impl/DummyInternalTableImpl.java index 7047fe6f9c8..775480661c5 100644 --- a/modules/table/src/testFixtures/java/org/apache/ignite/internal/table/impl/DummyInternalTableImpl.java +++ b/modules/table/src/testFixtures/java/org/apache/ignite/internal/table/impl/DummyInternalTableImpl.java @@ -37,6 +37,8 @@ import java.util.Map; import java.util.UUID; import java.util.concurrent.CompletableFuture; +import java.util.concurrent.Executors; +import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicLong; import org.apache.ignite.distributed.TestPartitionDataStorage; @@ -106,7 +108,7 @@ import org.apache.ignite.internal.table.distributed.replicator.TransactionStateResolver; import org.apache.ignite.internal.table.distributed.schema.AlwaysSyncedSchemaSyncService; import org.apache.ignite.internal.table.distributed.storage.InternalTableImpl; -import org.apache.ignite.internal.testframework.CommonTestScheduler; +import org.apache.ignite.internal.thread.NamedThreadFactory; import org.apache.ignite.internal.tx.HybridTimestampTracker; import org.apache.ignite.internal.tx.InternalTransaction; import org.apache.ignite.internal.tx.TxManager; @@ -166,6 +168,10 @@ public class DummyInternalTableImpl extends InternalTableImpl { private static final AtomicInteger nextTableId = new AtomicInteger(10_001); + private static final ScheduledExecutorService COMMON_SCHEDULER = Executors.newSingleThreadScheduledExecutor( + new NamedThreadFactory("DummyInternalTable-common-scheduler-", true, LOG) + ); + /** * Creates a new local table. * @@ -572,7 +578,7 @@ public static TxManagerImpl txManager( resourcesRegistry, transactionInflights, new TestLowWatermark(), - CommonTestScheduler.instance() + COMMON_SCHEDULER ); assertThat(txManager.startAsync(new ComponentContext()), willCompleteSuccessfully()); From b7a96047dc07759bb2a741e7b0b3c5f4daf5a4e1 Mon Sep 17 00:00:00 2001 From: Roman Puchkovskiy Date: Thu, 16 Jan 2025 16:07:40 +0400 Subject: [PATCH 04/18] IGNITE-23747 / add a TODO about implicit RO txs expiration --- .../org/apache/ignite/internal/tx/impl/TxManagerImpl.java | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/modules/transactions/src/main/java/org/apache/ignite/internal/tx/impl/TxManagerImpl.java b/modules/transactions/src/main/java/org/apache/ignite/internal/tx/impl/TxManagerImpl.java index b7e7fbb2368..a3ece913837 100644 --- a/modules/transactions/src/main/java/org/apache/ignite/internal/tx/impl/TxManagerImpl.java +++ b/modules/transactions/src/main/java/org/apache/ignite/internal/tx/impl/TxManagerImpl.java @@ -451,7 +451,10 @@ private ReadOnlyTransactionImpl beginReadOnlyTransaction( // Implicit transactions are finished as soon as their operation/query is finished, they cannot be abandoned, so there is // no need to register them. - if (!implicit) { + // TODO: https://issues.apache.org/jira/browse/IGNITE-24229 - schedule expiration for multi-key implicit transactions? + boolean scheduleExpiration = !implicit; + + if (scheduleExpiration) { transactionExpirationRegistry.register(transaction, roExpirationTimeFor(beginTimestamp, options)); } @@ -459,7 +462,7 @@ private ReadOnlyTransactionImpl beginReadOnlyTransaction( lowWatermark.unlock(txId); // We only register explicit transactions, so we only unregister them as well. - if (!implicit) { + if (scheduleExpiration) { transactionExpirationRegistry.unregister(transaction); } }); From ac4e077f3800c2b9ab42271d1c194705f35d66d4 Mon Sep 17 00:00:00 2001 From: Roman Puchkovskiy Date: Thu, 16 Jan 2025 16:27:27 +0400 Subject: [PATCH 05/18] IGNITE-23747 / use physical time, not hybrid timestamps --- .../impl/TransactionExpirationRegistry.java | 25 +++++----- .../internal/tx/impl/TxManagerImpl.java | 12 ++--- .../TransactionExpirationRegistryTest.java | 49 +++++++++---------- 3 files changed, 44 insertions(+), 42 deletions(-) diff --git a/modules/transactions/src/main/java/org/apache/ignite/internal/tx/impl/TransactionExpirationRegistry.java b/modules/transactions/src/main/java/org/apache/ignite/internal/tx/impl/TransactionExpirationRegistry.java index 8e50f7d19de..58186af23ed 100644 --- a/modules/transactions/src/main/java/org/apache/ignite/internal/tx/impl/TransactionExpirationRegistry.java +++ b/modules/transactions/src/main/java/org/apache/ignite/internal/tx/impl/TransactionExpirationRegistry.java @@ -26,7 +26,6 @@ import java.util.concurrent.ConcurrentSkipListMap; import java.util.concurrent.locks.ReadWriteLock; import java.util.concurrent.locks.ReentrantReadWriteLock; -import org.apache.ignite.internal.hlc.HybridTimestamp; import org.apache.ignite.internal.logger.IgniteLogger; import org.apache.ignite.internal.logger.Loggers; import org.apache.ignite.internal.tx.InternalTransaction; @@ -34,13 +33,17 @@ class TransactionExpirationRegistry { private static final IgniteLogger LOG = Loggers.forClass(TransactionExpirationRegistry.class); - private final NavigableMap> txsByExpirationTime = new ConcurrentSkipListMap<>(); - private final Map expirationTimeByTx = new ConcurrentHashMap<>(); + /** Map from expiration timestamp (number of millis since Unix epoch) to set of transactions expiring at the timestamp. */ + private final NavigableMap> txsByExpirationTime = new ConcurrentSkipListMap<>(); + /** Map from registered transaction to its expiration timestamp. */ + private final Map expirationTimeByTx = new ConcurrentHashMap<>(); private final ReadWriteLock watermarkLock = new ReentrantReadWriteLock(); - private volatile HybridTimestamp watermark = HybridTimestamp.MIN_VALUE; - void register(InternalTransaction tx, HybridTimestamp txExpirationTime) { + /** Watermark at which expiration has already happened (millis since Unix epoch). */ + private volatile long watermark = Long.MIN_VALUE; + + void register(InternalTransaction tx, long txExpirationTime) { if (isExpired(txExpirationTime)) { abortTransaction(tx); return; @@ -66,8 +69,8 @@ void register(InternalTransaction tx, HybridTimestamp txExpirationTime) { } } - private boolean isExpired(HybridTimestamp expirationTime) { - return expirationTime.compareTo(watermark) <= 0; + private boolean isExpired(long expirationTime) { + return expirationTime <= watermark; } private static void abortTransaction(InternalTransaction tx) { @@ -78,13 +81,13 @@ private static void abortTransaction(InternalTransaction tx) { }); } - void expireUpTo(HybridTimestamp expirationTime) { + void expireUpTo(long expirationTime) { List> transactionSetsToExpire; watermarkLock.writeLock().lock(); try { - NavigableMap> headMap = txsByExpirationTime.headMap(expirationTime, true); + NavigableMap> headMap = txsByExpirationTime.headMap(expirationTime, true); transactionSetsToExpire = new ArrayList<>(headMap.values()); headMap.clear(); @@ -103,11 +106,11 @@ void expireUpTo(HybridTimestamp expirationTime) { } void abortAllRegistered() { - expireUpTo(HybridTimestamp.MAX_VALUE); + expireUpTo(Long.MAX_VALUE); } void unregister(InternalTransaction tx) { - HybridTimestamp expirationTime = expirationTimeByTx.remove(tx); + Long expirationTime = expirationTimeByTx.remove(tx); if (expirationTime != null) { txsByExpirationTime.compute(expirationTime, (k, set) -> { diff --git a/modules/transactions/src/main/java/org/apache/ignite/internal/tx/impl/TxManagerImpl.java b/modules/transactions/src/main/java/org/apache/ignite/internal/tx/impl/TxManagerImpl.java index a3ece913837..4e05dcc4510 100644 --- a/modules/transactions/src/main/java/org/apache/ignite/internal/tx/impl/TxManagerImpl.java +++ b/modules/transactions/src/main/java/org/apache/ignite/internal/tx/impl/TxManagerImpl.java @@ -455,7 +455,7 @@ private ReadOnlyTransactionImpl beginReadOnlyTransaction( boolean scheduleExpiration = !implicit; if (scheduleExpiration) { - transactionExpirationRegistry.register(transaction, roExpirationTimeFor(beginTimestamp, options)); + transactionExpirationRegistry.register(transaction, roExpirationPhysicalTimeFor(beginTimestamp, options)); } txFuture.whenComplete((unused, throwable) -> { @@ -474,12 +474,12 @@ private ReadOnlyTransactionImpl beginReadOnlyTransaction( } } - private HybridTimestamp roExpirationTimeFor(HybridTimestamp beginTimestamp, InternalTxOptions options) { - long effectiveTimeoutMillis = options.timeoutMillis() == 0 ? defaultTransactionTimeout() : options.timeoutMillis(); - return beginTimestamp.addPhysicalTime(effectiveTimeoutMillis); + private long roExpirationPhysicalTimeFor(HybridTimestamp beginTimestamp, InternalTxOptions options) { + long effectiveTimeoutMillis = options.timeoutMillis() == 0 ? defaultTransactionTimeoutMillis() : options.timeoutMillis(); + return beginTimestamp.getPhysical() + effectiveTimeoutMillis; } - private long defaultTransactionTimeout() { + private long defaultTransactionTimeoutMillis() { return txConfig.timeout().value(); } @@ -855,7 +855,7 @@ private void expireTransactionsUpToNow() { try { expirationTime = clockService.current(); - transactionExpirationRegistry.expireUpTo(expirationTime); + transactionExpirationRegistry.expireUpTo(expirationTime.getPhysical()); } catch (Throwable t) { LOG.error("Could not expire transactions up to {}", t, expirationTime); } diff --git a/modules/transactions/src/test/java/org/apache/ignite/internal/tx/impl/TransactionExpirationRegistryTest.java b/modules/transactions/src/test/java/org/apache/ignite/internal/tx/impl/TransactionExpirationRegistryTest.java index 6cdb0e2e227..086e0fac834 100644 --- a/modules/transactions/src/test/java/org/apache/ignite/internal/tx/impl/TransactionExpirationRegistryTest.java +++ b/modules/transactions/src/test/java/org/apache/ignite/internal/tx/impl/TransactionExpirationRegistryTest.java @@ -23,7 +23,6 @@ import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; -import org.apache.ignite.internal.hlc.HybridTimestamp; import org.apache.ignite.internal.testframework.BaseIgniteAbstractTest; import org.apache.ignite.internal.tx.InternalTransaction; import org.junit.jupiter.api.BeforeEach; @@ -50,10 +49,10 @@ void configureMocks() { @Test void abortsTransactionsBeforeExpirationTime() { - registry.register(tx1, new HybridTimestamp(1000, 0)); - registry.register(tx2, new HybridTimestamp(2000, 0)); + registry.register(tx1, 1000); + registry.register(tx2, 2000); - registry.expireUpTo(new HybridTimestamp(3000, 0)); + registry.expireUpTo(3000); verify(tx1).rollbackAsync(); verify(tx2).rollbackAsync(); @@ -61,38 +60,38 @@ void abortsTransactionsBeforeExpirationTime() { @Test void abortsTransactionsExactlyOnExpirationTime() { - registry.register(tx1, new HybridTimestamp(1000, 0)); + registry.register(tx1, 1000); - registry.expireUpTo(new HybridTimestamp(1000, 0)); + registry.expireUpTo(1000); verify(tx1).rollbackAsync(); } @Test void doesNotAbortTransactionsAfterExpirationTime() { - registry.register(tx1, new HybridTimestamp(1000, 1)); + registry.register(tx1, 1001); - registry.expireUpTo(new HybridTimestamp(1000, 0)); + registry.expireUpTo(1000); verify(tx1, never()).rollbackAsync(); } @Test void abortsTransactionsExpiredAfterFewExpirations() { - registry.register(tx1, new HybridTimestamp(1000, 1)); + registry.register(tx1, 1000); - registry.expireUpTo(new HybridTimestamp(1000, 0)); - registry.expireUpTo(new HybridTimestamp(2000, 0)); + registry.expireUpTo(1000); + registry.expireUpTo(2000); verify(tx1).rollbackAsync(); } @Test void abortsAlreadyExpiredTransactionOnRegistration() { - registry.expireUpTo(new HybridTimestamp(2000, 0)); + registry.expireUpTo(2000); - registry.register(tx1, new HybridTimestamp(1000, 0)); - registry.register(tx2, new HybridTimestamp(2000, 0)); + registry.register(tx1, 1000); + registry.register(tx2, 2000); verify(tx1).rollbackAsync(); verify(tx2).rollbackAsync(); @@ -100,12 +99,12 @@ void abortsAlreadyExpiredTransactionOnRegistration() { @Test void abortsAlreadyExpiredTransactionJustOnce() { - registry.expireUpTo(new HybridTimestamp(2000, 0)); + registry.expireUpTo(2000); - registry.register(tx1, new HybridTimestamp(1000, 0)); - registry.register(tx2, new HybridTimestamp(2000, 0)); + registry.register(tx1, 1000); + registry.register(tx2, 2000); - registry.expireUpTo(new HybridTimestamp(2000, 0)); + registry.expireUpTo(2000); verify(tx1, times(1)).rollbackAsync(); verify(tx2, times(1)).rollbackAsync(); @@ -113,8 +112,8 @@ void abortsAlreadyExpiredTransactionJustOnce() { @Test void abortsAllRegistered() { - registry.register(tx1, new HybridTimestamp(1000, 0)); - registry.register(tx2, HybridTimestamp.MAX_VALUE); + registry.register(tx1, 1000); + registry.register(tx2, Long.MAX_VALUE); registry.abortAllRegistered(); @@ -126,8 +125,8 @@ void abortsAllRegistered() { void abortsOnRegistrationAfterAbortingAllRegistered() { registry.abortAllRegistered(); - registry.register(tx1, new HybridTimestamp(1000, 0)); - registry.register(tx2, HybridTimestamp.MAX_VALUE); + registry.register(tx1, 1000); + registry.register(tx2, Long.MAX_VALUE); verify(tx1).rollbackAsync(); verify(tx2).rollbackAsync(); @@ -135,11 +134,11 @@ void abortsOnRegistrationAfterAbortingAllRegistered() { @Test void removesTransactionOnUnregister() { - registry.register(tx1, new HybridTimestamp(1000, 0)); + registry.register(tx1, 1000); registry.unregister(tx1); - registry.expireUpTo(new HybridTimestamp(2000, 0)); + registry.expireUpTo(2000); // Should not be aborted due to expiration as we removed the transaction. verify(tx1, never()).rollbackAsync(); @@ -147,7 +146,7 @@ void removesTransactionOnUnregister() { @Test void unregisterIsIdempotent() { - registry.register(tx1, new HybridTimestamp(1000, 0)); + registry.register(tx1, 1000); registry.unregister(tx1); registry.unregister(tx1); From 2603ee7ce3ce4aaaddc17653be4c89e199dc1d0e Mon Sep 17 00:00:00 2001 From: Roman Puchkovskiy Date: Thu, 16 Jan 2025 16:29:08 +0400 Subject: [PATCH 06/18] IGNITE-23747 / move a TODO --- .../src/main/java/org/apache/ignite/tx/TransactionOptions.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/modules/api/src/main/java/org/apache/ignite/tx/TransactionOptions.java b/modules/api/src/main/java/org/apache/ignite/tx/TransactionOptions.java index 5f745572062..94e6e2b1747 100644 --- a/modules/api/src/main/java/org/apache/ignite/tx/TransactionOptions.java +++ b/modules/api/src/main/java/org/apache/ignite/tx/TransactionOptions.java @@ -36,12 +36,13 @@ public long timeoutMillis() { return timeoutMillis; } + // TODO: remove a note that timeouts are not supported for RW after IGNITE-15936 is implemented. /** * Sets transaction timeout, in milliseconds. * * @param timeoutMillis Transaction timeout, in milliseconds. Cannot be negative; 0 means 'use default timeout'. * For RO transactions, the default timeout is configured via ignite.transaction.timeout configuration property. - * For RW transactions, timeouts are not supported yet. TODO: IGNITE-15936 + * For RW transactions, timeouts are not supported yet. * @return {@code this} for chaining. */ public TransactionOptions timeoutMillis(long timeoutMillis) { From d52388f6df9aa62ff4b503daa2ad47c7221f2479 Mon Sep 17 00:00:00 2001 From: Roman Puchkovskiy Date: Thu, 16 Jan 2025 16:33:21 +0400 Subject: [PATCH 07/18] IGNITE-23747 / improve javadoc --- .../main/java/org/apache/ignite/tx/TransactionOptions.java | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/modules/api/src/main/java/org/apache/ignite/tx/TransactionOptions.java b/modules/api/src/main/java/org/apache/ignite/tx/TransactionOptions.java index 94e6e2b1747..96594b5f4fd 100644 --- a/modules/api/src/main/java/org/apache/ignite/tx/TransactionOptions.java +++ b/modules/api/src/main/java/org/apache/ignite/tx/TransactionOptions.java @@ -41,8 +41,10 @@ public long timeoutMillis() { * Sets transaction timeout, in milliseconds. * * @param timeoutMillis Transaction timeout, in milliseconds. Cannot be negative; 0 means 'use default timeout'. - * For RO transactions, the default timeout is configured via ignite.transaction.timeout configuration property. - * For RW transactions, timeouts are not supported yet. + *
    + *
  • For RO transactions, the default timeout is configured via ignite.transaction.timeout configuration property.
    • + *
  • For RW transactions, timeouts are not supported yet.
    • + *
* @return {@code this} for chaining. */ public TransactionOptions timeoutMillis(long timeoutMillis) { From 45dd720e538b54d00fef1b0b474e1e6712663656 Mon Sep 17 00:00:00 2001 From: Roman Puchkovskiy Date: Thu, 16 Jan 2025 16:34:25 +0400 Subject: [PATCH 08/18] IGNITE-23747 / improve javadoc --- .../apache/ignite/internal/client/tx/ClientLazyTransaction.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/client/src/main/java/org/apache/ignite/internal/client/tx/ClientLazyTransaction.java b/modules/client/src/main/java/org/apache/ignite/internal/client/tx/ClientLazyTransaction.java index 57530d6a365..6e727999d41 100644 --- a/modules/client/src/main/java/org/apache/ignite/internal/client/tx/ClientLazyTransaction.java +++ b/modules/client/src/main/java/org/apache/ignite/internal/client/tx/ClientLazyTransaction.java @@ -166,7 +166,7 @@ private synchronized CompletableFuture ensureStarted( } /** - * Returns actual {@link ClientTransaction} started by this transaction or throws an exception if no transaction was started yet. + * Returns actual {@link ClientTransaction} started by this transaction. */ public ClientTransaction startedTx() { var tx0 = tx; From cd866cf4900ddd5915fa9e1e0db8502cbc2089b6 Mon Sep 17 00:00:00 2001 From: Roman Puchkovskiy Date: Thu, 16 Jan 2025 17:51:34 +0400 Subject: [PATCH 09/18] IGNITE-23747 / uncomment an assertion --- .../tx/readonly/ItReadOnlyTxTimeoutOneNodeTest.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/modules/transactions/src/integrationTest/java/org/apache/ignite/internal/tx/readonly/ItReadOnlyTxTimeoutOneNodeTest.java b/modules/transactions/src/integrationTest/java/org/apache/ignite/internal/tx/readonly/ItReadOnlyTxTimeoutOneNodeTest.java index 0bb57262b7f..d3e00753790 100644 --- a/modules/transactions/src/integrationTest/java/org/apache/ignite/internal/tx/readonly/ItReadOnlyTxTimeoutOneNodeTest.java +++ b/modules/transactions/src/integrationTest/java/org/apache/ignite/internal/tx/readonly/ItReadOnlyTxTimeoutOneNodeTest.java @@ -19,6 +19,7 @@ import static java.util.concurrent.TimeUnit.SECONDS; import static org.apache.ignite.internal.testframework.IgniteTestUtils.waitForCondition; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import org.apache.ignite.Ignite; @@ -26,6 +27,7 @@ import org.apache.ignite.internal.tx.impl.ReadOnlyTransactionImpl; import org.apache.ignite.table.Table; import org.apache.ignite.tx.Transaction; +import org.apache.ignite.tx.TransactionException; import org.apache.ignite.tx.TransactionOptions; import org.junit.jupiter.api.Test; @@ -59,9 +61,7 @@ void roTransactionTimesOut() throws Exception { "Transaction should have been finished due to timeout" ); - // TODO: Uncomment the following lines after https://issues.apache.org/jira/browse/IGNITE-23980 is fixed. - // assertThrows(TransactionException.class, () -> doGetOn(table, roTx)); - // assertThrows(TransactionException.class, roTx::commit); + assertThrows(TransactionException.class, () -> doGetOn(table, roTx)); } private static void doGetOn(Table table, Transaction tx) { From a813260edb58b04af797da5f292f98cf42afebe8 Mon Sep 17 00:00:00 2001 From: Roman Puchkovskiy Date: Thu, 16 Jan 2025 17:54:36 +0400 Subject: [PATCH 10/18] IGNITE-23747 / use computeIfPresent() --- .../internal/tx/impl/TransactionExpirationRegistry.java | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/modules/transactions/src/main/java/org/apache/ignite/internal/tx/impl/TransactionExpirationRegistry.java b/modules/transactions/src/main/java/org/apache/ignite/internal/tx/impl/TransactionExpirationRegistry.java index 58186af23ed..9fc28bcaae8 100644 --- a/modules/transactions/src/main/java/org/apache/ignite/internal/tx/impl/TransactionExpirationRegistry.java +++ b/modules/transactions/src/main/java/org/apache/ignite/internal/tx/impl/TransactionExpirationRegistry.java @@ -113,11 +113,7 @@ void unregister(InternalTransaction tx) { Long expirationTime = expirationTimeByTx.remove(tx); if (expirationTime != null) { - txsByExpirationTime.compute(expirationTime, (k, set) -> { - if (set == null) { - return null; - } - + txsByExpirationTime.computeIfPresent(expirationTime, (k, set) -> { set.remove(tx); return set.isEmpty() ? null : set; From 902cb17e988c0f0af50b5b470e5e493f575544c8 Mon Sep 17 00:00:00 2001 From: Roman Puchkovskiy Date: Thu, 16 Jan 2025 18:34:01 +0400 Subject: [PATCH 11/18] IGNITE-23747 / re-introduce the assertion about commit after timing out --- .../internal/tx/readonly/ItReadOnlyTxTimeoutOneNodeTest.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/modules/transactions/src/integrationTest/java/org/apache/ignite/internal/tx/readonly/ItReadOnlyTxTimeoutOneNodeTest.java b/modules/transactions/src/integrationTest/java/org/apache/ignite/internal/tx/readonly/ItReadOnlyTxTimeoutOneNodeTest.java index d3e00753790..c03ed59f1f2 100644 --- a/modules/transactions/src/integrationTest/java/org/apache/ignite/internal/tx/readonly/ItReadOnlyTxTimeoutOneNodeTest.java +++ b/modules/transactions/src/integrationTest/java/org/apache/ignite/internal/tx/readonly/ItReadOnlyTxTimeoutOneNodeTest.java @@ -62,6 +62,8 @@ void roTransactionTimesOut() throws Exception { ); assertThrows(TransactionException.class, () -> doGetOn(table, roTx)); + // TODO: uncomment the following assert after IGNITE-24233 is fixed. + // assertThrows(TransactionException.class, roTx::commit); } private static void doGetOn(Table table, Transaction tx) { From ac39830b04a42f1df1a2e4c422c3fb1bc829a852 Mon Sep 17 00:00:00 2001 From: Roman Puchkovskiy Date: Thu, 16 Jan 2025 20:42:25 +0400 Subject: [PATCH 12/18] IGNITE-23747 / optimize set usage --- .../impl/TransactionExpirationRegistry.java | 60 ++++++++++++++----- .../TransactionExpirationRegistryTest.java | 11 ++++ 2 files changed, 57 insertions(+), 14 deletions(-) diff --git a/modules/transactions/src/main/java/org/apache/ignite/internal/tx/impl/TransactionExpirationRegistry.java b/modules/transactions/src/main/java/org/apache/ignite/internal/tx/impl/TransactionExpirationRegistry.java index 9fc28bcaae8..b3c0351958d 100644 --- a/modules/transactions/src/main/java/org/apache/ignite/internal/tx/impl/TransactionExpirationRegistry.java +++ b/modules/transactions/src/main/java/org/apache/ignite/internal/tx/impl/TransactionExpirationRegistry.java @@ -18,6 +18,7 @@ package org.apache.ignite.internal.tx.impl; import java.util.ArrayList; +import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.NavigableMap; @@ -33,8 +34,12 @@ class TransactionExpirationRegistry { private static final IgniteLogger LOG = Loggers.forClass(TransactionExpirationRegistry.class); - /** Map from expiration timestamp (number of millis since Unix epoch) to set of transactions expiring at the timestamp. */ - private final NavigableMap> txsByExpirationTime = new ConcurrentSkipListMap<>(); + /** + * Map from expiration timestamp (number of millis since Unix epoch) to transactions expiring at the timestamp. + * Each value is either a transaction or a set of at least 2 transactions. + */ + private final NavigableMap txsByExpirationTime = new ConcurrentSkipListMap<>(); + /** Map from registered transaction to its expiration timestamp. */ private final Map expirationTimeByTx = new ConcurrentHashMap<>(); @@ -57,11 +62,26 @@ void register(InternalTransaction tx, long txExpirationTime) { return; } - Set txsExpiringAtTs = txsByExpirationTime.computeIfAbsent( + txsByExpirationTime.compute( txExpirationTime, - k -> ConcurrentHashMap.newKeySet() + (k, txOrSet) -> { + if (txOrSet == null) { + return tx; + } + + Set txsExpiringAtTs; + if (txOrSet instanceof Set) { + txsExpiringAtTs = (Set) txOrSet; + } else { + txsExpiringAtTs = new HashSet<>(); + txsExpiringAtTs.add((InternalTransaction) txOrSet); + } + + txsExpiringAtTs.add(tx); + + return txsExpiringAtTs; + } ); - txsExpiringAtTs.add(tx); expirationTimeByTx.put(tx, txExpirationTime); } finally { @@ -82,13 +102,13 @@ private static void abortTransaction(InternalTransaction tx) { } void expireUpTo(long expirationTime) { - List> transactionSetsToExpire; + List transactionsAndSetsToExpire; watermarkLock.writeLock().lock(); try { - NavigableMap> headMap = txsByExpirationTime.headMap(expirationTime, true); - transactionSetsToExpire = new ArrayList<>(headMap.values()); + NavigableMap headMap = txsByExpirationTime.headMap(expirationTime, true); + transactionsAndSetsToExpire = new ArrayList<>(headMap.values()); headMap.clear(); watermark = expirationTime; @@ -96,10 +116,16 @@ void expireUpTo(long expirationTime) { watermarkLock.writeLock().unlock(); } - for (Set set : transactionSetsToExpire) { - for (InternalTransaction tx : set) { - expirationTimeByTx.remove(tx); + for (Object txOrSet : transactionsAndSetsToExpire) { + if (txOrSet instanceof Set) { + for (InternalTransaction tx : (Set) txOrSet) { + expirationTimeByTx.remove(tx); + abortTransaction(tx); + } + } else { + InternalTransaction tx = (InternalTransaction) txOrSet; + expirationTimeByTx.remove(tx); abortTransaction(tx); } } @@ -113,10 +139,16 @@ void unregister(InternalTransaction tx) { Long expirationTime = expirationTimeByTx.remove(tx); if (expirationTime != null) { - txsByExpirationTime.computeIfPresent(expirationTime, (k, set) -> { - set.remove(tx); + txsByExpirationTime.computeIfPresent(expirationTime, (k, txOrSet) -> { + if (txOrSet instanceof Set) { + Set set = (Set) txOrSet; + + set.remove(tx); - return set.isEmpty() ? null : set; + return set.size() == 1 ? set.iterator().next() : set; + } else { + return null; + } }); } } diff --git a/modules/transactions/src/test/java/org/apache/ignite/internal/tx/impl/TransactionExpirationRegistryTest.java b/modules/transactions/src/test/java/org/apache/ignite/internal/tx/impl/TransactionExpirationRegistryTest.java index 086e0fac834..3f6a0b68114 100644 --- a/modules/transactions/src/test/java/org/apache/ignite/internal/tx/impl/TransactionExpirationRegistryTest.java +++ b/modules/transactions/src/test/java/org/apache/ignite/internal/tx/impl/TransactionExpirationRegistryTest.java @@ -86,6 +86,17 @@ void abortsTransactionsExpiredAfterFewExpirations() { verify(tx1).rollbackAsync(); } + @Test + void abortsTransactionsWithSameExpirationTime() { + registry.register(tx1, 1000); + registry.register(tx2, 1000); + + registry.expireUpTo(2000); + + verify(tx1).rollbackAsync(); + verify(tx2).rollbackAsync(); + } + @Test void abortsAlreadyExpiredTransactionOnRegistration() { registry.expireUpTo(2000); From 75ea771df7c54e34a95694d4b0a373ae30dfe33c Mon Sep 17 00:00:00 2001 From: Roman Puchkovskiy Date: Thu, 16 Jan 2025 20:43:35 +0400 Subject: [PATCH 13/18] IGNITE-23747 / add an assertion --- .../internal/tx/impl/TransactionExpirationRegistryTest.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/modules/transactions/src/test/java/org/apache/ignite/internal/tx/impl/TransactionExpirationRegistryTest.java b/modules/transactions/src/test/java/org/apache/ignite/internal/tx/impl/TransactionExpirationRegistryTest.java index 3f6a0b68114..6f4650c13be 100644 --- a/modules/transactions/src/test/java/org/apache/ignite/internal/tx/impl/TransactionExpirationRegistryTest.java +++ b/modules/transactions/src/test/java/org/apache/ignite/internal/tx/impl/TransactionExpirationRegistryTest.java @@ -18,6 +18,7 @@ package org.apache.ignite.internal.tx.impl; import static org.apache.ignite.internal.util.CompletableFutures.nullCompletedFuture; +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; import static org.mockito.Mockito.lenient; import static org.mockito.Mockito.never; import static org.mockito.Mockito.times; @@ -160,6 +161,7 @@ void unregisterIsIdempotent() { registry.register(tx1, 1000); registry.unregister(tx1); - registry.unregister(tx1); + + assertDoesNotThrow(() -> registry.unregister(tx1)); } } From a887d6260270374015894749836815230d873c30 Mon Sep 17 00:00:00 2001 From: Roman Puchkovskiy Date: Fri, 17 Jan 2025 11:08:17 +0400 Subject: [PATCH 14/18] IGNITE-23747 / fix ItTxResourcesVacuumTest --- .../tx/distributed/ItTxResourcesVacuumTest.java | 6 +++++- .../ignite/internal/tx/impl/TxManagerImpl.java | 16 +++++++++++++++- 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/modules/transactions/src/integrationTest/java/org/apache/ignite/tx/distributed/ItTxResourcesVacuumTest.java b/modules/transactions/src/integrationTest/java/org/apache/ignite/tx/distributed/ItTxResourcesVacuumTest.java index 851a69461b0..2c1e16b7354 100644 --- a/modules/transactions/src/integrationTest/java/org/apache/ignite/tx/distributed/ItTxResourcesVacuumTest.java +++ b/modules/transactions/src/integrationTest/java/org/apache/ignite/tx/distributed/ItTxResourcesVacuumTest.java @@ -786,7 +786,11 @@ public void testRoReadTheCorrectDataInBetween() { } private static Transaction beginReadOnlyTx(IgniteImpl node) { - return node.transactions().begin(new TransactionOptions().readOnly(true)); + return node.transactions().begin( + new TransactionOptions() + .readOnly(true) + .timeoutMillis(Long.MAX_VALUE) + ); } /** diff --git a/modules/transactions/src/main/java/org/apache/ignite/internal/tx/impl/TxManagerImpl.java b/modules/transactions/src/main/java/org/apache/ignite/internal/tx/impl/TxManagerImpl.java index 4e05dcc4510..54ba6d4804b 100644 --- a/modules/transactions/src/main/java/org/apache/ignite/internal/tx/impl/TxManagerImpl.java +++ b/modules/transactions/src/main/java/org/apache/ignite/internal/tx/impl/TxManagerImpl.java @@ -476,7 +476,21 @@ private ReadOnlyTransactionImpl beginReadOnlyTransaction( private long roExpirationPhysicalTimeFor(HybridTimestamp beginTimestamp, InternalTxOptions options) { long effectiveTimeoutMillis = options.timeoutMillis() == 0 ? defaultTransactionTimeoutMillis() : options.timeoutMillis(); - return beginTimestamp.getPhysical() + effectiveTimeoutMillis; + return sumWithSaturation(beginTimestamp.getPhysical(), effectiveTimeoutMillis); + } + + private static long sumWithSaturation(long a, long b) { + assert a >= 0 : a; + assert b >= 0 : b; + + long sum = a + b; + + if (sum < 0) { + // Overflow. + return Long.MAX_VALUE; + } else { + return sum; + } } private long defaultTransactionTimeoutMillis() { From 77fd928f85eba2d40b33a5d8996b88909dfd74ad Mon Sep 17 00:00:00 2001 From: Roman Puchkovskiy Date: Fri, 17 Jan 2025 11:10:26 +0400 Subject: [PATCH 15/18] IGNITE-23747 / increase default timeout to 10 seconds to allow test_filter_clause.test to pass --- .../tx/configuration/TransactionConfigurationSchema.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/transactions/src/main/java/org/apache/ignite/internal/tx/configuration/TransactionConfigurationSchema.java b/modules/transactions/src/main/java/org/apache/ignite/internal/tx/configuration/TransactionConfigurationSchema.java index 1ec9d1ae74a..3329687cba0 100644 --- a/modules/transactions/src/main/java/org/apache/ignite/internal/tx/configuration/TransactionConfigurationSchema.java +++ b/modules/transactions/src/main/java/org/apache/ignite/internal/tx/configuration/TransactionConfigurationSchema.java @@ -39,7 +39,7 @@ public class TransactionConfigurationSchema { /** Default transaction timeout (milliseconds). */ @Range(min = 1) @Value(hasDefault = true) - public final long timeout = 3_000; + public final long timeout = 10_000; /** Timeout for implicit transactions (milliseconds). */ @Range(min = 1) From 13d32ee8babaa2dee06716722d29a61fed00a524 Mon Sep 17 00:00:00 2001 From: Roman Puchkovskiy Date: Fri, 17 Jan 2025 11:13:19 +0400 Subject: [PATCH 16/18] IGNITE-23747 / improve javadoc --- .../tx/configuration/TransactionConfigurationSchema.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/transactions/src/main/java/org/apache/ignite/internal/tx/configuration/TransactionConfigurationSchema.java b/modules/transactions/src/main/java/org/apache/ignite/internal/tx/configuration/TransactionConfigurationSchema.java index 3329687cba0..47bf5b8b447 100644 --- a/modules/transactions/src/main/java/org/apache/ignite/internal/tx/configuration/TransactionConfigurationSchema.java +++ b/modules/transactions/src/main/java/org/apache/ignite/internal/tx/configuration/TransactionConfigurationSchema.java @@ -31,7 +31,7 @@ public class TransactionConfigurationSchema { /** Default checking transaction interval. */ public static final long DEFAULT_ABANDONED_CHECK_TS = 5_000; - /** Checking transaction interval (milliseconds). */ + /** How often abandoned transactions are searched for (milliseconds). */ @Range(min = 0) @Value(hasDefault = true) public final long abandonedCheckTs = DEFAULT_ABANDONED_CHECK_TS; From 43df82acb02e2aeb7817fa42386b0cf78107d5f1 Mon Sep 17 00:00:00 2001 From: Roman Puchkovskiy Date: Fri, 17 Jan 2025 11:37:44 +0400 Subject: [PATCH 17/18] IGNITE-23747 / introduce more methods to TxManager --- .../ItInternalTableReadOnlyScanTest.java | 2 +- .../storage/InternalTableImpl.java | 2 +- .../apache/ignite/internal/tx/TxManager.java | 25 +++++++++++++++++- .../ignite/internal/tx/TxManagerTest.java | 26 +++++++++---------- 4 files changed, 39 insertions(+), 16 deletions(-) diff --git a/modules/table/src/integrationTest/java/org/apache/ignite/distributed/ItInternalTableReadOnlyScanTest.java b/modules/table/src/integrationTest/java/org/apache/ignite/distributed/ItInternalTableReadOnlyScanTest.java index 200c22bf59a..1e33ce0e110 100644 --- a/modules/table/src/integrationTest/java/org/apache/ignite/distributed/ItInternalTableReadOnlyScanTest.java +++ b/modules/table/src/integrationTest/java/org/apache/ignite/distributed/ItInternalTableReadOnlyScanTest.java @@ -49,7 +49,7 @@ protected Publisher scan(int part, @Nullable InternalTransaction tx) @Override protected InternalTransaction startTx() { - return internalTbl.txManager().beginExplicit(HYBRID_TIMESTAMP_TRACKER, true, InternalTxOptions.defaults()); + return internalTbl.txManager().beginExplicitRo(HYBRID_TIMESTAMP_TRACKER, InternalTxOptions.defaults()); } @Override diff --git a/modules/table/src/main/java/org/apache/ignite/internal/table/distributed/storage/InternalTableImpl.java b/modules/table/src/main/java/org/apache/ignite/internal/table/distributed/storage/InternalTableImpl.java index d516385639a..1f1a61d946f 100644 --- a/modules/table/src/main/java/org/apache/ignite/internal/table/distributed/storage/InternalTableImpl.java +++ b/modules/table/src/main/java/org/apache/ignite/internal/table/distributed/storage/InternalTableImpl.java @@ -505,7 +505,7 @@ private InternalTransaction startImplicitRwTxIfNeeded(@Nullable InternalTransact } private InternalTransaction startImplicitRoTxIfNeeded(@Nullable InternalTransaction tx) { - return tx == null ? txManager.beginImplicit(observableTimestampTracker, true) : tx; + return tx == null ? txManager.beginImplicitRo(observableTimestampTracker) : tx; } /** diff --git a/modules/transactions/src/main/java/org/apache/ignite/internal/tx/TxManager.java b/modules/transactions/src/main/java/org/apache/ignite/internal/tx/TxManager.java index 3f956c21927..e0263de63b5 100644 --- a/modules/transactions/src/main/java/org/apache/ignite/internal/tx/TxManager.java +++ b/modules/transactions/src/main/java/org/apache/ignite/internal/tx/TxManager.java @@ -46,7 +46,18 @@ default InternalTransaction beginImplicitRw(HybridTimestampTracker timestampTrac } /** - * Starts an implicit read-write transaction coordinated by a local node. + * Starts an implicit read-only transaction coordinated by a local node. + * + * @param timestampTracker Observable timestamp tracker is used to track a timestamp for either read-write or read-only + * transaction execution. The tracker is also used to determine the read timestamp for read-only transactions. + * @return The transaction. + */ + default InternalTransaction beginImplicitRo(HybridTimestampTracker timestampTracker) { + return beginImplicit(timestampTracker, true); + } + + /** + * Starts an implicit transaction coordinated by a local node. * * @param timestampTracker Observable timestamp tracker is used to track a timestamp for either read-write or read-only * transaction execution. The tracker is also used to determine the read timestamp for read-only transactions. @@ -68,6 +79,18 @@ default InternalTransaction beginExplicitRw(HybridTimestampTracker timestampTrac return beginExplicit(timestampTracker, false, options); } + /** + * Starts an explicit read-only transaction coordinated by a local node. + * + * @param timestampTracker Observable timestamp tracker is used to track a timestamp for either read-write or read-only + * transaction execution. The tracker is also used to determine the read timestamp for read-only transactions. + * @param options Transaction options. + * @return The transaction. + */ + default InternalTransaction beginExplicitRo(HybridTimestampTracker timestampTracker, InternalTxOptions options) { + return beginExplicit(timestampTracker, true, options); + } + /** * Starts either read-write or read-only explicit transaction, depending on {@code readOnly} parameter value. * diff --git a/modules/transactions/src/test/java/org/apache/ignite/internal/tx/TxManagerTest.java b/modules/transactions/src/test/java/org/apache/ignite/internal/tx/TxManagerTest.java index d9cbd4e0a79..e3b17c18dbc 100644 --- a/modules/transactions/src/test/java/org/apache/ignite/internal/tx/TxManagerTest.java +++ b/modules/transactions/src/test/java/org/apache/ignite/internal/tx/TxManagerTest.java @@ -204,9 +204,9 @@ public void tearDown() { @Test public void testBegin() { InternalTransaction tx0 = txManager.beginExplicitRw(hybridTimestampTracker, InternalTxOptions.defaults()); - InternalTransaction tx1 = txManager.beginExplicit(hybridTimestampTracker, true, InternalTxOptions.defaults()); + InternalTransaction tx1 = txManager.beginExplicitRo(hybridTimestampTracker, InternalTxOptions.defaults()); InternalTransaction tx2 = txManager.beginImplicitRw(hybridTimestampTracker); - InternalTransaction tx3 = txManager.beginImplicit(hybridTimestampTracker, true); + InternalTransaction tx3 = txManager.beginImplicitRo(hybridTimestampTracker); assertNotNull(tx0.id()); assertNotNull(tx1.id()); @@ -257,7 +257,7 @@ void testCreateNewRoTxAfterUpdateLowerWatermark() { IgniteInternalException exception = assertThrows( IgniteInternalException.class, - () -> txManager.beginExplicit(hybridTimestampTracker, true, InternalTxOptions.defaults()) + () -> txManager.beginExplicitRo(hybridTimestampTracker, InternalTxOptions.defaults()) ); assertEquals(Transactions.TX_READ_ONLY_TOO_OLD_ERR, exception.code()); @@ -272,8 +272,8 @@ void testUpdateLowerWatermark() { hybridTimestampTracker.update(clockService.now()); - InternalTransaction roTx0 = txManager.beginExplicit(hybridTimestampTracker, true, InternalTxOptions.defaults()); - InternalTransaction roTx1 = txManager.beginExplicit(hybridTimestampTracker, true, InternalTxOptions.defaults()); + InternalTransaction roTx0 = txManager.beginExplicitRo(hybridTimestampTracker, InternalTxOptions.defaults()); + InternalTransaction roTx1 = txManager.beginExplicitRo(hybridTimestampTracker, InternalTxOptions.defaults()); CompletableFuture readOnlyTxsFuture = lowWatermark.updateAndNotify(roTx1.readTimestamp()); assertFalse(readOnlyTxsFuture.isDone()); @@ -404,7 +404,7 @@ public void testOnlyPendingCommit(boolean startReadOnlyTransaction) { assertEquals(0, txManager.finished()); // Start transaction. - InternalTransaction tx = txManager.beginExplicit(hybridTimestampTracker, true, InternalTxOptions.defaults()); + InternalTransaction tx = txManager.beginExplicitRo(hybridTimestampTracker, InternalTxOptions.defaults()); assertEquals(1, txManager.pending()); assertEquals(0, txManager.finished()); @@ -432,7 +432,7 @@ public void testOnlyPendingRollback(boolean startReadOnlyTransaction) { // Start transaction. InternalTransaction tx = - startReadOnlyTransaction ? txManager.beginExplicit(hybridTimestampTracker, true, InternalTxOptions.defaults()) + startReadOnlyTransaction ? txManager.beginExplicitRo(hybridTimestampTracker, InternalTxOptions.defaults()) : txManager.beginExplicitRw(hybridTimestampTracker, InternalTxOptions.defaults()); assertEquals(1, txManager.pending()); assertEquals(0, txManager.finished()); @@ -461,14 +461,14 @@ public void testObservableTimestamp() { HybridTimestamp now = clockService.now(); - InternalTransaction tx = txManager.beginExplicit(hybridTimestampTracker, true, InternalTxOptions.defaults()); + InternalTransaction tx = txManager.beginExplicitRo(hybridTimestampTracker, InternalTxOptions.defaults()); assertTrue(abs(now.getPhysical() - tx.readTimestamp().getPhysical()) > compareThreshold); tx.commit(); hybridTimestampTracker.update(now); - tx = txManager.beginExplicit(hybridTimestampTracker, true, InternalTxOptions.defaults()); + tx = txManager.beginExplicitRo(hybridTimestampTracker, InternalTxOptions.defaults()); assertTrue(abs(now.getPhysical() - tx.readTimestamp().getPhysical()) < compareThreshold); tx.commit(); @@ -482,7 +482,7 @@ public void testObservableTimestamp() { hybridTimestampTracker.update(timestampInPast); - tx = txManager.beginExplicit(hybridTimestampTracker, true, InternalTxOptions.defaults()); + tx = txManager.beginExplicitRo(hybridTimestampTracker, InternalTxOptions.defaults()); long readTime = now.getPhysical() - idleSafeTimePropagationPeriodMsSupplier.getAsLong() - clockService.maxClockSkewMillis(); @@ -499,7 +499,7 @@ public void testObservableTimestampLocally() { HybridTimestamp now = clockService.now(); - InternalTransaction tx = txManager.beginExplicit(hybridTimestampTracker, true, InternalTxOptions.defaults()); + InternalTransaction tx = txManager.beginExplicitRo(hybridTimestampTracker, InternalTxOptions.defaults()); HybridTimestamp firstReadTs = tx.readTimestamp(); @@ -509,7 +509,7 @@ public void testObservableTimestampLocally() { + idleSafeTimePropagationPeriodMsSupplier.getAsLong() + clockService.maxClockSkewMillis()); tx.commit(); - tx = txManager.beginExplicit(hybridTimestampTracker, true, InternalTxOptions.defaults()); + tx = txManager.beginExplicitRo(hybridTimestampTracker, InternalTxOptions.defaults()); assertTrue(firstReadTs.compareTo(tx.readTimestamp()) <= 0); @@ -759,7 +759,7 @@ void testCreateBeginTsInsideInUpdateRwTxCount() { return result; }).when(localRwTxCounter).inUpdateRwTxCountLock(any()); - txManager.beginExplicit(hybridTimestampTracker, false, InternalTxOptions.defaults()); + txManager.beginExplicitRw(hybridTimestampTracker, InternalTxOptions.defaults()); } private InternalTransaction prepareTransaction() { From e8b7636f563946bdf580814e1446595a6081fbd3 Mon Sep 17 00:00:00 2001 From: Roman Puchkovskiy Date: Fri, 17 Jan 2025 11:40:04 +0400 Subject: [PATCH 18/18] IGNITE-23747 / improve javadoc --- .../src/main/java/org/apache/ignite/internal/tx/TxManager.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/modules/transactions/src/main/java/org/apache/ignite/internal/tx/TxManager.java b/modules/transactions/src/main/java/org/apache/ignite/internal/tx/TxManager.java index e0263de63b5..3cfa0d89eb6 100644 --- a/modules/transactions/src/main/java/org/apache/ignite/internal/tx/TxManager.java +++ b/modules/transactions/src/main/java/org/apache/ignite/internal/tx/TxManager.java @@ -62,7 +62,6 @@ default InternalTransaction beginImplicitRo(HybridTimestampTracker timestampTrac * @param timestampTracker Observable timestamp tracker is used to track a timestamp for either read-write or read-only * transaction execution. The tracker is also used to determine the read timestamp for read-only transactions. * @param readOnly {@code true} in order to start a read-only transaction, {@code false} in order to start read-write one. - * Calling begin with readOnly {@code false} is an equivalent of TxManager#begin(). * @return The transaction. */ InternalTransaction beginImplicit(HybridTimestampTracker timestampTracker, boolean readOnly); @@ -98,7 +97,6 @@ default InternalTransaction beginExplicitRo(HybridTimestampTracker timestampTrac * transaction execution. The tracker is also used to determine the read timestamp for read-only transactions. Each client * should pass its own tracker to provide linearizability between read-write and read-only transactions started by this client. * @param readOnly {@code true} in order to start a read-only transaction, {@code false} in order to start read-write one. - * Calling begin with readOnly {@code false} is an equivalent of TxManager#begin(). * @param txOptions Options. * @return The started transaction. */