[GitHub] [ignite] agura commented on a change in pull request #6976: IGNITE-6804 Warning if unordered map used in locking cache operation.

[GitBox <gi...@apache.org>]

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:
users@infra.apache.org


With regards,
Apache Git Services