sanpwc commented on code in PR #2720: URL: https://github.com/apache/ignite-3/pull/2720#discussion_r1415367473
########## modules/table/src/integrationTest/java/org/apache/ignite/distributed/ItLockTableTest.java: ########## @@ -0,0 +1,274 @@ +/* + * 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.distributed; + +import static org.apache.ignite.internal.replicator.ReplicaManager.DEFAULT_IDLE_SAFE_TIME_PROPAGATION_PERIOD_MILLISECONDS; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.util.ArrayList; +import java.util.List; +import java.util.concurrent.CompletableFuture; +import org.apache.ignite.internal.configuration.testframework.ConfigurationExtension; +import org.apache.ignite.internal.configuration.testframework.InjectConfiguration; +import org.apache.ignite.internal.hlc.HybridClock; +import org.apache.ignite.internal.logger.IgniteLogger; +import org.apache.ignite.internal.logger.Loggers; +import org.apache.ignite.internal.placementdriver.PlacementDriver; +import org.apache.ignite.internal.raft.configuration.RaftConfiguration; +import org.apache.ignite.internal.replicator.ReplicaService; +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.table.TableViewInternal; +import org.apache.ignite.internal.testframework.IgniteAbstractTest; +import org.apache.ignite.internal.tx.DeadlockPreventionPolicy; +import org.apache.ignite.internal.tx.HybridTimestampTracker; +import org.apache.ignite.internal.tx.InternalTransaction; +import org.apache.ignite.internal.tx.impl.HeapLockManager; +import org.apache.ignite.internal.tx.impl.HeapLockManager.LockState; +import org.apache.ignite.internal.tx.impl.HeapUnboundedLockManager; +import org.apache.ignite.internal.tx.impl.TransactionIdGenerator; +import org.apache.ignite.internal.tx.impl.TxManagerImpl; +import org.apache.ignite.internal.type.NativeTypes; +import org.apache.ignite.network.ClusterNode; +import org.apache.ignite.network.ClusterService; +import org.apache.ignite.raft.jraft.test.TestUtils; +import org.apache.ignite.table.RecordView; +import org.apache.ignite.table.Tuple; +import org.apache.ignite.tx.Transaction; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Disabled; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.TestInfo; +import org.junit.jupiter.api.extension.ExtendWith; + +/** + * Test lock table. + */ +@ExtendWith(ConfigurationExtension.class) +public class ItLockTableTest extends IgniteAbstractTest { + private static final IgniteLogger LOG = Loggers.forClass(ItLockTableTest.class); + + private static int EMP_TABLE_ID = 2; + + private static final int CACHE_SIZE = 10; + + private static final String TABLE_NAME = "test"; + + private static SchemaDescriptor TABLE_SCHEMA = new SchemaDescriptor( + 1, + new Column[]{new Column("id".toUpperCase(), NativeTypes.INT32, false)}, + new Column[]{ + new Column("name".toUpperCase(), NativeTypes.STRING, true), + new Column("salary".toUpperCase(), NativeTypes.DOUBLE, true) + } + ); + + protected TableViewInternal testTable; + + protected final TestInfo testInfo; + + //TODO fsync can be turned on again after https://issues.apache.org/jira/browse/IGNITE-20195 + @InjectConfiguration("mock: { fsync: false }") + protected static RaftConfiguration raftConfiguration; + + @InjectConfiguration + protected static GcConfiguration gcConfig; + + private ItTxTestCluster txTestCluster; + + private HybridTimestampTracker timestampTracker = new HybridTimestampTracker(); + + /** + * The constructor. + * + * @param testInfo Test info. + */ + public ItLockTableTest(TestInfo testInfo) { + this.testInfo = testInfo; + } + + @BeforeEach + public void before() throws Exception { + txTestCluster = new ItTxTestCluster( + testInfo, + raftConfiguration, + workDir, + 1, + 1, + false, + timestampTracker + ) { + @Override + protected TxManagerImpl newTxManager( + ClusterService clusterService, + ReplicaService replicaSvc, + HybridClock clock, + TransactionIdGenerator generator, + ClusterNode node, + PlacementDriver placementDriver + ) { + return new TxManagerImpl( + clusterService, + replicaSvc, + new HeapLockManager( + DeadlockPreventionPolicy.NO_OP, + HeapLockManager.SLOTS, + CACHE_SIZE, + new HeapUnboundedLockManager()), + clock, + generator, + placementDriver, + () -> DEFAULT_IDLE_SAFE_TIME_PROPAGATION_PERIOD_MILLISECONDS + ); + } + }; + txTestCluster.prepareCluster(); + + testTable = txTestCluster.startTable(TABLE_NAME, EMP_TABLE_ID, TABLE_SCHEMA); + + log.info("Tables have been started"); + } + + @AfterEach + public void after() throws Exception { + txTestCluster.shutdownCluster(); + } + + @Test + @Disabled("https://issues.apache.org/jira/browse/IGNITE-20894";) Review Comment: Do we have given deadlock issue in current main? @Cyrill claims that we don't. ########## modules/table/src/integrationTest/java/org/apache/ignite/distributed/ItLockTableTest.java: ########## @@ -0,0 +1,274 @@ +/* + * 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.distributed; + +import static org.apache.ignite.internal.replicator.ReplicaManager.DEFAULT_IDLE_SAFE_TIME_PROPAGATION_PERIOD_MILLISECONDS; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.util.ArrayList; +import java.util.List; +import java.util.concurrent.CompletableFuture; +import org.apache.ignite.internal.configuration.testframework.ConfigurationExtension; +import org.apache.ignite.internal.configuration.testframework.InjectConfiguration; +import org.apache.ignite.internal.hlc.HybridClock; +import org.apache.ignite.internal.logger.IgniteLogger; +import org.apache.ignite.internal.logger.Loggers; +import org.apache.ignite.internal.placementdriver.PlacementDriver; +import org.apache.ignite.internal.raft.configuration.RaftConfiguration; +import org.apache.ignite.internal.replicator.ReplicaService; +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.table.TableViewInternal; +import org.apache.ignite.internal.testframework.IgniteAbstractTest; +import org.apache.ignite.internal.tx.DeadlockPreventionPolicy; +import org.apache.ignite.internal.tx.HybridTimestampTracker; +import org.apache.ignite.internal.tx.InternalTransaction; +import org.apache.ignite.internal.tx.impl.HeapLockManager; +import org.apache.ignite.internal.tx.impl.HeapLockManager.LockState; +import org.apache.ignite.internal.tx.impl.HeapUnboundedLockManager; +import org.apache.ignite.internal.tx.impl.TransactionIdGenerator; +import org.apache.ignite.internal.tx.impl.TxManagerImpl; +import org.apache.ignite.internal.type.NativeTypes; +import org.apache.ignite.network.ClusterNode; +import org.apache.ignite.network.ClusterService; +import org.apache.ignite.raft.jraft.test.TestUtils; +import org.apache.ignite.table.RecordView; +import org.apache.ignite.table.Tuple; +import org.apache.ignite.tx.Transaction; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Disabled; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.TestInfo; +import org.junit.jupiter.api.extension.ExtendWith; + +/** + * Test lock table. + */ +@ExtendWith(ConfigurationExtension.class) +public class ItLockTableTest extends IgniteAbstractTest { + private static final IgniteLogger LOG = Loggers.forClass(ItLockTableTest.class); + + private static int EMP_TABLE_ID = 2; + + private static final int CACHE_SIZE = 10; + + private static final String TABLE_NAME = "test"; + + private static SchemaDescriptor TABLE_SCHEMA = new SchemaDescriptor( + 1, + new Column[]{new Column("id".toUpperCase(), NativeTypes.INT32, false)}, + new Column[]{ + new Column("name".toUpperCase(), NativeTypes.STRING, true), + new Column("salary".toUpperCase(), NativeTypes.DOUBLE, true) + } + ); + + protected TableViewInternal testTable; + + protected final TestInfo testInfo; + + //TODO fsync can be turned on again after https://issues.apache.org/jira/browse/IGNITE-20195 + @InjectConfiguration("mock: { fsync: false }") + protected static RaftConfiguration raftConfiguration; + + @InjectConfiguration + protected static GcConfiguration gcConfig; + + private ItTxTestCluster txTestCluster; + + private HybridTimestampTracker timestampTracker = new HybridTimestampTracker(); + + /** + * The constructor. + * + * @param testInfo Test info. + */ + public ItLockTableTest(TestInfo testInfo) { + this.testInfo = testInfo; + } + + @BeforeEach + public void before() throws Exception { + txTestCluster = new ItTxTestCluster( + testInfo, + raftConfiguration, + workDir, + 1, + 1, + false, + timestampTracker + ) { + @Override + protected TxManagerImpl newTxManager( + ClusterService clusterService, + ReplicaService replicaSvc, + HybridClock clock, + TransactionIdGenerator generator, + ClusterNode node, + PlacementDriver placementDriver + ) { + return new TxManagerImpl( + clusterService, + replicaSvc, + new HeapLockManager( + DeadlockPreventionPolicy.NO_OP, + HeapLockManager.SLOTS, + CACHE_SIZE, + new HeapUnboundedLockManager()), + clock, + generator, + placementDriver, + () -> DEFAULT_IDLE_SAFE_TIME_PROPAGATION_PERIOD_MILLISECONDS + ); + } + }; + txTestCluster.prepareCluster(); + + testTable = txTestCluster.startTable(TABLE_NAME, EMP_TABLE_ID, TABLE_SCHEMA); + + log.info("Tables have been started"); + } + + @AfterEach + public void after() throws Exception { + txTestCluster.shutdownCluster(); + } + + @Test + @Disabled("https://issues.apache.org/jira/browse/IGNITE-20894";) + public void testDeadlockRecovery() { + RecordView<Tuple> view = testTable.recordView(); + Tuple t1 = tuple(0, "0"); + assertTrue(view.insert(null, t1)); + + Tuple t2 = tuple(1, "1"); + assertTrue(view.insert(null, t2)); + + InternalTransaction tx1 = (InternalTransaction) txTestCluster.igniteTransactions().begin(); + InternalTransaction tx2 = (InternalTransaction) txTestCluster.igniteTransactions().begin(); + + LOG.info("id1={}", tx1.id()); + LOG.info("id2={}", tx2.id()); + + assertTrue(tx2.id().compareTo(tx1.id()) > 0); + + Tuple t10 = view.get(tx1, keyTuple(0)); + Tuple t21 = view.get(tx2, keyTuple(1)); + + assertEquals(t1.stringValue("name"), t10.stringValue("name")); + assertEquals(t2.stringValue("name"), t21.stringValue("name")); + + view.upsertAsync(tx1, tuple(1, "11")); + view.upsertAsync(tx2, tuple(0, "00")); + + assertTrue(TestUtils.waitForCondition(() -> { + int total = 0; + HeapLockManager lockManager = (HeapLockManager) txTestCluster.txManagers.get(txTestCluster.localNodeName).lockManager(); + for (int j = 0; j < lockManager.getSlots().length; j++) { + LockState slot = lockManager.getSlots()[j]; + + total += slot.waitersCount(); + } + + return total == 8; + }, 10_000), "Some lockers are missing"); + + tx1.commit(); + } + + /** + * Test that a lock table behaves correctly in case of lock cache overflow. + */ + @Test + public void testCollision() { + RecordView<Tuple> view = testTable.recordView(); + + int i = 0; + final int count = 1000; + List<Transaction> txns = new ArrayList<>(); + while (i++ < count) { + Transaction tx = txTestCluster.igniteTransactions().begin(); + view.insertAsync(tx, tuple(i, "x-" + i)); + txns.add(tx); + } + + assertTrue(TestUtils.waitForCondition(() -> { + int total = 0; + HeapLockManager lockManager = (HeapLockManager) txTestCluster.txManagers.get(txTestCluster.localNodeName).lockManager(); + for (int j = 0; j < lockManager.getSlots().length; j++) { + LockState slot = lockManager.getSlots()[j]; + total += slot.waitersCount(); + } + + return total == count && lockManager.available() == 0; + }, 10_000), "Some lockers are missing"); + + int empty = 0; + int coll = 0; + + HeapLockManager lm = (HeapLockManager) txTestCluster.txManagers.get(txTestCluster.localNodeName).lockManager(); + for (int j = 0; j < lm.getSlots().length; j++) { + LockState slot = lm.getSlots()[j]; + int cnt = slot.waitersCount(); + if (cnt == 0) { + empty++; + } + if (cnt > 1) { + coll += cnt; + } + } + + LOG.info("LockTable [emptySlots={} collisions={}]", empty, coll); + + assertTrue(coll > 0); Review Comment: That'll be true if there is at least one slot.waitersCount() > 0 and within given test it's always true because on line 213 we've asserted that slot.waiters() accumulator == 1000. Am I missing something? ########## modules/table/src/main/java/org/apache/ignite/internal/table/distributed/replicator/PartitionReplicaListener.java: ########## @@ -1566,6 +1570,7 @@ private CompletableFuture<Void> cleanup( UUID txId, int attemptsToCleanupReplica ) { + // Avoid invoking async chain in raft threads. Review Comment: Why it's here? Is it actually a todo? or you mean that cleanup itself should be async or what? ########## modules/table/src/main/java/org/apache/ignite/internal/table/distributed/storage/InternalTableImpl.java: ########## @@ -256,13 +257,15 @@ public String name() { * @param tx The transaction, not null if explicit. * @param fac Replica requests factory. * @param noWriteChecker Used to handle operations producing no updates. + * @param retryOnLockConflict {@code True} to retry on lock conflict. * @return The future. */ private <R> CompletableFuture<R> enlistInTx( BinaryRowEx row, @Nullable InternalTransaction tx, IgniteTriFunction<InternalTransaction, ReplicationGroupId, Long, ReplicaRequest> fac, - BiPredicate<R, ReplicaRequest> noWriteChecker + BiPredicate<R, ReplicaRequest> noWriteChecker, + boolean retryOnLockConflict Review Comment: So, for now given logic is not available for SQL, right? ########## modules/table/src/main/java/org/apache/ignite/internal/table/distributed/storage/InternalTableImpl.java: ########## @@ -431,7 +437,7 @@ private CompletableFuture<Collection<BinaryRow>> enlistCursorInTx( if (primaryReplicaAndTerm != null) { fut = replicaSvc.invoke(primaryReplicaAndTerm.get1(), mapFunc.apply(primaryReplicaAndTerm.get2())); } else { - fut = enlistWithRetry(tx, partId, mapFunc, ATTEMPTS_TO_ENLIST_PARTITION, false, null); + fut = enlistWithRetry(tx, partId, mapFunc, ATTEMPTS_TO_ENLIST_PARTITION, false, null, false); Review Comment: Details required. If it's "page"-specific retry it should be possible to do it, is it? ########## modules/table/src/testFixtures/java/org/apache/ignite/internal/table/impl/DummyInternalTableImpl.java: ########## @@ -101,19 +102,19 @@ import org.apache.ignite.network.TopologyService; import org.apache.ignite.tx.TransactionException; import org.jetbrains.annotations.Nullable; +import org.jetbrains.annotations.TestOnly; /** * Dummy table storage implementation. */ +@TestOnly Review Comment: Given class is in testFixtures, so it's intentionally @TestOnly. ########## modules/transactions/src/main/java/org/apache/ignite/internal/tx/LockKey.java: ########## @@ -73,6 +75,12 @@ public boolean equals(Object o) { @Override public int hashCode() { + // Apply more efficient hashing to byte buffers to decrease collisions + if (key instanceof ByteBuffer) { + ByteBuffer key1 = (ByteBuffer) key; + return HashUtils.hash32(HashUtils.hash64(key1, 0, key1.capacity(), contextId != null ? contextId.hashCode() : 0)); Review Comment: hash32 calls hash64 internally ``` public static int hash32(long data, int seed) { long hash = hash64(data, seed); return (int) (hash ^ (hash >>> 32)); } ``` ########## modules/transactions/src/main/java/org/apache/ignite/internal/tx/impl/HeapLockManager.java: ########## @@ -51,53 +55,121 @@ import org.apache.ignite.internal.tx.event.LockEvent; import org.apache.ignite.internal.tx.event.LockEventParameters; import org.jetbrains.annotations.Nullable; +import org.jetbrains.annotations.TestOnly; /** * A {@link LockManager} implementation which stores lock queues in the heap. * * <p>Lock waiters are placed in the queue, ordered according to comparator provided by {@link HeapLockManager#deadlockPreventionPolicy}. - * When a new waiter is placed in the queue, it's validated against current lock owner: if there is an owner with a higher transaction id - * lock request is denied. + * When a new waiter is placed in the queue, it's validated against current lock owner: if there is an owner with a higher priority (as + * defined by comparator) lock request is denied. * * <p>Read lock can be upgraded to write lock (only available for the lowest read-locked entry of * the queue). + * + * <p>Additionally limits the lock map size. Review Comment: What will happen if it'll be exceeded? Exception? If true - which one? Do we have test for that? ########## modules/table/src/integrationTest/java/org/apache/ignite/distributed/ItTxDistributedTestSingleNode.java: ########## @@ -148,6 +148,7 @@ public void before() throws Exception { @AfterEach public void after() throws Exception { txTestCluster.shutdownCluster(); + Mockito.framework().clearInlineMocks(); Review Comment: Could you please extract mockito related changes to the separate PR. Earlier you've mentioned that > Some memory leaks in tests related to mockito were fixed. It is recommended to have clearInlineMocks for each method instead of a suite, because the suite can be quite big and produce OOMs with a current lock table size. > In certain specific, rare scenarios (issue https://github.com/mockito/mockito/issues/1614) inline mocking causes memory leaks. There is no clean way to mitigate this problem completely. Hence, we introduced a new API to explicitly clear mock state (only make sense in inline mocking!): 1. It would be nice to have an examples and explanations of such scenarios. 2. Is it still possible to use static mocks? 3. Why should we have `Mockito.framework().clearInlineMocks();` inside `ItTxDistributedTestSingleNode` if we already have one inside `BaseIgniteAbstractTest`. Is it enough to just move clearInlineMocks from @AfterAll to @AfterEach in `BaseIgniteAbstractTest`? In any case, it would be nice to split such Mockito related stuff from lock enhancements. ########## modules/table/src/integrationTest/java/org/apache/ignite/distributed/ItLockTableTest.java: ########## @@ -0,0 +1,274 @@ +/* + * 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.distributed; + +import static org.apache.ignite.internal.replicator.ReplicaManager.DEFAULT_IDLE_SAFE_TIME_PROPAGATION_PERIOD_MILLISECONDS; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.util.ArrayList; +import java.util.List; +import java.util.concurrent.CompletableFuture; +import org.apache.ignite.internal.configuration.testframework.ConfigurationExtension; +import org.apache.ignite.internal.configuration.testframework.InjectConfiguration; +import org.apache.ignite.internal.hlc.HybridClock; +import org.apache.ignite.internal.logger.IgniteLogger; +import org.apache.ignite.internal.logger.Loggers; +import org.apache.ignite.internal.placementdriver.PlacementDriver; +import org.apache.ignite.internal.raft.configuration.RaftConfiguration; +import org.apache.ignite.internal.replicator.ReplicaService; +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.table.TableViewInternal; +import org.apache.ignite.internal.testframework.IgniteAbstractTest; +import org.apache.ignite.internal.tx.DeadlockPreventionPolicy; +import org.apache.ignite.internal.tx.HybridTimestampTracker; +import org.apache.ignite.internal.tx.InternalTransaction; +import org.apache.ignite.internal.tx.impl.HeapLockManager; +import org.apache.ignite.internal.tx.impl.HeapLockManager.LockState; +import org.apache.ignite.internal.tx.impl.HeapUnboundedLockManager; +import org.apache.ignite.internal.tx.impl.TransactionIdGenerator; +import org.apache.ignite.internal.tx.impl.TxManagerImpl; +import org.apache.ignite.internal.type.NativeTypes; +import org.apache.ignite.network.ClusterNode; +import org.apache.ignite.network.ClusterService; +import org.apache.ignite.raft.jraft.test.TestUtils; +import org.apache.ignite.table.RecordView; +import org.apache.ignite.table.Tuple; +import org.apache.ignite.tx.Transaction; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Disabled; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.TestInfo; +import org.junit.jupiter.api.extension.ExtendWith; + +/** + * Test lock table. + */ +@ExtendWith(ConfigurationExtension.class) +public class ItLockTableTest extends IgniteAbstractTest { + private static final IgniteLogger LOG = Loggers.forClass(ItLockTableTest.class); + + private static int EMP_TABLE_ID = 2; + + private static final int CACHE_SIZE = 10; + + private static final String TABLE_NAME = "test"; + + private static SchemaDescriptor TABLE_SCHEMA = new SchemaDescriptor( + 1, + new Column[]{new Column("id".toUpperCase(), NativeTypes.INT32, false)}, + new Column[]{ + new Column("name".toUpperCase(), NativeTypes.STRING, true), + new Column("salary".toUpperCase(), NativeTypes.DOUBLE, true) + } + ); + + protected TableViewInternal testTable; + + protected final TestInfo testInfo; + + //TODO fsync can be turned on again after https://issues.apache.org/jira/browse/IGNITE-20195 + @InjectConfiguration("mock: { fsync: false }") + protected static RaftConfiguration raftConfiguration; + + @InjectConfiguration + protected static GcConfiguration gcConfig; + + private ItTxTestCluster txTestCluster; + + private HybridTimestampTracker timestampTracker = new HybridTimestampTracker(); + + /** + * The constructor. + * + * @param testInfo Test info. + */ + public ItLockTableTest(TestInfo testInfo) { + this.testInfo = testInfo; + } + + @BeforeEach + public void before() throws Exception { + txTestCluster = new ItTxTestCluster( + testInfo, + raftConfiguration, + workDir, + 1, + 1, + false, + timestampTracker + ) { + @Override + protected TxManagerImpl newTxManager( + ClusterService clusterService, + ReplicaService replicaSvc, + HybridClock clock, + TransactionIdGenerator generator, + ClusterNode node, + PlacementDriver placementDriver + ) { + return new TxManagerImpl( + clusterService, + replicaSvc, + new HeapLockManager( + DeadlockPreventionPolicy.NO_OP, + HeapLockManager.SLOTS, + CACHE_SIZE, + new HeapUnboundedLockManager()), + clock, + generator, + placementDriver, + () -> DEFAULT_IDLE_SAFE_TIME_PROPAGATION_PERIOD_MILLISECONDS + ); + } + }; + txTestCluster.prepareCluster(); + + testTable = txTestCluster.startTable(TABLE_NAME, EMP_TABLE_ID, TABLE_SCHEMA); + + log.info("Tables have been started"); + } + + @AfterEach + public void after() throws Exception { + txTestCluster.shutdownCluster(); + } + + @Test + @Disabled("https://issues.apache.org/jira/browse/IGNITE-20894";) + public void testDeadlockRecovery() { + RecordView<Tuple> view = testTable.recordView(); + Tuple t1 = tuple(0, "0"); + assertTrue(view.insert(null, t1)); + + Tuple t2 = tuple(1, "1"); + assertTrue(view.insert(null, t2)); + + InternalTransaction tx1 = (InternalTransaction) txTestCluster.igniteTransactions().begin(); + InternalTransaction tx2 = (InternalTransaction) txTestCluster.igniteTransactions().begin(); + + LOG.info("id1={}", tx1.id()); + LOG.info("id2={}", tx2.id()); + + assertTrue(tx2.id().compareTo(tx1.id()) > 0); + + Tuple t10 = view.get(tx1, keyTuple(0)); + Tuple t21 = view.get(tx2, keyTuple(1)); + + assertEquals(t1.stringValue("name"), t10.stringValue("name")); + assertEquals(t2.stringValue("name"), t21.stringValue("name")); + + view.upsertAsync(tx1, tuple(1, "11")); + view.upsertAsync(tx2, tuple(0, "00")); + + assertTrue(TestUtils.waitForCondition(() -> { + int total = 0; + HeapLockManager lockManager = (HeapLockManager) txTestCluster.txManagers.get(txTestCluster.localNodeName).lockManager(); + for (int j = 0; j < lockManager.getSlots().length; j++) { + LockState slot = lockManager.getSlots()[j]; + + total += slot.waitersCount(); + } + + return total == 8; + }, 10_000), "Some lockers are missing"); + + tx1.commit(); + } + + /** + * Test that a lock table behaves correctly in case of lock cache overflow. + */ + @Test + public void testCollision() { + RecordView<Tuple> view = testTable.recordView(); + + int i = 0; + final int count = 1000; + List<Transaction> txns = new ArrayList<>(); + while (i++ < count) { + Transaction tx = txTestCluster.igniteTransactions().begin(); + view.insertAsync(tx, tuple(i, "x-" + i)); + txns.add(tx); + } + + assertTrue(TestUtils.waitForCondition(() -> { + int total = 0; + HeapLockManager lockManager = (HeapLockManager) txTestCluster.txManagers.get(txTestCluster.localNodeName).lockManager(); + for (int j = 0; j < lockManager.getSlots().length; j++) { + LockState slot = lockManager.getSlots()[j]; + total += slot.waitersCount(); + } + + return total == count && lockManager.available() == 0; Review Comment: Taking into the consideration the fact that each insert takes several locks (index_key, table, table_key) what do we check here? ########## modules/table/src/main/java/org/apache/ignite/internal/table/distributed/storage/InternalTableImpl.java: ########## @@ -465,25 +472,31 @@ private <R> CompletableFuture<R> enlistWithRetry( Function<Long, ReplicaRequest> mapFunc, int attempts, boolean full, - @Nullable BiPredicate<R, ReplicaRequest> noWriteChecker + @Nullable BiPredicate<R, ReplicaRequest> noWriteChecker, + boolean retryOnLockConflict ) { - return enlist(partId, tx) - .thenCompose(primaryReplicaAndTerm -> trackingInvoke(tx, partId, mapFunc, full, primaryReplicaAndTerm, noWriteChecker)) - .handle((response, e) -> { - if (e == null) { - return completedFuture(response); - } + return (CompletableFuture<R>) enlist(partId, tx) + .thenCompose(primaryReplicaAndTerm -> trackingInvoke(tx, partId, mapFunc, full, primaryReplicaAndTerm, noWriteChecker, + retryOnLockConflict)) + .handle((res0, e) -> { + if (e != null) { + // We can safely retry indefinitely on deadlock prevention. + if (retryOnLockConflict && e.getCause() instanceof LockException) { + return enlistWithRetry(tx, partId, mapFunc, attempts, full, noWriteChecker, true); Review Comment: Well, currently we will hang the retrying transaction forever, because there's no tx timeout right now. In any case I believe that it should be configurable. As a user I may desire fail fast approach in case of a deadlock instead of retries until tx times out. ########## modules/table/src/integrationTest/java/org/apache/ignite/distributed/ItLockTableTest.java: ########## @@ -0,0 +1,274 @@ +/* + * 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.distributed; + +import static org.apache.ignite.internal.replicator.ReplicaManager.DEFAULT_IDLE_SAFE_TIME_PROPAGATION_PERIOD_MILLISECONDS; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.util.ArrayList; +import java.util.List; +import java.util.concurrent.CompletableFuture; +import org.apache.ignite.internal.configuration.testframework.ConfigurationExtension; +import org.apache.ignite.internal.configuration.testframework.InjectConfiguration; +import org.apache.ignite.internal.hlc.HybridClock; +import org.apache.ignite.internal.logger.IgniteLogger; +import org.apache.ignite.internal.logger.Loggers; +import org.apache.ignite.internal.placementdriver.PlacementDriver; +import org.apache.ignite.internal.raft.configuration.RaftConfiguration; +import org.apache.ignite.internal.replicator.ReplicaService; +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.table.TableViewInternal; +import org.apache.ignite.internal.testframework.IgniteAbstractTest; +import org.apache.ignite.internal.tx.DeadlockPreventionPolicy; +import org.apache.ignite.internal.tx.HybridTimestampTracker; +import org.apache.ignite.internal.tx.InternalTransaction; +import org.apache.ignite.internal.tx.impl.HeapLockManager; +import org.apache.ignite.internal.tx.impl.HeapLockManager.LockState; +import org.apache.ignite.internal.tx.impl.HeapUnboundedLockManager; +import org.apache.ignite.internal.tx.impl.TransactionIdGenerator; +import org.apache.ignite.internal.tx.impl.TxManagerImpl; +import org.apache.ignite.internal.type.NativeTypes; +import org.apache.ignite.network.ClusterNode; +import org.apache.ignite.network.ClusterService; +import org.apache.ignite.raft.jraft.test.TestUtils; +import org.apache.ignite.table.RecordView; +import org.apache.ignite.table.Tuple; +import org.apache.ignite.tx.Transaction; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Disabled; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.TestInfo; +import org.junit.jupiter.api.extension.ExtendWith; + +/** + * Test lock table. + */ +@ExtendWith(ConfigurationExtension.class) +public class ItLockTableTest extends IgniteAbstractTest { + private static final IgniteLogger LOG = Loggers.forClass(ItLockTableTest.class); + + private static int EMP_TABLE_ID = 2; + + private static final int CACHE_SIZE = 10; + + private static final String TABLE_NAME = "test"; + + private static SchemaDescriptor TABLE_SCHEMA = new SchemaDescriptor( + 1, + new Column[]{new Column("id".toUpperCase(), NativeTypes.INT32, false)}, + new Column[]{ + new Column("name".toUpperCase(), NativeTypes.STRING, true), + new Column("salary".toUpperCase(), NativeTypes.DOUBLE, true) + } + ); + + protected TableViewInternal testTable; + + protected final TestInfo testInfo; + + //TODO fsync can be turned on again after https://issues.apache.org/jira/browse/IGNITE-20195 + @InjectConfiguration("mock: { fsync: false }") + protected static RaftConfiguration raftConfiguration; + + @InjectConfiguration + protected static GcConfiguration gcConfig; + + private ItTxTestCluster txTestCluster; + + private HybridTimestampTracker timestampTracker = new HybridTimestampTracker(); + + /** + * The constructor. + * + * @param testInfo Test info. + */ + public ItLockTableTest(TestInfo testInfo) { + this.testInfo = testInfo; + } + + @BeforeEach + public void before() throws Exception { + txTestCluster = new ItTxTestCluster( + testInfo, + raftConfiguration, + workDir, + 1, + 1, + false, + timestampTracker + ) { + @Override + protected TxManagerImpl newTxManager( + ClusterService clusterService, + ReplicaService replicaSvc, + HybridClock clock, + TransactionIdGenerator generator, + ClusterNode node, + PlacementDriver placementDriver + ) { + return new TxManagerImpl( + clusterService, + replicaSvc, + new HeapLockManager( + DeadlockPreventionPolicy.NO_OP, + HeapLockManager.SLOTS, + CACHE_SIZE, + new HeapUnboundedLockManager()), + clock, + generator, + placementDriver, + () -> DEFAULT_IDLE_SAFE_TIME_PROPAGATION_PERIOD_MILLISECONDS + ); + } + }; + txTestCluster.prepareCluster(); + + testTable = txTestCluster.startTable(TABLE_NAME, EMP_TABLE_ID, TABLE_SCHEMA); + + log.info("Tables have been started"); + } + + @AfterEach + public void after() throws Exception { + txTestCluster.shutdownCluster(); + } + + @Test + @Disabled("https://issues.apache.org/jira/browse/IGNITE-20894";) + public void testDeadlockRecovery() { + RecordView<Tuple> view = testTable.recordView(); + Tuple t1 = tuple(0, "0"); + assertTrue(view.insert(null, t1)); + + Tuple t2 = tuple(1, "1"); + assertTrue(view.insert(null, t2)); + + InternalTransaction tx1 = (InternalTransaction) txTestCluster.igniteTransactions().begin(); + InternalTransaction tx2 = (InternalTransaction) txTestCluster.igniteTransactions().begin(); + + LOG.info("id1={}", tx1.id()); + LOG.info("id2={}", tx2.id()); + + assertTrue(tx2.id().compareTo(tx1.id()) > 0); + + Tuple t10 = view.get(tx1, keyTuple(0)); + Tuple t21 = view.get(tx2, keyTuple(1)); + + assertEquals(t1.stringValue("name"), t10.stringValue("name")); + assertEquals(t2.stringValue("name"), t21.stringValue("name")); + + view.upsertAsync(tx1, tuple(1, "11")); + view.upsertAsync(tx2, tuple(0, "00")); + + assertTrue(TestUtils.waitForCondition(() -> { + int total = 0; + HeapLockManager lockManager = (HeapLockManager) txTestCluster.txManagers.get(txTestCluster.localNodeName).lockManager(); + for (int j = 0; j < lockManager.getSlots().length; j++) { + LockState slot = lockManager.getSlots()[j]; + + total += slot.waitersCount(); + } + + return total == 8; Review Comment: Could you please add a comment explaining why it's 8. 1. view.get(tx1, keyTuple(0)); [S_commit(index(keyTuple(0)))] + IS_commit(table) S_commit(keyTuple(0)) // 3 locks total 2. view.get(tx2, keyTuple(1)); [S_commit(index(keyTuple(1)))] + IS_commit(table) S_commit(keyTuple(1)) // 6 locks total 3. view.upsertAsync(tx1, tuple(1, "11")); [X_commit(index(keyTuple(1)))] // 7 locks total and a lock conflict on an index(keyTuple(1)) 4. view.upsertAsync(tx2, tuple(0, "00")); [X_commit(index(keyTuple(0)))] // 8 locks total and a lock conflict on an index(keyTuple(0)) Is that correct or we skip the index communication part? In any case, please add an explanation within code. ########## modules/table/src/main/java/org/apache/ignite/internal/table/RecordBinaryViewImpl.java: ########## @@ -430,7 +431,16 @@ public CompletableFuture<Void> streamData(Publisher<Tuple> publisher, @Nullable var partitioner = new TupleStreamerPartitionAwarenessProvider(rowConverter.registry(), tbl.partitions()); StreamerBatchSender<Tuple, Integer> batchSender = (partitionId, items) -> withSchemaSync(null, (schemaVersion) -> { - return this.tbl.upsertAll(mapToBinary(items, schemaVersion, false), partitionId); + return this.tbl.upsertAll(mapToBinary(items, schemaVersion, false), partitionId).thenAccept(new Consumer<Void>() { + @Override + public void accept(Void unused) { Review Comment: Test code to remove. ########## modules/runner/src/integrationTest/java/org/apache/ignite/internal/benchmark/LockManagerBenchmark.java: ########## @@ -0,0 +1,119 @@ +/* + * 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.benchmark; + +import java.util.ArrayList; +import java.util.List; +import java.util.UUID; +import java.util.concurrent.TimeUnit; +import org.apache.ignite.internal.TestHybridClock; +import org.apache.ignite.internal.hlc.HybridClock; +import org.apache.ignite.internal.storage.RowId; +import org.apache.ignite.internal.tx.LockKey; +import org.apache.ignite.internal.tx.LockManager; +import org.apache.ignite.internal.tx.LockMode; +import org.apache.ignite.internal.tx.impl.HeapLockManager; +import org.apache.ignite.internal.tx.impl.TransactionIdGenerator; +import org.openjdk.jmh.annotations.Benchmark; +import org.openjdk.jmh.annotations.Measurement; +import org.openjdk.jmh.annotations.Mode; +import org.openjdk.jmh.annotations.OutputTimeUnit; +import org.openjdk.jmh.annotations.Param; +import org.openjdk.jmh.annotations.Scope; +import org.openjdk.jmh.annotations.Setup; +import org.openjdk.jmh.annotations.State; +import org.openjdk.jmh.annotations.TearDown; +import org.openjdk.jmh.annotations.Warmup; +import org.openjdk.jmh.runner.Runner; +import org.openjdk.jmh.runner.RunnerException; +import org.openjdk.jmh.runner.options.Options; +import org.openjdk.jmh.runner.options.OptionsBuilder; + +/** + * Benchmark lock manager. + */ +@State(Scope.Benchmark) +@OutputTimeUnit(TimeUnit.MICROSECONDS) +public class LockManagerBenchmark { + private LockManager lockManager; + private TransactionIdGenerator generator; + private HybridClock clock; + + /** + * Initializes session and statement. + */ + @Setup + public void setUp() { + lockManager = new HeapLockManager(); + generator = new TransactionIdGenerator(0); + clock = new TestHybridClock(() -> 0L); + } + + /** + * Closes resources. + */ + @TearDown + public void tearDown() throws Exception { + if (!lockManager.isEmpty()) { + throw new AssertionError("Invalid lock manager state"); + } + } + + /** + * Concurrent active transactions. + */ + @Param({"200"}) + private int concTxns; + + /** + * Take and release some locks. + */ + @Benchmark + @Warmup(iterations = 1, time = 3) + @Measurement(iterations = 1, time = 10) + public void lockCommit() { + List<UUID> ids = new ArrayList<>(concTxns); + + int c = 0; + + for (int i = 0; i < concTxns; i++) { + UUID txId = generator.transactionIdFor(clock.now()); + ids.add(txId); + lockManager.acquire(txId, new LockKey(0, new RowId(0, new UUID(0, c++))), LockMode.X).join(); Review Comment: So, within given benchmark you never face lock contention, right? ########## modules/table/src/main/java/org/apache/ignite/internal/table/distributed/replicator/PartitionReplicaListener.java: ########## @@ -2118,10 +2123,10 @@ private CompletableFuture<List<BinaryRow>> processReadOnlyDirectMultiEntryAction */ private CompletableFuture<ReplicaResult> processMultiEntryAction(ReadWriteMultiRowReplicaRequest request, String txCoordinatorId) { UUID txId = request.transactionId(); - TablePartitionId committedPartitionId = request.commitPartitionId().asTablePartitionId(); + TablePartitionId commitdPartitionId = request.commitPartitionId().asTablePartitionId(); Review Comment: Why? ########## modules/table/src/main/java/org/apache/ignite/internal/table/distributed/storage/InternalTableImpl.java: ########## @@ -465,25 +472,31 @@ private <R> CompletableFuture<R> enlistWithRetry( Function<Long, ReplicaRequest> mapFunc, int attempts, boolean full, - @Nullable BiPredicate<R, ReplicaRequest> noWriteChecker + @Nullable BiPredicate<R, ReplicaRequest> noWriteChecker, + boolean retryOnLockConflict ) { - return enlist(partId, tx) - .thenCompose(primaryReplicaAndTerm -> trackingInvoke(tx, partId, mapFunc, full, primaryReplicaAndTerm, noWriteChecker)) - .handle((response, e) -> { - if (e == null) { - return completedFuture(response); - } + return (CompletableFuture<R>) enlist(partId, tx) + .thenCompose(primaryReplicaAndTerm -> trackingInvoke(tx, partId, mapFunc, full, primaryReplicaAndTerm, noWriteChecker, + retryOnLockConflict)) + .handle((res0, e) -> { + if (e != null) { + // We can safely retry indefinitely on deadlock prevention. + if (retryOnLockConflict && e.getCause() instanceof LockException) { Review Comment: From my point of view, several approaches of a retry logic are possible: 1. Retry on the tx level. ``` tx = transactions.begin(); view.insert(tx, k1,v1); // Succeeds view.insert(tx, k2, v2); // Deadlock here ``` In that case we can relase all tx locks and retry an entire transaction. Should not be possible if user had chance to see the results of any operations (commonly reads). 2. Retry on operation level ``` tx = transactions.begin(); view.insert(tx, k1,v1); // Succeeds [1] view.insertAll(tx, (k2,v2), (k3,v3)); // Lock(k2) succeeds however k3faced a lock conflict. [2] ``` In that case we may release all locks that were acquired by insertAll [2] but not insert [1] and then retry the insertAll. AFAIK it's the one that was implemented, right? 3. Retry on key level. May be implemented within DeadlockPreventionLogic itself without even throwing LockException. 3 is the same as 2 in case of single key operation. At the moment of writing this comment I didn't check code below, so I don't know how it's implemented. Any comments and explanations will be helpful. ########## modules/table/src/main/java/org/apache/ignite/internal/table/distributed/storage/InternalTableImpl.java: ########## @@ -958,7 +987,8 @@ public CompletableFuture<Void> upsertAll(Collection<BinaryRowEx> rows, InternalT tx, this::upsertAllInternal, RowBatch::allResultFutures, - (res, req) -> false + (res, req) -> false, + false Review Comment: Here and in some other places, why it's `false`? ########## modules/transactions/src/main/java/org/apache/ignite/internal/tx/DeadlockPreventionPolicy.java: ########## @@ -27,6 +27,11 @@ * See also {@link org.apache.ignite.internal.tx.impl.HeapLockManager}. */ public interface DeadlockPreventionPolicy { + /** + * No-op policy which does nothing to prevent deadlocks. + */ + DeadlockPreventionPolicy NO_OP = new DeadlockPreventionPolicy() {}; Review Comment: Minor. Because we have NO_OP explicitly we may use it in `NoneDeadlockPreventionTest`. ########## modules/transactions/src/main/java/org/apache/ignite/internal/tx/DeadlockPreventionPolicy.java: ########## @@ -27,6 +27,11 @@ * See also {@link org.apache.ignite.internal.tx.impl.HeapLockManager}. */ public interface DeadlockPreventionPolicy { + /** + * No-op policy which does nothing to prevent deadlocks. + */ + DeadlockPreventionPolicy NO_OP = new DeadlockPreventionPolicy() {}; Review Comment: Minor. Because we have NO_OP explicitly we may use it in `NoneDeadlockPreventionTest`. ########## modules/transactions/src/main/java/org/apache/ignite/internal/tx/LockManager.java: ########## @@ -55,12 +56,20 @@ public interface LockManager extends EventProducer<LockEvent, LockEventParameter void release(UUID txId, LockKey lockKey, LockMode lockMode); /** - * Retrieves all locks for the specified transaction id. + * Retrieves all waiters for the specified transaction id. Review Comment: Why 'waiters'? In other places 'lock' is used, e.g. `Release all locks associated with a transaction.` ########## modules/transactions/src/main/java/org/apache/ignite/internal/tx/impl/HeapLockManager.java: ########## @@ -51,53 +55,121 @@ import org.apache.ignite.internal.tx.event.LockEvent; import org.apache.ignite.internal.tx.event.LockEventParameters; import org.jetbrains.annotations.Nullable; +import org.jetbrains.annotations.TestOnly; /** * A {@link LockManager} implementation which stores lock queues in the heap. * * <p>Lock waiters are placed in the queue, ordered according to comparator provided by {@link HeapLockManager#deadlockPreventionPolicy}. - * When a new waiter is placed in the queue, it's validated against current lock owner: if there is an owner with a higher transaction id - * lock request is denied. + * When a new waiter is placed in the queue, it's validated against current lock owner: if there is an owner with a higher priority (as + * defined by comparator) lock request is denied. * * <p>Read lock can be upgraded to write lock (only available for the lowest read-locked entry of * the queue). + * + * <p>Additionally limits the lock map size. */ public class HeapLockManager extends AbstractEventProducer<LockEvent, LockEventParameters> implements LockManager { - private ConcurrentHashMap<LockKey, LockState> locks = new ConcurrentHashMap<>(); + /** + * Table size. TODO make it configurable IGNITE-20694 + */ + public static final int SLOTS = 131072; //16536; Review Comment: 16536 What's that? And why it's 131072? ########## modules/transactions/src/main/java/org/apache/ignite/internal/tx/LockManager.java: ########## @@ -36,14 +36,15 @@ public interface LockManager extends EventProducer<LockEvent, LockEventParameter * @param lockMode Lock mode, for example shared, exclusive, intention-shared etc. * @return The future with gained lock that will be completed when a lock is successfully acquired. */ - public CompletableFuture<Lock> acquire(UUID txId, LockKey lockKey, LockMode lockMode); + CompletableFuture<Lock> acquire(UUID txId, LockKey lockKey, LockMode lockMode); /** * Attempts to release the specified lock. * * @param lock Lock to release. */ - public void release(Lock lock); + @TestOnly Review Comment: It's also called from within org.apache.ignite.internal.tx.impl.HeapUnboundedLockManager#releaseAll. BTW it's strange. Does it mean that we don't have checks that verify that @TestOnly code is only used from within tests? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
