This is an automated email from the ASF dual-hosted git repository. ilyak pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/ignite.git
The following commit(s) were added to refs/heads/master by this push: new 6e3e23a IGNITE-6804 Dev warning if unordered collection passed to bulk cache op - Fixes #6976. 6e3e23a is described below commit 6e3e23a78ff62a3acf6a72e1ea0b4afe46a0b94a Author: Ilya Kasnacheev <ilya.kasnach...@gmail.com> AuthorDate: Tue Feb 11 16:35:59 2020 +0300 IGNITE-6804 Dev warning if unordered collection passed to bulk cache op - Fixes #6976. Signed-off-by: Ilya Kasnacheev <ilya.kasnach...@gmail.com> --- .../main/java/org/apache/ignite/IgniteCache.java | 40 ++- .../processors/cache/GridCacheAdapter.java | 101 ++++++ .../distributed/dht/atomic/GridDhtAtomicCache.java | 26 +- .../dht/colocated/GridDhtColocatedCache.java | 2 + .../distributed/near/GridNearAtomicCache.java | 2 + .../near/GridNearTransactionalCache.java | 2 + .../cache/local/atomic/GridLocalAtomicCache.java | 13 +- .../GridCacheHashMapPutAllWarningsTest.java | 384 +++++++++++++++++++++ .../cache/GridCacheAbstractLocalStoreSelfTest.java | 15 +- .../cache/GridCacheAbstractMetricsSelfTest.java | 7 +- ...acheAtomicEntryProcessorDeploymentSelfTest.java | 4 +- .../GridCacheInterceptorAbstractSelfTest.java | 6 +- .../processors/cache/IgniteCacheGroupsTest.java | 2 +- .../cache/IgniteClientCacheStartFailoverTest.java | 4 +- .../GridCacheTransformEventSelfTest.java | 4 +- .../atomic/AtomicPutAllChangingTopologyTest.java | 4 +- .../near/GridNearCacheStoreUpdateTest.java | 10 +- .../IgniteCacheStoreSessionAbstractTest.java | 12 +- .../cache/transactions/TxRollbackAsyncTest.java | 4 +- .../transactions/TxRollbackOnTimeoutTest.java | 4 +- .../junits/common/GridCommonAbstractTest.java | 3 +- .../ignite/testsuites/IgniteCacheTestSuite5.java | 3 + .../processors/cache/BigEntryQueryTest.java | 7 +- .../cpp/core/include/ignite/cache/cache.h | 6 + .../dotnet/Apache.Ignite.Core/Cache/ICache.cs | 24 ++ 25 files changed, 637 insertions(+), 52 deletions(-) diff --git a/modules/core/src/main/java/org/apache/ignite/IgniteCache.java b/modules/core/src/main/java/org/apache/ignite/IgniteCache.java index 0e59a78..49c8908 100644 --- a/modules/core/src/main/java/org/apache/ignite/IgniteCache.java +++ b/modules/core/src/main/java/org/apache/ignite/IgniteCache.java @@ -643,6 +643,10 @@ public interface IgniteCache<K, V> extends javax.cache.Cache<K, V>, IgniteAsyncS * <p> * Please refer to documentation for {@link CacheAtomicityMode#ATOMIC} for information on * system behavior in crash scenarios for atomic caches. + * <p> + * Keys are locked in the order in which they appear in map. It is caller's responsibility to + * make sure keys always follow same order, such as by using {@link java.util.TreeMap}. Using unordered map, + * such as {@link java.util.HashMap}, while calling this method in parallel <b>will lead to deadlock</b>. * * @param map Map containing keys and entry processors to be applied to values. * @param args Additional arguments to pass to the {@link EntryProcessor}. @@ -657,7 +661,7 @@ public interface IgniteCache<K, V> extends javax.cache.Cache<K, V>, IgniteAsyncS Object... args) throws TransactionException; /** - * Asynchronously version of the {@link #invokeAll(Map, Object...)} method. + * Asynchronous version of the {@link #invokeAll(Map, Object...)} method. * * @param map Map containing keys and entry processors to be applied to values. * @param args Additional arguments to pass to the {@link EntryProcessor}. @@ -902,6 +906,11 @@ public interface IgniteCache<K, V> extends javax.cache.Cache<K, V>, IgniteAsyncS /** * {@inheritDoc} + * <p> + * Keys are locked in the order in which they appear in map. It is caller's responsibility to + * make sure keys always follow same order, such as by using {@link java.util.TreeMap}. Using unordered map, + * such as {@link java.util.HashMap}, while calling this method in parallel <b>will lead to deadlock</b>. + * * @throws TransactionException If operation within transaction is failed. */ @IgniteAsyncSupported @@ -923,6 +932,10 @@ public interface IgniteCache<K, V> extends javax.cache.Cache<K, V>, IgniteAsyncS * <p> * In Default Consistency mode, individual puts occur atomically but not * the entire putAll. Listeners may observe individual updates. + * <p> + * Keys are locked in the order in which they appear in map. It is caller's responsibility to + * make sure keys always follow same order, such as by using {@link java.util.TreeMap}. Using unordered map, + * such as {@link java.util.HashMap}, while calling this method in parallel <b>will lead to deadlock</b>. * * @param map Map containing keys and values to put into the cache. * @return a Future representing pending completion of the operation. @@ -1103,6 +1116,11 @@ public interface IgniteCache<K, V> extends javax.cache.Cache<K, V>, IgniteAsyncS /** * {@inheritDoc} + * <p> + * Keys are locked in the order in which they appear in key set. It is caller's responsibility to + * make sure keys always follow same order, such as by using {@link java.util.TreeSet}. Using unordered map, + * such as {@link java.util.HashSet}, while calling this method in parallel <b>will lead to deadlock</b>. + * * @throws TransactionException If operation within transaction is failed. */ @IgniteAsyncSupported @@ -1119,6 +1137,10 @@ public interface IgniteCache<K, V> extends javax.cache.Cache<K, V>, IgniteAsyncS * <li>if the cache is a write-through cache, the {@link CacheWriter}</li> * </ul> * If the key set is empty, the {@link CacheWriter} is not called. + * <p> + * Keys are locked in the order in which they appear in key set. It is caller's responsibility to + * make sure keys always follow same order, such as by using {@link java.util.TreeSet}. Using unordered map, + * such as {@link java.util.HashSet}, while calling this method in parallel <b>will lead to deadlock</b>. * * @param keys Keys set. * @return a Future representing pending completion of the operation. @@ -1360,6 +1382,10 @@ public interface IgniteCache<K, V> extends javax.cache.Cache<K, V>, IgniteAsyncS * <p> * Please refer to documentation for {@link CacheAtomicityMode#ATOMIC} for information on * system behavior in crash scenarios for atomic caches. + * <p> + * Keys are locked in the order in which they appear in key set. It is caller's responsibility to + * make sure keys always follow same order, such as by using {@link java.util.TreeSet}. Using unordered map, + * such as {@link java.util.HashSet}, while calling this method in parallel <b>will lead to deadlock</b>. * * @throws TransactionException If operation within transaction is failed. */ @@ -1389,6 +1415,10 @@ public interface IgniteCache<K, V> extends javax.cache.Cache<K, V>, IgniteAsyncS * <p> * Please refer to documentation for {@link CacheAtomicityMode#ATOMIC} for information on * system behavior in crash scenarios for atomic caches. + * <p> + * Keys are locked in the order in which they appear in key set. It is caller's responsibility to + * make sure keys always follow same order, such as by using {@link java.util.TreeSet}. Using unordered map, + * such as {@link java.util.HashSet}, while calling this method in parallel <b>will lead to deadlock</b>. * * @param keys The set of keys. * @param entryProcessor The {@link EntryProcessor} to invoke. @@ -1422,6 +1452,10 @@ public interface IgniteCache<K, V> extends javax.cache.Cache<K, V>, IgniteAsyncS * An instance of entry processor must be stateless as it may be invoked multiple times on primary and * backup nodes in the cache. It is guaranteed that the value passed to the entry processor will be always * the same. + * <p> + * Keys are locked in the order in which they appear in key set. It is caller's responsibility to + * make sure keys always follow same order, such as by using {@link java.util.TreeSet}. Using unordered map, + * such as {@link java.util.HashSet}, while calling this method in parallel <b>will lead to deadlock</b>. * * @param keys The set of keys for entries to process. * @param entryProcessor The {@link CacheEntryProcessor} to invoke. @@ -1466,6 +1500,10 @@ public interface IgniteCache<K, V> extends javax.cache.Cache<K, V>, IgniteAsyncS * An instance of entry processor must be stateless as it may be invoked multiple times on primary and * backup nodes in the cache. It is guaranteed that the value passed to the entry processor will be always * the same. + * <p> + * Keys are locked in the order in which they appear in key set. It is caller's responsibility to + * make sure keys always follow same order, such as by using {@link java.util.TreeSet}. Using unordered map, + * such as {@link java.util.HashSet}, while calling this method in parallel <b>will lead to deadlock</b>. * * @param keys The set of keys for entries to process. * @param entryProcessor The {@link CacheEntryProcessor} to invoke. diff --git a/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/GridCacheAdapter.java b/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/GridCacheAdapter.java index 00eb65e..265448b 100644 --- a/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/GridCacheAdapter.java +++ b/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/GridCacheAdapter.java @@ -34,6 +34,8 @@ import java.util.List; import java.util.Map; import java.util.NoSuchElementException; import java.util.Set; +import java.util.SortedMap; +import java.util.SortedSet; import java.util.UUID; import java.util.concurrent.Callable; import java.util.concurrent.ExecutorService; @@ -114,6 +116,7 @@ import org.apache.ignite.internal.transactions.IgniteTxHeuristicCheckedException import org.apache.ignite.internal.transactions.IgniteTxRollbackCheckedException; import org.apache.ignite.internal.transactions.IgniteTxTimeoutCheckedException; import org.apache.ignite.internal.transactions.TransactionCheckedException; +import org.apache.ignite.internal.util.GridSerializableMap; import org.apache.ignite.internal.util.future.GridCompoundFuture; import org.apache.ignite.internal.util.future.GridEmbeddedFuture; import org.apache.ignite.internal.util.future.GridFinishedFuture; @@ -134,6 +137,7 @@ import org.apache.ignite.internal.util.typedef.X; import org.apache.ignite.internal.util.typedef.internal.A; import org.apache.ignite.internal.util.typedef.internal.CU; import org.apache.ignite.internal.util.typedef.internal.GPC; +import org.apache.ignite.internal.util.typedef.internal.LT; import org.apache.ignite.internal.util.typedef.internal.S; import org.apache.ignite.internal.util.typedef.internal.U; import org.apache.ignite.lang.IgniteBiPredicate; @@ -1979,6 +1983,8 @@ public abstract class GridCacheAdapter<K, V> implements IgniteInternalCache<K, V if (keyCheck) validateCacheKeys(keys); + warnIfUnordered(keys, BulkOperation.GET); + return getAllAsync0(ctx.cacheKeysView(keys), readerArgs, readThrough, @@ -2726,6 +2732,8 @@ public abstract class GridCacheAdapter<K, V> implements IgniteInternalCache<K, V if (keyCheck) validateCacheKeys(keys); + warnIfUnordered(keys, BulkOperation.INVOKE); + final boolean statsEnabled = ctx.statisticsEnabled(); final long start = statsEnabled ? System.nanoTime() : 0L; @@ -2815,6 +2823,8 @@ public abstract class GridCacheAdapter<K, V> implements IgniteInternalCache<K, V if (keyCheck) validateCacheKeys(keys); + warnIfUnordered(keys, BulkOperation.INVOKE); + final boolean statsEnabled = ctx.statisticsEnabled(); final long start = statsEnabled ? System.nanoTime() : 0L; @@ -2865,6 +2875,8 @@ public abstract class GridCacheAdapter<K, V> implements IgniteInternalCache<K, V if (keyCheck) validateCacheKeys(map.keySet()); + warnIfUnordered(map, BulkOperation.INVOKE); + final boolean statsEnabled = ctx.statisticsEnabled(); final long start = statsEnabled ? System.nanoTime() : 0L; @@ -2910,6 +2922,8 @@ public abstract class GridCacheAdapter<K, V> implements IgniteInternalCache<K, V if (keyCheck) validateCacheKeys(map.keySet()); + warnIfUnordered(map, BulkOperation.INVOKE); + final boolean statsEnabled = ctx.statisticsEnabled(); final long start = statsEnabled ? System.nanoTime() : 0L; @@ -3055,6 +3069,8 @@ public abstract class GridCacheAdapter<K, V> implements IgniteInternalCache<K, V if (keyCheck) validateCacheKeys(m.keySet()); + warnIfUnordered(m, BulkOperation.PUT); + putAll0(m); if (statsEnabled) @@ -3086,6 +3102,8 @@ public abstract class GridCacheAdapter<K, V> implements IgniteInternalCache<K, V if (keyCheck) validateCacheKeys(m.keySet()); + warnIfUnordered(m, BulkOperation.PUT); + return putAllAsync0(m); } @@ -3243,6 +3261,8 @@ public abstract class GridCacheAdapter<K, V> implements IgniteInternalCache<K, V if (keyCheck) validateCacheKeys(keys); + warnIfUnordered(keys, BulkOperation.REMOVE); + removeAll0(keys); if (statsEnabled) @@ -3283,6 +3303,8 @@ public abstract class GridCacheAdapter<K, V> implements IgniteInternalCache<K, V if (keyCheck) validateCacheKeys(keys); + warnIfUnordered(keys, BulkOperation.REMOVE); + IgniteInternalFuture<Object> fut = removeAllAsync0(keys); if (statsEnabled) @@ -5180,6 +5202,85 @@ public abstract class GridCacheAdapter<K, V> implements IgniteInternalCache<K, V } /** + * Checks that given map is sorted or otherwise constant order, or processed inside deadlock-detecting transaction. + * + * Issues developer warning otherwise. + * + * @param m Map to examine. + */ + protected void warnIfUnordered(Map<?, ?> m, BulkOperation op) { + if (m == null || m.size() <= 1) + return; + + if (m instanceof SortedMap || m instanceof GridSerializableMap) + return; + + Transaction tx = ctx.kernalContext().cache().transactions().tx(); + + if (tx != null && !op.canBlockTx(tx.concurrency(), tx.isolation())) + return; + + LT.warn(log, "Unordered map " + m.getClass().getName() + + " is used for " + op.title() + " operation on cache " + name() + ". " + + "This can lead to a distributed deadlock. Switch to a sorted map like TreeMap instead."); + } + + /** + * Checks that given collection is sorted set, or processed inside deadlock-detecting transaction. + * + * Issues developer warning otherwise. + * + * @param coll Collection to examine. + */ + protected void warnIfUnordered(Collection<?> coll, BulkOperation op) { + if (coll == null || coll.size() <= 1) + return; + + if (coll instanceof SortedSet || coll instanceof GridCacheAdapter.KeySet) + return; + + // To avoid false positives, once removeAll() is called, cache will never issue Remove All warnings. + if (ctx.lastRemoveAllJobFut().get() != null && op == BulkOperation.REMOVE) + return; + + Transaction tx = ctx.kernalContext().cache().transactions().tx(); + + if (op == BulkOperation.GET && tx == null) + return; + + if (tx != null && !op.canBlockTx(tx.concurrency(), tx.isolation())) + return; + + LT.warn(log, "Unordered collection " + coll.getClass().getName() + + " is used for " + op.title() + " operation on cache " + name() + ". " + + "This can lead to a distributed deadlock. Switch to a sorted set like TreeSet instead."); + } + + /** */ + protected enum BulkOperation { + GET, + PUT, + INVOKE, + REMOVE; + + /** */ + public String title() { + return name().toLowerCase() + "All"; + } + + /** */ + public boolean canBlockTx(TransactionConcurrency concurrency, TransactionIsolation isolation) { + if (concurrency == OPTIMISTIC && isolation == SERIALIZABLE) + return false; + + if (this == GET && concurrency == PESSIMISTIC && isolation == READ_COMMITTED) + return false; + + return true; + } + } + + /** * @param it Internal entry iterator. * @param deserializeBinary Deserialize binary flag. * @return Public API iterator. diff --git a/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/distributed/dht/atomic/GridDhtAtomicCache.java b/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/distributed/dht/atomic/GridDhtAtomicCache.java index 9f8271a..a416416 100644 --- a/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/distributed/dht/atomic/GridDhtAtomicCache.java +++ b/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/distributed/dht/atomic/GridDhtAtomicCache.java @@ -565,6 +565,8 @@ public class GridDhtAtomicCache<K, V> extends GridDhtCacheAdapter<K, V> { if (keyCheck) validateCacheKeys(keys); + warnIfUnordered(keys, BulkOperation.GET); + CacheOperationContext opCtx = ctx.operationContextPerCall(); subjId = ctx.subjectIdPerCall(subjId, opCtx); @@ -696,6 +698,10 @@ public class GridDhtAtomicCache<K, V> extends GridDhtCacheAdapter<K, V> { /** {@inheritDoc} */ @Override public IgniteInternalFuture<?> putAllConflictAsync(Map<KeyCacheObject, GridCacheDrInfo> conflictMap) { ctx.dr().onReceiveCacheEntriesReceived(conflictMap.size()); + if (map != null && keyCheck) + validateCacheKeys(conflictMap.keySet()); + + warnIfUnordered(conflictMap, BulkOperation.PUT); return updateAll0(null, null, @@ -807,6 +813,11 @@ public class GridDhtAtomicCache<K, V> extends GridDhtCacheAdapter<K, V> { @Override public <T> Map<K, EntryProcessorResult<T>> invokeAll(Set<? extends K> keys, EntryProcessor<K, V, T> entryProcessor, Object... args) throws IgniteCheckedException { + if (map != null && keyCheck) + validateCacheKeys(keys); + + warnIfUnordered(keys, BulkOperation.INVOKE); + return invokeAll0(false, keys, entryProcessor, args).get(); } @@ -885,6 +896,11 @@ public class GridDhtAtomicCache<K, V> extends GridDhtCacheAdapter<K, V> { @Override public <T> IgniteInternalFuture<Map<K, EntryProcessorResult<T>>> invokeAllAsync(Set<? extends K> keys, final EntryProcessor<K, V, T> entryProcessor, Object... args) { + if (map != null && keyCheck) + validateCacheKeys(keys); + + warnIfUnordered(keys, BulkOperation.INVOKE); + return invokeAll0(true, keys, entryProcessor, args); } @@ -902,9 +918,6 @@ public class GridDhtAtomicCache<K, V> extends GridDhtCacheAdapter<K, V> { Object... args) { A.notNull(keys, "keys", entryProcessor, "entryProcessor"); - if (keyCheck) - validateCacheKeys(keys); - final boolean statsEnabled = ctx.statisticsEnabled(); final long start = statsEnabled ? System.nanoTime() : 0L; @@ -953,6 +966,8 @@ public class GridDhtAtomicCache<K, V> extends GridDhtCacheAdapter<K, V> { if (keyCheck) validateCacheKeys(map.keySet()); + warnIfUnordered(map, BulkOperation.INVOKE); + final boolean statsEnabled = ctx.statisticsEnabled(); final long start = statsEnabled ? System.nanoTime() : 0L; @@ -983,6 +998,8 @@ public class GridDhtAtomicCache<K, V> extends GridDhtCacheAdapter<K, V> { if (keyCheck) validateCacheKeys(map.keySet()); + warnIfUnordered(map, BulkOperation.INVOKE); + final boolean statsEnabled = ctx.statisticsEnabled(); final long start = statsEnabled ? System.nanoTime() : 0L; @@ -1030,9 +1047,6 @@ public class GridDhtAtomicCache<K, V> extends GridDhtCacheAdapter<K, V> { ) { assert ctx.updatesAllowed(); - if (map != null && keyCheck) - validateCacheKeys(map.keySet()); - ctx.checkSecurity(SecurityPermission.CACHE_PUT); final CacheOperationContext opCtx = ctx.operationContextPerCall(); diff --git a/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/distributed/dht/colocated/GridDhtColocatedCache.java b/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/distributed/dht/colocated/GridDhtColocatedCache.java index 63a0582..57214da 100644 --- a/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/distributed/dht/colocated/GridDhtColocatedCache.java +++ b/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/distributed/dht/colocated/GridDhtColocatedCache.java @@ -331,6 +331,8 @@ public class GridDhtColocatedCache<K, V> extends GridDhtTransactionalCacheAdapte if (keyCheck) validateCacheKeys(keys); + warnIfUnordered(keys, BulkOperation.GET); + GridNearTxLocal tx = checkCurrentTx(); final CacheOperationContext opCtx = ctx.operationContextPerCall(); diff --git a/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/distributed/near/GridNearAtomicCache.java b/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/distributed/near/GridNearAtomicCache.java index a9b34a4..4212b87 100644 --- a/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/distributed/near/GridNearAtomicCache.java +++ b/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/distributed/near/GridNearAtomicCache.java @@ -420,6 +420,8 @@ public class GridNearAtomicCache<K, V> extends GridNearCacheAdapter<K, V> { if (keyCheck) validateCacheKeys(keys); + warnIfUnordered(keys, BulkOperation.GET); + CacheOperationContext opCtx = ctx.operationContextPerCall(); subjId = ctx.subjectIdPerCall(subjId, opCtx); diff --git a/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/distributed/near/GridNearTransactionalCache.java b/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/distributed/near/GridNearTransactionalCache.java index 151aeb5..a9eec8e 100644 --- a/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/distributed/near/GridNearTransactionalCache.java +++ b/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/distributed/near/GridNearTransactionalCache.java @@ -133,6 +133,8 @@ public class GridNearTransactionalCache<K, V> extends GridNearCacheAdapter<K, V> if (keyCheck) validateCacheKeys(keys); + warnIfUnordered(keys, BulkOperation.GET); + GridNearTxLocal tx = ctx.tm().threadLocalTx(ctx); CacheOperationContext opCtx = ctx.operationContextPerCall(); diff --git a/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/local/atomic/GridLocalAtomicCache.java b/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/local/atomic/GridLocalAtomicCache.java index c62b1df..fb40d0c 100644 --- a/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/local/atomic/GridLocalAtomicCache.java +++ b/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/local/atomic/GridLocalAtomicCache.java @@ -387,6 +387,8 @@ public class GridLocalAtomicCache<K, V> extends GridLocalCache<K, V> { if (keyCheck) validateCacheKeys(keys); + warnIfUnordered(keys, BulkOperation.GET); + final IgniteCacheExpiryPolicy expiry = expiryPolicy(opCtx != null ? opCtx.expiry() : null); boolean success = true; @@ -563,6 +565,8 @@ public class GridLocalAtomicCache<K, V> extends GridLocalCache<K, V> { if (keyCheck) validateCacheKeys(keys); + warnIfUnordered(keys, BulkOperation.INVOKE); + final boolean statsEnabled = ctx.statisticsEnabled(); final long start = statsEnabled ? System.nanoTime() : 0L; @@ -650,6 +654,8 @@ public class GridLocalAtomicCache<K, V> extends GridLocalCache<K, V> { if (keyCheck) validateCacheKeys(keys); + warnIfUnordered(keys, BulkOperation.INVOKE); + final boolean statsEnabled = ctx.statisticsEnabled(); final long start = statsEnabled ? System.nanoTime() : 0L; @@ -682,6 +688,8 @@ public class GridLocalAtomicCache<K, V> extends GridLocalCache<K, V> { if (keyCheck) validateCacheKeys(map.keySet()); + warnIfUnordered(map, BulkOperation.INVOKE); + final boolean statsEnabled = ctx.statisticsEnabled(); final long start = statsEnabled ? System.nanoTime() : 0L; @@ -717,6 +725,8 @@ public class GridLocalAtomicCache<K, V> extends GridLocalCache<K, V> { if (keyCheck) validateCacheKeys(map.keySet()); + warnIfUnordered(map, BulkOperation.INVOKE); + final boolean statsEnabled = ctx.statisticsEnabled(); final long start = statsEnabled ? System.nanoTime() : 0L; @@ -858,9 +868,6 @@ public class GridLocalAtomicCache<K, V> extends GridLocalCache<K, V> { boolean readThrough, boolean keepBinary ) throws IgniteCheckedException { - if (keyCheck) - validateCacheKeys(keys); - if (op == DELETE) ctx.checkSecurity(SecurityPermission.CACHE_REMOVE); else diff --git a/modules/core/src/test/java/org/apache/ignite/internal/GridCacheHashMapPutAllWarningsTest.java b/modules/core/src/test/java/org/apache/ignite/internal/GridCacheHashMapPutAllWarningsTest.java new file mode 100644 index 0000000..1e49d90 --- /dev/null +++ b/modules/core/src/test/java/org/apache/ignite/internal/GridCacheHashMapPutAllWarningsTest.java @@ -0,0 +1,384 @@ +/* + * 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; + +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; +import java.util.HashMap; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.TreeSet; +import javax.cache.processor.EntryProcessor; +import javax.cache.processor.EntryProcessorException; +import javax.cache.processor.EntryProcessorResult; +import javax.cache.processor.MutableEntry; +import org.apache.ignite.Ignite; +import org.apache.ignite.IgniteCache; +import org.apache.ignite.cache.CacheAtomicityMode; +import org.apache.ignite.cache.CacheMode; +import org.apache.ignite.configuration.CacheConfiguration; +import org.apache.ignite.configuration.IgniteConfiguration; +import org.apache.ignite.testframework.ListeningTestLogger; +import org.apache.ignite.testframework.MvccFeatureChecker; +import org.apache.ignite.testframework.junits.common.GridCommonAbstractTest; +import org.apache.ignite.transactions.Transaction; +import org.apache.ignite.transactions.TransactionConcurrency; +import org.apache.ignite.transactions.TransactionIsolation; +import org.junit.Test; + +/** + * Test exchange manager warnings. + */ +public class GridCacheHashMapPutAllWarningsTest extends GridCommonAbstractTest { + /** */ + private ListeningTestLogger testLog; + + /** */ + @Override protected IgniteConfiguration getConfiguration(String instanceName) throws Exception { + IgniteConfiguration cfg = super.getConfiguration(instanceName); + + cfg.setGridLogger(testLog); + + return cfg; + } + + /** */ + @Override protected void afterTest() throws Exception { + super.afterTest(); + + stopAllGrids(); + } + + /** + * @throws Exception If failed. + */ + @Test + public void testHashMapPutAllExactMessage() throws Exception { + List<String> messages = Collections.synchronizedList(new ArrayList<>()); + + testLog = new ListeningTestLogger(false, log()); + + testLog.registerListener((s) -> { + if (s.contains("deadlock")) + messages.add(s); + }); + + Ignite ignite = startGrid(0); + + IgniteCache<Integer, String> c = ignite.getOrCreateCache(new CacheConfiguration<>("exact")); + + HashMap<Integer, String> m = new HashMap<>(); + + m.put(1, "foo"); + m.put(2, "bar"); + + c.putAll(m); + + assertEquals(2, c.size()); + + int found = 0; + + for (String message : messages) { + if (message.contains("Unordered map java.util.HashMap is used for putAll operation on cache exact. " + + "This can lead to a distributed deadlock. Switch to a sorted map like TreeMap instead.")) + found++; + } + + assertEquals(1, found); + } + + /** + * @throws Exception If failed. + */ + @Test + public void testHashMapPutAllExplicitOptimistic() throws Exception { + if (MvccFeatureChecker.forcedMvcc()) + return; + + List<String> messages = Collections.synchronizedList(new ArrayList<>()); + + testLog = new ListeningTestLogger(false, log()); + + testLog.registerListener((s) -> { + if (s.contains("deadlock")) + messages.add(s); + }); + + Ignite ignite = startGrid(0); + + IgniteCache<Integer, String> c = ignite.getOrCreateCache(new CacheConfiguration<Integer, String>("explicitTx") + .setAtomicityMode(CacheAtomicityMode.TRANSACTIONAL)); + + ignite.transactions().txStart(TransactionConcurrency.OPTIMISTIC, TransactionIsolation.SERIALIZABLE); + + HashMap<Integer, String> m = new HashMap<>(); + + m.put(1, "foo"); + m.put(2, "bar"); + + c.putAllAsync(m); + + ignite.transactions().tx().commit(); + + assertEquals(2, c.size()); + + for (String message : messages) { + assertFalse(message.contains("Unordered map")); + assertFalse(message.contains("operation on cache")); + } + } + + /** + * @throws Exception If failed. + */ + @Test + public void testHashMapInvokeAllLocal() throws Exception { + List<String> messages = Collections.synchronizedList(new ArrayList<>()); + + testLog = new ListeningTestLogger(false, log()); + + testLog.registerListener((s) -> { + if (s.contains("deadlock")) + messages.add(s); + }); + + Ignite ignite = startGrid(0); + + IgniteCache<Integer, String> c = ignite.getOrCreateCache(new CacheConfiguration<Integer, String>("invoke") + .setCacheMode(CacheMode.LOCAL)); + + c.put(1, "foo"); + c.put(2, "bar"); + + Map<Integer, EntryProcessorResult<String>> result = c.invokeAll(new HashSet<>(Arrays.asList(1, 2)), + new EntryProcessor<Integer, String, String>() { + @Override public String process(MutableEntry entry, Object... arguments) throws EntryProcessorException { + String newVal = entry.getValue() + "2"; + + entry.setValue(newVal); + + return newVal; + } + }); + + assertEquals(2, result.size()); + assertEquals("bar2", c.get(2)); + + int found = 0; + + for (String message : messages) { + if (message.contains("Unordered collection java.util.HashSet is used for invokeAll operation on cache invoke. ")) + found++; + } + + assertEquals(1, found); + } + + /** + * @throws Exception If failed. + */ + @Test + public void testTreeMapRemoveAll() throws Exception { + List<String> messages = Collections.synchronizedList(new ArrayList<>()); + + testLog = new ListeningTestLogger(false, log()); + + testLog.registerListener((s) -> { + if (s.contains("deadlock")) + messages.add(s); + }); + + Ignite ignite = startGrid(0); + + IgniteCache<Integer, String> c = ignite.getOrCreateCache(new CacheConfiguration<Integer, String>("remove") + .setCacheMode(CacheMode.PARTITIONED)); + + c.put(1, "foo"); + c.put(2, "bar"); + + c.removeAll(new TreeSet<>(Arrays.asList(1, 3))); + + assertEquals(1, c.size()); + + int found = 0; + + for (String message : messages) { + if (message.contains("Unordered collection ")) + found++; + + if (message.contains("operation on cache")) + found++; + } + + assertEquals(0, found); + } + + /** + * @throws Exception If failed. + */ + @Test + public void testTreeMapRemoveAllEntries() throws Exception { + List<String> messages = Collections.synchronizedList(new ArrayList<>()); + + testLog = new ListeningTestLogger(false, log()); + + testLog.registerListener((s) -> { + if (s.contains("deadlock")) + messages.add(s); + }); + + Ignite ignite = startGrid(0); + startGrid(1); + + IgniteCache<Integer, String> c = ignite.getOrCreateCache(new CacheConfiguration<Integer, String>("entries") + .setCacheMode(CacheMode.REPLICATED) + .setAtomicityMode(CacheAtomicityMode.ATOMIC) + .setBackups(1)); + + for (int i = 0; i < 1000; i++) { + c.put(i, "foo"); + c.put(i * 2, "bar"); + } + + c.removeAll(); + + assertEquals(0, c.size()); + + for (String message : messages) { + assertFalse(message.contains("Unordered collection ")); + + assertFalse(message.contains("operation on cache")); + } + } + + /** + * @throws Exception If failed. + */ + @Test + public void testTreeMapClearEntries() throws Exception { + List<String> messages = Collections.synchronizedList(new ArrayList<>()); + + testLog = new ListeningTestLogger(false, log()); + + testLog.registerListener((s) -> { + if (s.contains("deadlock")) + messages.add(s); + }); + + Ignite ignite = startGrid(0); + startGrid(1); + + IgniteCache<Integer, String> c = ignite.getOrCreateCache(new CacheConfiguration<Integer, String>("entries") + .setCacheMode(CacheMode.PARTITIONED) + .setAtomicityMode(CacheAtomicityMode.ATOMIC) + .setBackups(1)); + + for (int i = 0; i < 1000; i++) { + c.put(i, "foo"); + c.put(i * 2, "bar"); + } + + c.clear(); + + assertEquals(0, c.size()); + + for (String message : messages) { + assertFalse(message.contains("Unordered ")); + + assertFalse(message.contains("operation on cache")); + } + } + + /** + * @throws Exception If failed. + */ + @Test + public void testHashSetGetAllReplicated() throws Exception { + List<String> messages = Collections.synchronizedList(new ArrayList<>()); + + testLog = new ListeningTestLogger(false, log()); + + testLog.registerListener((s) -> { + if (s.contains("deadlock")) + messages.add(s); + }); + + Ignite ignite = startGrid(0); + + IgniteCache<Integer, String> c = ignite.getOrCreateCache(new CacheConfiguration<Integer, String>("get") + .setCacheMode(CacheMode.REPLICATED)); + + c.put(1, "foo"); + c.put(2, "bar"); + + assertEquals(1, c.getAll(new HashSet<>(Arrays.asList(1, 3))).size()); + + int found = 0; + + for (String message : messages) { + if (message.contains("Unordered collection ")) + found++; + + if (message.contains("operation on cache")) + found++; + } + + assertEquals(0, found); + } + + /** + * @throws Exception If failed. + */ + @Test + public void testHashSetGetAllTx() throws Exception { + List<String> messages = Collections.synchronizedList(new ArrayList<>()); + + testLog = new ListeningTestLogger(false, log()); + + testLog.registerListener((s) -> { + if (s.contains("deadlock")) + messages.add(s); + }); + + Ignite ignite = startGrid(0); + + IgniteCache<Integer, String> c = ignite.getOrCreateCache(new CacheConfiguration<Integer, String>("getTx") + .setAtomicityMode(CacheAtomicityMode.TRANSACTIONAL) + .setCacheMode(CacheMode.PARTITIONED)); + + c.put(1, "foo"); + c.put(2, "bar"); + + try (Transaction tx = ignite.transactions().txStart(TransactionConcurrency.PESSIMISTIC, TransactionIsolation.REPEATABLE_READ)) { + assertEquals(1, c.getAll(new HashSet<>(Arrays.asList(1, 3))).size()); + + tx.commit(); + } + + int found = 0; + + for (String message : messages) { + if (message.contains("Unordered collection java.util.HashSet is used for getAll operation on cache getTx.")) + found++; + } + + assertEquals(1, found); + } +} diff --git a/modules/core/src/test/java/org/apache/ignite/internal/processors/cache/GridCacheAbstractLocalStoreSelfTest.java b/modules/core/src/test/java/org/apache/ignite/internal/processors/cache/GridCacheAbstractLocalStoreSelfTest.java index 6c050d2..14963e6 100644 --- a/modules/core/src/test/java/org/apache/ignite/internal/processors/cache/GridCacheAbstractLocalStoreSelfTest.java +++ b/modules/core/src/test/java/org/apache/ignite/internal/processors/cache/GridCacheAbstractLocalStoreSelfTest.java @@ -20,10 +20,10 @@ package org.apache.ignite.internal.processors.cache; import com.google.common.collect.Lists; import java.util.Arrays; import java.util.Collection; -import java.util.HashMap; import java.util.Map; import java.util.Random; import java.util.Set; +import java.util.TreeMap; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicInteger; @@ -341,7 +341,7 @@ public abstract class GridCacheAbstractLocalStoreSelfTest extends GridCommonAbst cache.put(key1, key1); cache.put(key2, key2); - Map<Integer, Integer> m = new HashMap<>(); + Map<Integer, Integer> m = new TreeMap<>(); for (int i = KEYS; i < KEYS + 100; i++) m.put(i, i); @@ -453,7 +453,7 @@ public abstract class GridCacheAbstractLocalStoreSelfTest extends GridCommonAbst for (int j = 0; j < 5; j++) cache.get(rn.nextInt(KEYS)); - Map<Integer, Integer> m = new HashMap<>(5); + Map<Integer, Integer> m = new TreeMap<>(); for (int j = 0; j < 5; j++) { Integer key = rn.nextInt(KEYS); @@ -565,7 +565,7 @@ public abstract class GridCacheAbstractLocalStoreSelfTest extends GridCommonAbst assertTrue(kP != kB && kB != kN && kN != kP); - Map<Integer, Integer> m = new HashMap<>(3); + Map<Integer, Integer> m = new TreeMap<>(); m.put(kP, kP); m.put(kB, kB); @@ -595,11 +595,10 @@ public abstract class GridCacheAbstractLocalStoreSelfTest extends GridCommonAbst .withSkipStore().withAllowAtomicOpsInTx(); try (Transaction tx = grid(i).transactions().txStart()) { - Map<Integer, Integer> m = new HashMap<>(3); + Map<Integer, Integer> m = new TreeMap<>(); - for (int j = 0; j < 50; j++) { + for (int j = 0; j < 50; j++) m.put(rn.nextInt(1000), 1000); - } cache.putAll(m); @@ -816,7 +815,7 @@ public abstract class GridCacheAbstractLocalStoreSelfTest extends GridCommonAbst /** {@inheritDoc} */ @Override public Map<K, IgniteBiTuple<V, ?>> loadAll(Iterable<? extends K> keys) throws CacheLoaderException { - Map<K, IgniteBiTuple<V, ?>> res = new HashMap<>(); + Map<K, IgniteBiTuple<V, ?>> res = new TreeMap<>(); for (K key : keys) { IgniteBiTuple<V, ?> val = map.get(key); diff --git a/modules/core/src/test/java/org/apache/ignite/internal/processors/cache/GridCacheAbstractMetricsSelfTest.java b/modules/core/src/test/java/org/apache/ignite/internal/processors/cache/GridCacheAbstractMetricsSelfTest.java index 57ee010..b6808ef 100644 --- a/modules/core/src/test/java/org/apache/ignite/internal/processors/cache/GridCacheAbstractMetricsSelfTest.java +++ b/modules/core/src/test/java/org/apache/ignite/internal/processors/cache/GridCacheAbstractMetricsSelfTest.java @@ -20,7 +20,6 @@ package org.apache.ignite.internal.processors.cache; import com.google.common.collect.ImmutableMap; import java.util.Arrays; import java.util.HashMap; -import java.util.HashSet; import java.util.LinkedHashSet; import java.util.Map; import java.util.Set; @@ -368,7 +367,7 @@ public abstract class GridCacheAbstractMetricsSelfTest extends GridCacheAbstract assertEquals(0.0, cache.localMetrics().getAverageRemoveTime(), 0.0); - Set<Integer> keys = new HashSet<>(4, 1); + Set<Integer> keys = new TreeSet<>(); keys.add(1); keys.add(2); keys.add(3); @@ -1377,7 +1376,7 @@ public abstract class GridCacheAbstractMetricsSelfTest extends GridCacheAbstract public void testInvokeAllMultipleKeysAvgTime() throws IgniteCheckedException { IgniteCache<Integer, Integer> cache = grid(0).cache(DEFAULT_CACHE_NAME); - Set<Integer> keys = new HashSet<>(); + Set<Integer> keys = new TreeSet<>(); keys.add(1); keys.add(2); @@ -1397,7 +1396,7 @@ public abstract class GridCacheAbstractMetricsSelfTest extends GridCacheAbstract public void testInvokeAllAsyncMultipleKeysAvgTime() throws IgniteCheckedException { IgniteCache<Integer, Integer> cache = grid(0).cache(DEFAULT_CACHE_NAME); - Set<Integer> keys = new HashSet<>(); + Set<Integer> keys = new TreeSet<>(); keys.add(1); keys.add(2); diff --git a/modules/core/src/test/java/org/apache/ignite/internal/processors/cache/GridCacheAtomicEntryProcessorDeploymentSelfTest.java b/modules/core/src/test/java/org/apache/ignite/internal/processors/cache/GridCacheAtomicEntryProcessorDeploymentSelfTest.java index b286596..2cef945 100644 --- a/modules/core/src/test/java/org/apache/ignite/internal/processors/cache/GridCacheAtomicEntryProcessorDeploymentSelfTest.java +++ b/modules/core/src/test/java/org/apache/ignite/internal/processors/cache/GridCacheAtomicEntryProcessorDeploymentSelfTest.java @@ -17,8 +17,8 @@ package org.apache.ignite.internal.processors.cache; -import java.util.HashSet; import java.util.Map; +import java.util.TreeSet; import javax.cache.processor.EntryProcessorResult; import org.apache.ignite.IgniteCache; import org.apache.ignite.cache.CacheAtomicityMode; @@ -183,7 +183,7 @@ public class GridCacheAtomicEntryProcessorDeploymentSelfTest extends GridCommonA IgniteCache cache = getCache(); - HashSet keys = new HashSet(); + TreeSet keys = new TreeSet(); for (int i = 0; i < 3; i++) { String key = "key" + i; diff --git a/modules/core/src/test/java/org/apache/ignite/internal/processors/cache/GridCacheInterceptorAbstractSelfTest.java b/modules/core/src/test/java/org/apache/ignite/internal/processors/cache/GridCacheInterceptorAbstractSelfTest.java index 8cfbab3..b7bd7034 100644 --- a/modules/core/src/test/java/org/apache/ignite/internal/processors/cache/GridCacheInterceptorAbstractSelfTest.java +++ b/modules/core/src/test/java/org/apache/ignite/internal/processors/cache/GridCacheInterceptorAbstractSelfTest.java @@ -19,11 +19,11 @@ package org.apache.ignite.internal.processors.cache; import java.util.ArrayList; import java.util.Collection; -import java.util.HashMap; import java.util.LinkedHashSet; import java.util.List; import java.util.Map; import java.util.Set; +import java.util.TreeMap; import java.util.concurrent.atomic.AtomicInteger; import javax.cache.Cache; import javax.cache.processor.EntryProcessor; @@ -941,7 +941,7 @@ public abstract class GridCacheInterceptorAbstractSelfTest extends GridCacheAbst // Interceptor returns incremented new value. interceptor.retInterceptor = new PutIncrementInterceptor(); - Map<String, Integer> map = new HashMap<>(); + Map<String, Integer> map = new TreeMap<>(); final String key1; String key2; @@ -1030,7 +1030,7 @@ public abstract class GridCacheInterceptorAbstractSelfTest extends GridCacheAbst */ @SuppressWarnings("unchecked") private void testBatchRemove(Operation op) throws Exception { - Map<String, Integer> map = new HashMap<>(); + Map<String, Integer> map = new TreeMap<>(); final String key1; String key2; diff --git a/modules/core/src/test/java/org/apache/ignite/internal/processors/cache/IgniteCacheGroupsTest.java b/modules/core/src/test/java/org/apache/ignite/internal/processors/cache/IgniteCacheGroupsTest.java index 316ed48..dff20b3 100644 --- a/modules/core/src/test/java/org/apache/ignite/internal/processors/cache/IgniteCacheGroupsTest.java +++ b/modules/core/src/test/java/org/apache/ignite/internal/processors/cache/IgniteCacheGroupsTest.java @@ -1190,7 +1190,7 @@ public class IgniteCacheGroupsTest extends GridCommonAbstractTest { private Map<Integer, Integer> generateDataMap(int startKey, int cnt) { Random rnd = ThreadLocalRandom.current(); - Map<Integer, Integer> data = U.newHashMap(cnt); + Map<Integer, Integer> data = new TreeMap<>(); for (int i = 0; i < cnt; i++) data.put(startKey++, rnd.nextInt()); diff --git a/modules/core/src/test/java/org/apache/ignite/internal/processors/cache/IgniteClientCacheStartFailoverTest.java b/modules/core/src/test/java/org/apache/ignite/internal/processors/cache/IgniteClientCacheStartFailoverTest.java index 72f2155..0376913 100644 --- a/modules/core/src/test/java/org/apache/ignite/internal/processors/cache/IgniteClientCacheStartFailoverTest.java +++ b/modules/core/src/test/java/org/apache/ignite/internal/processors/cache/IgniteClientCacheStartFailoverTest.java @@ -18,11 +18,11 @@ package org.apache.ignite.internal.processors.cache; import java.util.ArrayList; -import java.util.HashMap; import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Set; +import java.util.TreeMap; import java.util.concurrent.Callable; import java.util.concurrent.CyclicBarrier; import java.util.concurrent.ThreadLocalRandom; @@ -518,7 +518,7 @@ public class IgniteClientCacheStartFailoverTest extends GridCommonAbstractTest { private List<String> startCaches(Ignite node, int keys) { List<String> cacheNames = new ArrayList<>(); - final Map<Integer, Integer> map = new HashMap<>(); + final Map<Integer, Integer> map = new TreeMap<>(); for (int i = 0; i < keys; i++) map.put(i, i); diff --git a/modules/core/src/test/java/org/apache/ignite/internal/processors/cache/distributed/GridCacheTransformEventSelfTest.java b/modules/core/src/test/java/org/apache/ignite/internal/processors/cache/distributed/GridCacheTransformEventSelfTest.java index f018285..6146085 100644 --- a/modules/core/src/test/java/org/apache/ignite/internal/processors/cache/distributed/GridCacheTransformEventSelfTest.java +++ b/modules/core/src/test/java/org/apache/ignite/internal/processors/cache/distributed/GridCacheTransformEventSelfTest.java @@ -20,9 +20,9 @@ package org.apache.ignite.internal.processors.cache.distributed; import java.io.Serializable; import java.util.ArrayList; import java.util.Arrays; -import java.util.HashSet; import java.util.List; import java.util.Set; +import java.util.TreeSet; import java.util.UUID; import javax.cache.processor.EntryProcessor; import javax.cache.processor.MutableEntry; @@ -221,7 +221,7 @@ public class GridCacheTransformEventSelfTest extends GridCommonAbstractTest { key++; } - keys = new HashSet<>(); + keys = new TreeSet<>(); keys.add(key1); keys.add(key2); diff --git a/modules/core/src/test/java/org/apache/ignite/internal/processors/cache/distributed/dht/atomic/AtomicPutAllChangingTopologyTest.java b/modules/core/src/test/java/org/apache/ignite/internal/processors/cache/distributed/dht/atomic/AtomicPutAllChangingTopologyTest.java index d654e47..a062905 100644 --- a/modules/core/src/test/java/org/apache/ignite/internal/processors/cache/distributed/dht/atomic/AtomicPutAllChangingTopologyTest.java +++ b/modules/core/src/test/java/org/apache/ignite/internal/processors/cache/distributed/dht/atomic/AtomicPutAllChangingTopologyTest.java @@ -17,10 +17,10 @@ package org.apache.ignite.internal.processors.cache.distributed.dht.atomic; -import java.util.HashMap; import java.util.LinkedList; import java.util.List; import java.util.Map; +import java.util.TreeMap; import java.util.concurrent.Callable; import java.util.concurrent.CountDownLatch; import org.apache.ignite.Ignite; @@ -124,7 +124,7 @@ public class AtomicPutAllChangingTopologyTest extends GridCommonAbstractTest { log.info("Created cache."); - Map<Integer, Integer> data = new HashMap<>(CACHE_SIZE); + Map<Integer, Integer> data = new TreeMap<>(); for (int i = 0; i < CACHE_SIZE; i++) data.put(i, i); diff --git a/modules/core/src/test/java/org/apache/ignite/internal/processors/cache/distributed/near/GridNearCacheStoreUpdateTest.java b/modules/core/src/test/java/org/apache/ignite/internal/processors/cache/distributed/near/GridNearCacheStoreUpdateTest.java index 51a48c9..0bdb4cb 100644 --- a/modules/core/src/test/java/org/apache/ignite/internal/processors/cache/distributed/near/GridNearCacheStoreUpdateTest.java +++ b/modules/core/src/test/java/org/apache/ignite/internal/processors/cache/distributed/near/GridNearCacheStoreUpdateTest.java @@ -18,9 +18,9 @@ package org.apache.ignite.internal.processors.cache.distributed.near; import java.io.Serializable; -import java.util.HashMap; import java.util.Map; import java.util.Random; +import java.util.TreeMap; import java.util.concurrent.Callable; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.CountDownLatch; @@ -267,8 +267,8 @@ public class GridNearCacheStoreUpdateTest extends GridCommonAbstractTest { * @throws Exception If fail. */ private void checkNearBatch(TransactionConcurrency txConc, TransactionIsolation txIsolation) throws Exception { - final Map<String, String> data1 = new HashMap<>(); - final Map<String, String> data2 = new HashMap<>(); + final Map<String, String> data1 = new TreeMap<>(); + final Map<String, String> data2 = new TreeMap<>(); for (int i = 0; i < 10; i++) { data1.put(String.valueOf(i), String.valueOf(i)); @@ -326,8 +326,8 @@ public class GridNearCacheStoreUpdateTest extends GridCommonAbstractTest { */ private void checkNearBatchConcurrent(TransactionConcurrency txConc, TransactionIsolation txIsolation) throws Exception { - final Map<String, String> data1 = new HashMap<>(); - final Map<String, String> data2 = new HashMap<>(); + final Map<String, String> data1 = new TreeMap<>(); + final Map<String, String> data2 = new TreeMap<>(); for (int j = 0; j < 10; j++) { data1.clear(); diff --git a/modules/core/src/test/java/org/apache/ignite/internal/processors/cache/integration/IgniteCacheStoreSessionAbstractTest.java b/modules/core/src/test/java/org/apache/ignite/internal/processors/cache/integration/IgniteCacheStoreSessionAbstractTest.java index cfc5f75..c240d53 100644 --- a/modules/core/src/test/java/org/apache/ignite/internal/processors/cache/integration/IgniteCacheStoreSessionAbstractTest.java +++ b/modules/core/src/test/java/org/apache/ignite/internal/processors/cache/integration/IgniteCacheStoreSessionAbstractTest.java @@ -20,11 +20,11 @@ package org.apache.ignite.internal.processors.cache.integration; import java.util.ArrayList; import java.util.Collection; import java.util.Collections; -import java.util.HashMap; import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Set; +import java.util.TreeMap; import javax.cache.Cache; import javax.cache.integration.CacheLoaderException; import javax.cache.integration.CacheWriterException; @@ -136,13 +136,13 @@ public abstract class IgniteCacheStoreSessionAbstractTest extends IgniteCacheAbs boolean tx = atomicityMode() == TRANSACTIONAL; - expData.add(new ExpectedData(false, "load", new HashMap<>(), cache.getName())); + expData.add(new ExpectedData(false, "load", new TreeMap<>(), cache.getName())); assertEquals(key, cache.get(key)); assertTrue(expData.isEmpty()); - expData.add(new ExpectedData(false, "loadAll", new HashMap<>(), cache.getName())); + expData.add(new ExpectedData(false, "loadAll", new TreeMap<>(), cache.getName())); assertEquals(3, cache.getAll(keys).size()); @@ -172,7 +172,7 @@ public abstract class IgniteCacheStoreSessionAbstractTest extends IgniteCacheAbs assertTrue(expData.isEmpty()); - Map<Object, Object> vals = new HashMap<>(); + Map<Object, Object> vals = new TreeMap<>(); for (Object key0 : keys) vals.put(key0, key0); @@ -202,7 +202,7 @@ public abstract class IgniteCacheStoreSessionAbstractTest extends IgniteCacheAbs * @param expCacheName Expected cache name. */ private void expectedData(boolean tx, String expMtd, String expCacheName) { - expData.add(new ExpectedData(tx, expMtd, new HashMap<>(), expCacheName)); + expData.add(new ExpectedData(tx, expMtd, new TreeMap<>(), expCacheName)); if (tx) expData.add(new ExpectedData(true, "sessionEnd", F.<Object, Object>asMap(0, expMtd), expCacheName)); @@ -290,7 +290,7 @@ public abstract class IgniteCacheStoreSessionAbstractTest extends IgniteCacheAbs checkSession("loadAll"); - Map<Object, Object> loaded = new HashMap<>(); + Map<Object, Object> loaded = new TreeMap<>(); for (Object key : keys) loaded.put(key, key); diff --git a/modules/core/src/test/java/org/apache/ignite/internal/processors/cache/transactions/TxRollbackAsyncTest.java b/modules/core/src/test/java/org/apache/ignite/internal/processors/cache/transactions/TxRollbackAsyncTest.java index 9b4a319..23213b8 100644 --- a/modules/core/src/test/java/org/apache/ignite/internal/processors/cache/transactions/TxRollbackAsyncTest.java +++ b/modules/core/src/test/java/org/apache/ignite/internal/processors/cache/transactions/TxRollbackAsyncTest.java @@ -18,12 +18,12 @@ package org.apache.ignite.internal.processors.cache.transactions; import java.util.Collection; -import java.util.HashMap; import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Random; import java.util.Set; +import java.util.TreeMap; import java.util.concurrent.ArrayBlockingQueue; import java.util.concurrent.BlockingQueue; import java.util.concurrent.Callable; @@ -498,7 +498,7 @@ public class TxRollbackAsyncTest extends GridCommonAbstractTest { TransactionConcurrency conc) throws Exception { final Ignite client = startClient(); - Map<Integer, Integer> entries = new HashMap<>(); + Map<Integer, Integer> entries = new TreeMap<>(); for (int i = 0; i < 1000000; i++) entries.put(i, i); diff --git a/modules/core/src/test/java/org/apache/ignite/internal/processors/cache/transactions/TxRollbackOnTimeoutTest.java b/modules/core/src/test/java/org/apache/ignite/internal/processors/cache/transactions/TxRollbackOnTimeoutTest.java index 0cc072a..87f5790 100644 --- a/modules/core/src/test/java/org/apache/ignite/internal/processors/cache/transactions/TxRollbackOnTimeoutTest.java +++ b/modules/core/src/test/java/org/apache/ignite/internal/processors/cache/transactions/TxRollbackOnTimeoutTest.java @@ -18,10 +18,10 @@ package org.apache.ignite.internal.processors.cache.transactions; import java.util.Collection; -import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Random; +import java.util.TreeMap; import java.util.concurrent.CountDownLatch; import java.util.concurrent.ThreadLocalRandom; import java.util.concurrent.atomic.AtomicBoolean; @@ -808,7 +808,7 @@ public class TxRollbackOnTimeoutTest extends GridCommonAbstractTest { private void testEnlistMany(boolean write) throws Exception { final Ignite client = startClient(); - Map<Integer, Integer> entries = new HashMap<>(); + Map<Integer, Integer> entries = new TreeMap<>(); for (int i = 0; i < 1000000; i++) entries.put(i, i); diff --git a/modules/core/src/test/java/org/apache/ignite/testframework/junits/common/GridCommonAbstractTest.java b/modules/core/src/test/java/org/apache/ignite/testframework/junits/common/GridCommonAbstractTest.java index 05b4941..593d1c3 100755 --- a/modules/core/src/test/java/org/apache/ignite/testframework/junits/common/GridCommonAbstractTest.java +++ b/modules/core/src/test/java/org/apache/ignite/testframework/junits/common/GridCommonAbstractTest.java @@ -28,6 +28,7 @@ import java.util.Iterator; import java.util.List; import java.util.Map; import java.util.Set; +import java.util.TreeMap; import java.util.UUID; import java.util.concurrent.Callable; import java.util.concurrent.CountDownLatch; @@ -2180,7 +2181,7 @@ public abstract class GridCommonAbstractTest extends GridAbstractTest { int mode = putType != null && putType.length > 0 ? putType[0] : 0; - Map<Integer, Integer> map = keys.stream().collect(Collectors.toMap(k -> k, k -> k)); + Map<Integer, Integer> map = keys.stream().collect(Collectors.toMap(k -> k, k -> k, (a, b) -> a, TreeMap::new)); switch (mode) { case 0: diff --git a/modules/core/src/test/java/org/apache/ignite/testsuites/IgniteCacheTestSuite5.java b/modules/core/src/test/java/org/apache/ignite/testsuites/IgniteCacheTestSuite5.java index b2403b7..2301168 100644 --- a/modules/core/src/test/java/org/apache/ignite/testsuites/IgniteCacheTestSuite5.java +++ b/modules/core/src/test/java/org/apache/ignite/testsuites/IgniteCacheTestSuite5.java @@ -26,6 +26,7 @@ import org.apache.ignite.cache.affinity.AffinityClientNodeSelfTest; import org.apache.ignite.cache.affinity.AffinityDistributionLoggingTest; import org.apache.ignite.cache.affinity.AffinityHistoryCleanupTest; import org.apache.ignite.cache.affinity.local.LocalAffinityFunctionTest; +import org.apache.ignite.internal.GridCacheHashMapPutAllWarningsTest; import org.apache.ignite.internal.GridCachePartitionExchangeManagerHistSizeTest; import org.apache.ignite.internal.GridCachePartitionExchangeManagerWarningsTest; import org.apache.ignite.internal.processors.cache.CacheKeepBinaryTransactionTest; @@ -130,6 +131,8 @@ public class IgniteCacheTestSuite5 { GridTestUtils.addTestIfNeeded(suite, GridCachePartitionExchangeManagerWarningsTest.class, ignoredTests); + GridTestUtils.addTestIfNeeded(suite, GridCacheHashMapPutAllWarningsTest.class, ignoredTests); + GridTestUtils.addTestIfNeeded(suite, NotMappedPartitionInTxTest.class, ignoredTests); GridTestUtils.addTestIfNeeded(suite, ConcurrentCacheStartTest.class, ignoredTests); diff --git a/modules/indexing/src/test/java/org/apache/ignite/internal/processors/cache/BigEntryQueryTest.java b/modules/indexing/src/test/java/org/apache/ignite/internal/processors/cache/BigEntryQueryTest.java index 7cf42b4..52b533a 100644 --- a/modules/indexing/src/test/java/org/apache/ignite/internal/processors/cache/BigEntryQueryTest.java +++ b/modules/indexing/src/test/java/org/apache/ignite/internal/processors/cache/BigEntryQueryTest.java @@ -22,8 +22,10 @@ import java.util.ArrayList; import java.util.Collections; import java.util.HashSet; import java.util.List; +import java.util.Map; import java.util.Random; import java.util.Set; +import java.util.TreeMap; import java.util.function.Function; import java.util.stream.Collectors; import java.util.stream.LongStream; @@ -79,9 +81,10 @@ public class BigEntryQueryTest extends GridCommonAbstractTest { .setWriteSynchronizationMode(CacheWriteSynchronizationMode.FULL_SYNC) .setIndexedTypes(Long.class, Value.class)); - cache.putAll(LongStream.range(610026643276160000L, 610026643276170000L).boxed() + cache.putAll((Map) LongStream.range(610026643276160000L, 610026643276170000L).boxed() .collect(Collectors.toMap(Function.identity(), - t -> Value.of(new byte[(random.nextInt(16)) * 1000])))); + t -> Value.of(new byte[(random.nextInt(16)) * 1000]), + (a, b) -> a, TreeMap::new))); for (int i = 0; i < 10; i++) { long start = 610026643276160000L; diff --git a/modules/platforms/cpp/core/include/ignite/cache/cache.h b/modules/platforms/cpp/core/include/ignite/cache/cache.h index e18654d..c0dd4a9 100644 --- a/modules/platforms/cpp/core/include/ignite/cache/cache.h +++ b/modules/platforms/cpp/core/include/ignite/cache/cache.h @@ -469,6 +469,9 @@ namespace ignite * * This method should only be used on the valid instance. * + * Keys are locked in the order of iteration. It is caller's responsibility to make sure keys always follow + * same order. If that is not observed, calling this method in parallel <b>will lead to deadlock</b>. + * * @param begin Iterator pointing to the beggining of the key-value pair sequence. * @param end Iterator pointing to the end of the key-value pair sequence. */ @@ -1214,6 +1217,9 @@ namespace ignite * * This method should only be used on the valid instance. * + * Keys are locked in the order of iteration. It is caller's responsibility to make sure keys always follow + * same order. If that is not observed, calling this method in parallel <b>will lead to deadlock</b>. + * * @param begin Iterator pointing to the beggining of the key sequence. * @param end Iterator pointing to the end of the key sequence. */ diff --git a/modules/platforms/dotnet/Apache.Ignite.Core/Cache/ICache.cs b/modules/platforms/dotnet/Apache.Ignite.Core/Cache/ICache.cs index a25fb97..49715f7 100644 --- a/modules/platforms/dotnet/Apache.Ignite.Core/Cache/ICache.cs +++ b/modules/platforms/dotnet/Apache.Ignite.Core/Cache/ICache.cs @@ -527,6 +527,10 @@ namespace Apache.Ignite.Core.Cache /// Stores given key-value pairs in cache. /// If write-through is enabled, the stored values will be persisted to store. /// This method is transactional and will enlist the entry into ongoing transaction if there is one. + /// + /// Keys are locked in the order in which they are enumerated. It is caller's responsibility to + /// make sure keys always follow same order, such as by using <see cref="SortedDictionary{K, V}"/>. Using unordered + /// dictionary, such as <see cref="Dictionary{K, V}"/>, while calling this method in parallel <b>will lead to deadlock</b>. /// </summary> /// <param name="vals">Key-value pairs to store in cache.</param> void PutAll(IEnumerable<KeyValuePair<TK, TV>> vals); @@ -535,6 +539,10 @@ namespace Apache.Ignite.Core.Cache /// Stores given key-value pairs in cache. /// If write-through is enabled, the stored values will be persisted to store. /// This method is transactional and will enlist the entry into ongoing transaction if there is one. + /// + /// Keys are locked in the order in which they are enumerated. It is caller's responsibility to + /// make sure keys always follow same order, such as by using <see cref="SortedDictionary{K, V}"/>. Using unordered + /// dictionary, such as <see cref="Dictionary{K, V}"/>, while calling this method in parallel <b>will lead to deadlock</b>. /// </summary> /// <param name="vals">Key-value pairs to store in cache.</param> Task PutAllAsync(IEnumerable<KeyValuePair<TK, TV>> vals); @@ -656,6 +664,10 @@ namespace Apache.Ignite.Core.Cache /// Removes given key mappings from cache. /// If write-through is enabled, the value will be removed from store. /// This method is transactional and will enlist the entry into ongoing transaction if there is one. + /// + /// Keys are locked in the order in which they are enumerated. It is caller's responsibility to + /// make sure keys always follow same order, such as by using <see cref="SortedSet{K}"/>. Using unordered + /// collection, such as <see cref="HashSet{K}"/>, while calling this method in parallel <b>will lead to deadlock</b>. /// </summary> /// <param name="keys">Keys whose mappings are to be removed from cache.</param> void RemoveAll(IEnumerable<TK> keys); @@ -664,6 +676,10 @@ namespace Apache.Ignite.Core.Cache /// Removes given key mappings from cache. /// If write-through is enabled, the value will be removed from store. /// This method is transactional and will enlist the entry into ongoing transaction if there is one. + /// + /// Keys are locked in the order in which they are enumerated. It is caller's responsibility to + /// make sure keys always follow same order, such as by using <see cref="SortedSet{K}"/>. Using unordered + /// collection, such as <see cref="HashSet{K}"/>, while calling this method in parallel <b>will lead to deadlock</b>. /// </summary> /// <param name="keys">Keys whose mappings are to be removed from cache.</param> Task RemoveAllAsync(IEnumerable<TK> keys); @@ -848,6 +864,10 @@ namespace Apache.Ignite.Core.Cache /// Implementations may choose to process the entries in any order, including concurrently. /// Furthermore there is no guarantee implementations will use the same processor instance /// to process each entry, as the case may be in a non-local cache topology. + /// + /// Keys are locked in the order in which they are enumerated. It is caller's responsibility to + /// make sure keys always follow same order, such as by using <see cref="SortedSet{K}"/>. Using unordered + /// collection, such as <see cref="HashSet{K}"/>, while calling this method in parallel <b>will lead to deadlock</b>. /// </summary> /// <typeparam name="TArg">The type of the argument.</typeparam> /// <typeparam name="TRes">The type of the result.</typeparam> @@ -872,6 +892,10 @@ namespace Apache.Ignite.Core.Cache /// Implementations may choose to process the entries in any order, including concurrently. /// Furthermore there is no guarantee implementations will use the same processor instance /// to process each entry, as the case may be in a non-local cache topology. + /// + /// Keys are locked in the order in which they are enumerated. It is caller's responsibility to + /// make sure keys always follow same order, such as by using <see cref="SortedSet{K}"/>. Using unordered + /// collection, such as <see cref="HashSet{K}"/>, while calling this method in parallel <b>will lead to deadlock</b>. /// </summary> /// <typeparam name="TArg">The type of the argument.</typeparam> /// <typeparam name="TRes">The type of the result.</typeparam>