agura commented on a change in pull request #6976: IGNITE-6804 Warning if 
unordered map used in locking cache operation.
URL: https://github.com/apache/ignite/pull/6976#discussion_r374240883
 
 

 ##########
 File path: 
modules/core/src/main/java/org/apache/ignite/internal/processors/cache/GridCacheAdapter.java
 ##########
 @@ -5176,6 +5198,91 @@ protected final void validateCacheKeys(Iterable<?> 
keys) {
         }
     }
 
+    /**
+     * 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 checkKeysOrdered(Map m, BulkOperation op) {
+        if (m == null || m.size() <= 1)
+            return;
+
+        if (m instanceof SortedMap || m instanceof GridSerializableMap)
+            return;
+
+        if (curTxDeadlockDetecting(op))
+            return;
+
+        LT.warn(log, "Unordered map " + m.getClass().getSimpleName() +
+            " 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 checkKeysOrdered(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;
+
+        if (curTxDeadlockDetecting(op))
+            return;
+
+        LT.warn(log, "Unordered collection " + coll.getClass().getSimpleName() 
+
+            " is used for " + op.title() + " operation on cache " + name() + 
". " +
+            "This can lead to a distributed deadlock. Switch to a sorted set 
like TreeSet instead.");
+    }
+
+    /** */
+    private boolean curTxDeadlockDetecting(BulkOperation op) {
+        Transaction tx = ctx.kernalContext().cache().transactions().tx();
+
+        if (tx != null && !tx.implicit() &&
+            ((ctx.tm().deadlockDetectionEnabled() && tx.timeout() > 0L) ||
+            (tx.concurrency() == OPTIMISTIC && tx.isolation() == SERIALIZABLE) 
||
 
 Review comment:
   Deadlock is impossible for `tx.concurrency() == OPTIMISTIC && tx.isolation() 
== SERIALIZABLE`. 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
[email protected]


With regards,
Apache Git Services

Reply via email to