IGNITE-27871 Improve deployment lookup to reduce deploy() contention … by oleg-vlsk · Pull Request #12760 · apache/ignite

oleg-vlsk · 2026-02-17T23:16:05Z

…for locally available tasks with peerClassLoadingEnabled=true

Thank you for submitting the pull request to the Apache Ignite.

In order to streamline the review of the contribution
we ask you to ensure the following steps have been taken:

The Contribution Checklist

There is a single JIRA ticket related to the pull request.
The web-link to the pull request is attached to the JIRA ticket.
The JIRA ticket has the Patch Available state.
The pull request body describes changes that have been made.
The description explains WHAT and WHY was made instead of HOW.
The pull request title is treated as the final commit message.
The following pattern must be used: IGNITE-XXXX Change summary where XXXX - number of JIRA issue.
A reviewer has been mentioned through the JIRA comments
(see the Maintainers list)
The pull request has been checked by the Teamcity Bot and
the green visa attached to the JIRA ticket (see TC.Bot: Check PR)

Notes

If you need any help, please email dev@ignite.apache.org or ask anу advice on http://asf.slack.com #ignite channel.

…for locally available tasks with peerClassLoadingEnabled=true

alex-plekhanov · 2026-02-18T07:39:04Z

modules/core/src/test/java/org/apache/ignite/testsuites/IgniteP2PSelfTestSuite.java

    P2PClassLoadingFailureHandlingTest.class,
-    P2PClassLoadingIssuesTest.class
+    P2PClassLoadingIssuesTest.class,
+    GridDeploymentLocalStoreReuseTest.class


Add comma to the end of line please (to reduce conflicts on merge)

alex-plekhanov · 2026-02-18T12:47:24Z

...t/java/org/apache/ignite/internal/managers/deployment/GridDeploymentLocalStoreReuseTest.java

+            CompletableFuture<T2<UUID, Set<UUID>>> fut = client.compute()
+                .withTimeout(timeout).
+                    <T2<UUID, Set<UUID>>, T2<UUID, Set<UUID>>>executeAsync2(TestTask.class.getName(), null)
+                .toCompletableFuture();
+
+            try {
+                fut.get();


client.compute().execute(TestTask.class.getName(), null);

Used this snippet, thank you.

alex-plekhanov · 2026-02-18T12:52:18Z

...t/java/org/apache/ignite/internal/managers/deployment/GridDeploymentLocalStoreReuseTest.java

+            List<IgniteInternalFuture<Void>> futs = new ArrayList<>(CLIENT_CNT);
+
+            for (IgniteClient client : clients)
+                futs.add(runAsync(() -> executeTasksOnClient(client, EXEC_CNT, 5_000L)));
+
+            waitForAllFutures(futs.toArray(new IgniteInternalFuture[0]));


runMultiThreaded(i -> executeTasksOnClient(clients.get(i), EXEC_CNT), CLIENT_CNT, "worker");

I decided not to go for multi-threaded execution as the perpose of the test is to verify certain behaviour during subsequent executions of the same task. So I ended up using a simple for-loop.

alex-plekhanov · 2026-02-18T12:54:07Z

...t/java/org/apache/ignite/internal/managers/deployment/GridDeploymentLocalStoreReuseTest.java

+            ClusterNode[] allServerNodes = grid(0).cluster().forServers().nodes().toArray(new ClusterNode[0]);
+
+            for (int i = 0; i < CLIENT_CNT; i++)
+                clients.add(startClient(allServerNodes));


You can connect to any server node,it's not necessary to provide all nodes, one is enough, i.e. clients.add(startClient(0));

alex-plekhanov · 2026-02-18T13:49:00Z

...t/java/org/apache/ignite/internal/managers/deployment/GridDeploymentLocalStoreReuseTest.java

+    /** */
+    private static class DeploymentListeningLogger extends ListeningTestLogger {
+        /** */
+        private final ConcurrentLinkedQueue<String> depNotFound = new ConcurrentLinkedQueue<>();
+
+        /** */
+        public DeploymentListeningLogger(IgniteLogger log) {
+            super(log);
+        }
+
+        /** {@inheritDoc} */
+        @Override public void debug(String msg) {
+            if (msg.contains("Deployment was not found for class with specific class loader"))
+                depNotFound.add(msg);
+
+            super.debug(msg);
+        }
+
+        /** {@inheritDoc} */
+        @Override public ListeningTestLogger getLogger(Object ctgr) {
+            return this;
+        }
+
+        /** */
+        public List<String> depNotFound() {
+            return depNotFound.stream().collect(Collectors.toUnmodifiableList());
+        }
+    }
+}


It's incorrect usage of listening logger, all you need is register listener like:

LogListener lsnr = LogListener.matches(notFoundMsg).times(CLIENT_CNT).build(); listeningTestLog.registerListener(lsnr);

listeningTestLog should be created on top of standard logger, for example:

setLoggerDebugLevel(); listeningTestLog = new ListeningTestLogger(log);

And passed to ignite configuration. No need for logger for each node.

alex-plekhanov · 2026-02-19T08:08:37Z

...core/src/main/java/org/apache/ignite/internal/managers/deployment/GridDeploymentManager.java

        meta.alias(rsrcName);
        meta.className(clsName);
        meta.senderNodeId(ctx.localNodeId());
+        meta.classLoader(ldr);


Setting classloader disables deployment SPI as far as I understand. See https://github.com/apache/ignite/blob/master/modules/core/src/main/java/org/apache/ignite/internal/managers/deployment/GridDeploymentLocalStore.java#L174

Moved the local app classloader check to GridDeploymentLocalStore#deployment so that in the initial call the meta does not contains classloader.

alex-plekhanov · 2026-02-19T08:35:16Z

...e/src/main/java/org/apache/ignite/internal/managers/deployment/GridDeploymentLocalStore.java

    private final ConcurrentMap<String, Deque<GridDeployment>> cache = new ConcurrentHashMap<>();

+    /** Deployment cache by classloader. */
+    private final ConcurrentMap<ClassLoader, Deque<GridDeployment>> cacheByLdr = new ConcurrentHashMap<>();


cacheByLdr always used under the lock mux, no ConcurrentMap overhead required here.
Also maybe it worth to use IdentityHashMap in case someone redefine classloader's equals() in a wrong way.

Done, thank you for the hint.

alex-plekhanov · 2026-02-19T08:57:45Z

...e/src/main/java/org/apache/ignite/internal/managers/deployment/GridDeploymentLocalStore.java


-                            dep = d;
+                    for (GridDeployment d : depsByLdr) {
+                        if (!d.undeployed() && d.classLoader() == ldr) {


If it's undeployed, it's cleaned from cache, how we can find it?
Why do we need to check classloader if we put in cache only items with exactly this classloader?

Yes, those were 'extra safety' checks. Changed the lookup logic altogether (see below).

alex-plekhanov · 2026-02-19T09:00:27Z

...e/src/main/java/org/apache/ignite/internal/managers/deployment/GridDeploymentLocalStore.java

+                        dep = candidate;
+                    }
+                }
+                else {


Do we still need this check? If deployment not found by classloader in classloader cache it can't be found in aliases cache. We preserve both caches synchronized and modify it only under the lock.

Removed this else block.

alex-plekhanov · 2026-02-19T09:01:57Z

...e/src/main/java/org/apache/ignite/internal/managers/deployment/GridDeploymentLocalStore.java

-                        if (d.classLoader() == ldr) {
-                            // Cache class and alias.
-                            fireEvt = d.addDeployedClass(cls, alias);
+                Deque<GridDeployment> depsByLdr = cacheByLdr.get(ldr);


Looks like it's one-to-one relation for deployment and classloader. Did I miss something?

Not necessarily. In GridDeploymentLocalStore#cache we can have several deployments with the same classloader associated with one alias/class name (see attached screenshots). Most recent deployment are added to the beginning of the queue (the addFirst() call in GridDeploymentLocalStore#deploy).

I'm talking about cacheByLdr, not cache. For cacheByLdr it looks like only one deployment is possible for one classloader.

…calStore#deployment, correct cache lookup mechanism in GridDeploymentLocalStore#deploy, simplify GridDeploymentLocalStoreReuseTest#testNoExcessiveLocalDeploymentCacheMisses

alex-plekhanov · 2026-02-22T07:24:27Z

...e/src/main/java/org/apache/ignite/internal/managers/deployment/GridDeploymentLocalStore.java

+                    ClassLoader ldr = Thread.currentThread().getContextClassLoader();
+
+                    if (ldr == null)
+                        ldr = U.resolveClassLoader(ctx.config());


Let's move ldr initialization outside the loop.

Just add || dep.classLoader() == ldr to the if condition

alex-plekhanov · 2026-02-22T07:34:34Z

...e/src/main/java/org/apache/ignite/internal/managers/deployment/GridDeploymentLocalStore.java

-                        if (d.classLoader() == ldr) {
-                            // Cache class and alias.
-                            fireEvt = d.addDeployedClass(cls, alias);
+                Deque<GridDeployment> depsByLdr = cacheByLdr.get(ldr);


I'm talking about cacheByLdr, not cache. For cacheByLdr it looks like only one deployment is possible for one classloader.

alex-plekhanov · 2026-02-22T07:36:46Z

...t/java/org/apache/ignite/internal/managers/deployment/GridDeploymentLocalStoreReuseTest.java

+        assertTrue(lsnr0.check(5_000));
+        assertTrue(lsnr1.check(5_000));


Why do we need to wait here? As far as I understand here strict happens-before between task completion and log message.

IGNITE-27871 Improve deployment lookup to reduce deploy() contention …

20e13b4

…for locally available tasks with peerClassLoadingEnabled=true

alex-plekhanov reviewed Feb 19, 2026

View reviewed changes

Valuyskiy.O.Y added 2 commits February 22, 2026 07:20

IGNITE-27871 Move local classloader checkup logic to GridDeploymentLo…

112ddd1

…calStore#deployment, correct cache lookup mechanism in GridDeploymentLocalStore#deploy, simplify GridDeploymentLocalStoreReuseTest#testNoExcessiveLocalDeploymentCacheMisses

IGNITE-27871 Remove GridDeploymentLocalStore#depByLdr

2c7cb8f

alex-plekhanov reviewed Feb 22, 2026

View reviewed changes

		assertTrue(lsnr0.check(5_000));
		assertTrue(lsnr1.check(5_000));

Comments

Conversation

oleg-vlsk commented Feb 17, 2026

The Contribution Checklist

Notes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

oleg-vlsk Feb 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

oleg-vlsk Feb 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

oleg-vlsk Feb 21, 2026 •

edited

Loading

oleg-vlsk Feb 21, 2026 •

edited

Loading