Skip to content

Commit 4984f18

Browse files
Valuyskiy.O.Yalex-plekhanov
authored andcommitted
IGNITE-27871 Fix local classes deployment lookup contention when peerClassLoadingEnabled=true - Fixes #12760.
Signed-off-by: Aleksey Plekhanov <plehanov.alex@gmail.com>
1 parent 231fa68 commit 4984f18

4 files changed

Lines changed: 217 additions & 103 deletions

File tree

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

Lines changed: 98 additions & 98 deletions
Original file line numberDiff line numberDiff line change
@@ -17,19 +17,13 @@
1717

1818
package org.apache.ignite.internal.managers.deployment;
1919

20-
import java.util.ArrayList;
2120
import java.util.Collection;
22-
import java.util.Deque;
2321
import java.util.HashMap;
24-
import java.util.HashSet;
22+
import java.util.IdentityHashMap;
2523
import java.util.Iterator;
2624
import java.util.Map;
27-
import java.util.Map.Entry;
2825
import java.util.Set;
2926
import java.util.UUID;
30-
import java.util.concurrent.ConcurrentHashMap;
31-
import java.util.concurrent.ConcurrentLinkedDeque;
32-
import java.util.concurrent.ConcurrentMap;
3327
import org.apache.ignite.IgniteCheckedException;
3428
import org.apache.ignite.compute.ComputeTask;
3529
import org.apache.ignite.compute.ComputeTaskName;
@@ -39,7 +33,6 @@
3933
import org.apache.ignite.internal.IgniteDeploymentCheckedException;
4034
import org.apache.ignite.internal.util.GridAnnotationsCache;
4135
import org.apache.ignite.internal.util.GridClassLoaderCache;
42-
import org.apache.ignite.internal.util.typedef.F;
4336
import org.apache.ignite.internal.util.typedef.internal.S;
4437
import org.apache.ignite.internal.util.typedef.internal.U;
4538
import org.apache.ignite.lang.IgniteUuid;
@@ -60,8 +53,11 @@
6053
* Storage for local deployments.
6154
*/
6255
class GridDeploymentLocalStore extends GridDeploymentStoreAdapter {
63-
/** Deployment cache by class name. */
64-
private final ConcurrentMap<String, Deque<GridDeployment>> cache = new ConcurrentHashMap<>();
56+
/** Primary index: deployment by class loader. Not thread-safe, access must be guarded by {@link #mux}. */
57+
private final Map<ClassLoader, GridDeployment> depByLdr = new IdentityHashMap<>();
58+
59+
/** Secondary index: deployments by alias or class name. Not thread-safe, access must be guarded by {@link #mux}. */
60+
private final Map<String, Map<ClassLoader, GridDeployment>> depsByAlias = new HashMap<>();
6561

6662
/** Mutex. */
6763
private final Object mux = new Object();
@@ -87,19 +83,14 @@ class GridDeploymentLocalStore extends GridDeploymentStoreAdapter {
8783
@Override public void stop() {
8884
spi.setListener(null);
8985

90-
Map<String, Collection<GridDeployment>> cp;
86+
Set<ClassLoader> ldrs = U.newIdentityHashSet();
9187

9288
synchronized (mux) {
93-
cp = new HashMap<>(cache);
94-
95-
for (Entry<String, Collection<GridDeployment>> entry : cp.entrySet())
96-
entry.setValue(new ArrayList<>(entry.getValue()));
89+
ldrs.addAll(depByLdr.keySet());
9790
}
9891

99-
for (Collection<GridDeployment> deps : cp.values()) {
100-
for (GridDeployment cls : deps)
101-
undeploy(cls.classLoader());
102-
}
92+
for (ClassLoader ldr : ldrs)
93+
undeploy(ldr);
10394

10495
if (log.isDebugEnabled())
10596
log.debug(stopInfo());
@@ -110,11 +101,10 @@ class GridDeploymentLocalStore extends GridDeploymentStoreAdapter {
110101
Set<ClassLoader> obsoleteClsLdrs = U.newIdentityHashSet();
111102

112103
synchronized (mux) {
113-
// There can be obsolete class loaders in cache after client node reconnect with the new node id.
114-
for (Entry<String, Deque<GridDeployment>> entry : cache.entrySet())
115-
for (GridDeployment dep : entry.getValue())
116-
if (!dep.classLoaderId().globalId().equals(ctx.localNodeId()))
117-
obsoleteClsLdrs.add(dep.classLoader());
104+
// There can be obsolete class loaders in deployment indexes after client node reconnect with the new node id.
105+
for (GridDeployment dep : depByLdr.values())
106+
if (!dep.classLoaderId().globalId().equals(ctx.localNodeId()))
107+
obsoleteClsLdrs.add(dep.classLoader());
118108
}
119109

120110
for (ClassLoader clsLdr : obsoleteClsLdrs)
@@ -123,13 +113,10 @@ class GridDeploymentLocalStore extends GridDeploymentStoreAdapter {
123113

124114
/** {@inheritDoc} */
125115
@Override public Collection<GridDeployment> getDeployments() {
126-
Collection<GridDeployment> deps = new ArrayList<>();
116+
Collection<GridDeployment> deps = U.newIdentityHashSet();
127117

128118
synchronized (mux) {
129-
for (Deque<GridDeployment> depList : cache.values())
130-
for (GridDeployment d : depList)
131-
if (!deps.contains(d))
132-
deps.add(d);
119+
deps.addAll(depByLdr.values());
133120
}
134121

135122
return deps;
@@ -138,10 +125,9 @@ class GridDeploymentLocalStore extends GridDeploymentStoreAdapter {
138125
/** {@inheritDoc} */
139126
@Nullable @Override public GridDeployment getDeployment(IgniteUuid ldrId) {
140127
synchronized (mux) {
141-
for (Deque<GridDeployment> deps : cache.values())
142-
for (GridDeployment dep : deps)
143-
if (dep.classLoaderId().equals(ldrId))
144-
return dep;
128+
for (GridDeployment dep : depByLdr.values())
129+
if (dep.classLoaderId().equals(ldrId))
130+
return dep;
145131
}
146132

147133
for (GridDeployment dep : ctx.task().getUsedDeployments())
@@ -248,22 +234,55 @@ class GridDeploymentLocalStore extends GridDeploymentStoreAdapter {
248234
* @return Deployment.
249235
*/
250236
@Nullable private GridDeployment deployment(final GridDeploymentMetadata meta) {
251-
Deque<GridDeployment> deps = cache.get(meta.alias());
237+
Map<ClassLoader, GridDeployment> deps;
238+
239+
synchronized (mux) {
240+
Map<ClassLoader, GridDeployment> cached = depsByAlias.get(meta.alias());
241+
242+
deps = cached == null ? null : new IdentityHashMap<>(cached);
243+
}
252244

253245
if (deps != null) {
254-
for (GridDeployment dep : deps) {
255-
if (dep.undeployed())
256-
continue;
246+
GridDeployment dep = null;
257247

258-
// local or remote deployment.
259-
if (dep.classLoaderId() == meta.classLoaderId() || dep.classLoader() == meta.classLoader()) {
260-
if (log.isTraceEnabled())
261-
log.trace("Deployment was found for class with specific class loader [alias=" + meta.alias() +
262-
", clsLdrId=" + meta.classLoaderId() + "]");
248+
if (meta.classLoader() != null)
249+
dep = deps.get(meta.classLoader());
263250

264-
return dep;
251+
if ((dep == null || dep.undeployed()) && meta.classLoaderId() != null) {
252+
for (GridDeployment d : deps.values()) {
253+
if (d.classLoaderId().equals(meta.classLoaderId()) && !d.undeployed()) {
254+
dep = d;
255+
256+
break;
257+
}
265258
}
266259
}
260+
261+
if (dep != null && !dep.undeployed()) {
262+
if (log.isTraceEnabled())
263+
log.trace("Deployment was found for class with specific class loader [alias=" + meta.alias() +
264+
", clsLdrId=" + meta.classLoaderId() + "]");
265+
266+
return dep;
267+
}
268+
269+
ClassLoader appLdr = Thread.currentThread().getContextClassLoader();
270+
271+
if (appLdr == null)
272+
appLdr = U.resolveClassLoader(ctx.config());
273+
274+
appLdr = (appLdr instanceof GridDeploymentClassLoader) ? null : appLdr;
275+
276+
if (appLdr != null)
277+
dep = deps.get(appLdr);
278+
279+
if (dep != null && !dep.undeployed()) {
280+
if (log.isTraceEnabled())
281+
log.trace("Deployment was found for class with the local app class loader [alias="
282+
+ meta.alias() + "]");
283+
284+
return dep;
285+
}
267286
}
268287

269288
if (log.isDebugEnabled())
@@ -288,42 +307,21 @@ private GridDeployment deploy(
288307
String alias,
289308
boolean recordEvt
290309
) {
291-
GridDeployment dep = null;
310+
GridDeployment dep;
292311

293312
synchronized (mux) {
294313
boolean fireEvt = false;
295314

296315
try {
297-
Deque<GridDeployment> cachedDeps = null;
298-
299-
// Find existing class loader info.
300-
for (Deque<GridDeployment> deps : cache.values()) {
301-
for (GridDeployment d : deps) {
302-
if (d.classLoader() == ldr) {
303-
// Cache class and alias.
304-
fireEvt = d.addDeployedClass(cls, alias);
305-
306-
cachedDeps = deps;
307-
308-
dep = d;
309-
310-
break;
311-
}
312-
}
313-
314-
if (cachedDeps != null)
315-
break;
316-
}
316+
dep = depByLdr.get(ldr);
317317

318-
if (cachedDeps != null) {
319-
assert dep != null;
318+
if (dep != null) {
319+
fireEvt = dep.addDeployedClass(cls, alias);
320320

321-
cache.put(alias, cachedDeps);
321+
addAliasMapping(alias, ldr, dep);
322322

323-
if (!cls.getName().equals(alias)) {
324-
// Cache by class name as well.
325-
cache.put(cls.getName(), cachedDeps);
326-
}
323+
if (!cls.getName().equals(alias))
324+
addAliasMapping(cls.getName(), ldr, dep);
327325

328326
return dep;
329327
}
@@ -339,19 +337,12 @@ private GridDeployment deploy(
339337
assert fireEvt : "Class was not added to newly created deployment [cls=" + cls +
340338
", depMode=" + depMode + ", dep=" + dep + ']';
341339

342-
Deque<GridDeployment> deps = F.<String, Deque<GridDeployment>>addIfAbsent(
343-
cache,
344-
alias,
345-
ConcurrentLinkedDeque::new
346-
);
340+
depByLdr.put(ldr, dep);
347341

348-
// Add at the beginning of the list for future fast access.
349-
deps.addFirst(dep);
342+
addAliasMapping(alias, ldr, dep);
350343

351-
if (!cls.getName().equals(alias)) {
352-
// Cache by class name as well.
353-
cache.put(cls.getName(), deps);
354-
}
344+
if (!cls.getName().equals(alias))
345+
addAliasMapping(cls.getName(), ldr, dep);
355346

356347
if (log.isDebugEnabled())
357348
log.debug("Created new deployment: " + dep);
@@ -543,33 +534,29 @@ private String getAlias(GridDeployment dep, Class<?> cls) {
543534
* @param ldr Class loader to undeploy.
544535
*/
545536
private void undeploy(ClassLoader ldr) {
546-
Collection<GridDeployment> doomed = new HashSet<>();
537+
GridDeployment dep;
547538

548539
synchronized (mux) {
549-
for (Iterator<Deque<GridDeployment>> i1 = cache.values().iterator(); i1.hasNext();) {
550-
Deque<GridDeployment> deps = i1.next();
540+
dep = depByLdr.remove(ldr);
551541

552-
for (Iterator<GridDeployment> i2 = deps.iterator(); i2.hasNext();) {
553-
GridDeployment dep = i2.next();
542+
if (dep != null) {
543+
dep.undeploy();
554544

555-
if (dep.classLoader() == ldr) {
556-
dep.undeploy();
545+
if (log.isInfoEnabled())
546+
log.info("Removed undeployed class: " + dep);
557547

558-
i2.remove();
548+
for (Iterator<Map<ClassLoader, GridDeployment>> it = depsByAlias.values().iterator(); it.hasNext();) {
549+
Map<ClassLoader, GridDeployment> deps = it.next();
559550

560-
doomed.add(dep);
551+
deps.remove(ldr);
561552

562-
if (log.isInfoEnabled())
563-
log.info("Removed undeployed class: " + dep);
564-
}
553+
if (deps.isEmpty())
554+
it.remove();
565555
}
566-
567-
if (deps.isEmpty())
568-
i1.remove();
569556
}
570557
}
571558

572-
for (GridDeployment dep : doomed) {
559+
if (dep != null) {
573560
if (dep.obsolete()) {
574561
// Resource cleanup.
575562
ctx.resource().onUndeployed(dep);
@@ -588,6 +575,19 @@ private void undeploy(ClassLoader ldr) {
588575
}
589576
}
590577

578+
/**
579+
* Adds deployment to the alias-based index. Must be called under {@link #mux}.
580+
*
581+
* @param key Alias or classname.
582+
* @param ldr Class loader.
583+
* @param dep Deployment.
584+
*/
585+
private void addAliasMapping(String key, ClassLoader ldr, GridDeployment dep) {
586+
Map<ClassLoader, GridDeployment> deps = depsByAlias.computeIfAbsent(key, k -> new IdentityHashMap<>());
587+
588+
deps.put(ldr, dep);
589+
}
590+
591591
/** {@inheritDoc} */
592592
@Override public String toString() {
593593
return S.toString(GridDeploymentLocalStore.class, this);

0 commit comments

Comments
 (0)