anton-vinogradov commented on code in PR #13440:
URL: https://github.com/apache/ignite/pull/13440#discussion_r3745409755
##########
modules/core/src/main/java/org/apache/ignite/internal/managers/deployment/GridDeploymentManager.java:
##########
@@ -402,6 +404,76 @@ private GridDeployment checkDeployment(GridDeployment
deployment, String store)
return locStore.getDeployment(meta);
}
+ /**
+ * Resolves the class loader the classes of a message must be read with.
Blocks when the deployment has to be
+ * requested from its owner, so it must not be called from a
socket-reading thread.
+ *
+ * @param msg Message carrying its own deployment.
+ * @return Class loader of the carried deployment, or the local one if the
message carries none.
+ * @throws IgniteDeploymentCheckedException If the deployment cannot be
obtained.
+ */
+ public ClassLoader classLoader(DeploymentAware msg) throws
IgniteDeploymentCheckedException {
+ return classLoader(msg.deploymentInfo(), msg.deployedClassName());
+ }
+
+ /**
+ * Resolves the class loader classes described by {@code depInfo} must be
read with. Blocks when the deployment has
+ * to be requested from its owner, so it must not be called from a
socket-reading thread.
+ *
+ * @param depInfo Deployment of the classes, or {@code null} when they
carry none.
+ * @param clsName Name of a class the deployment must be able to load.
+ * @return Class loader of the deployment, or the local one when there is
no deployment.
+ * @throws IgniteDeploymentCheckedException If the deployment cannot be
obtained.
+ */
+ public ClassLoader classLoader(@Nullable GridDeploymentInfo depInfo,
String clsName)
+ throws IgniteDeploymentCheckedException {
+ if (depInfo == null)
+ return U.resolveClassLoader(ctx.config());
+
+ return U.resolveClassLoader(globalDeployment(depInfo,
clsName).classLoader(), ctx.config());
+ }
+
+ /**
+ * Resolves the deployment {@code depInfo} describes, for the classes of
{@code clsName}. The sender of those
+ * classes is the node that created the class loader, or a participant
when the deployment has any.
+ *
+ * @param depInfo Deployment of the classes, as it came with the message
carrying them.
+ * @param clsName Name of a class the deployment must be able to load.
+ * @return The deployment the classes are loaded with.
+ * @throws IgniteDeploymentCheckedException If the deployment is gone or
peer class loading is off.
+ */
+ public GridDeployment globalDeployment(GridDeploymentInfo depInfo, String
clsName)
+ throws IgniteDeploymentCheckedException {
+ GridDeployment dep = globalDeployment(depInfo, clsName, clsName);
+
+ if (dep == null) {
+ throw new IgniteDeploymentCheckedException("Failed to obtain
deployment for class (is peer class " +
+ "loading turned on?): " + clsName);
+ }
+
+ return dep;
+ }
+
+ /**
+ * Resolves the deployment {@code depInfo} describes, as {@link
#globalDeployment(GridDeploymentInfo, String)}
+ * does, but under {@code rsrcName} (a task may be deployed under a name
of its own) and returns {@code null}
+ * instead of throwing, for callers that have somewhere else to look.
+ *
+ * @param depInfo Deployment of the classes, as it came with the message
carrying them.
+ * @param rsrcName Name the classes are deployed under.
+ * @param clsName Name of a class the deployment must be able to load.
+ * @return The deployment, or {@code null} when there is none.
+ */
+ @Nullable public GridDeployment globalDeployment(GridDeploymentInfo
depInfo, String rsrcName, String clsName) {
+ return getGlobalDeployment(depInfo.deployMode(),
+ rsrcName,
+ clsName,
+ depInfo.userVersion(),
+ depInfo.classLoaderId().globalId(),
Review Comment:
The two are the same node by construction, so this was not a shortcut - but
you are right that nothing at the call site said so.
A class loader id carries the id of the node that created it, and
`GridDeploymentClassLoader` asserts it in both places where such a loader is
built:
```java
assert nodeId.equals(clsLdrId.globalId()); // :184, constructor
assert nodeId.equals(ldrId.globalId()); // :324, register(nodeId,
ldrId), for a participant
```
So a message describing a deployment cannot name a sender that disagrees
with the loader it describes: the old code, which passed the sender separately,
blew up on that assert if the two ever diverged.
I have made it readable instead of implied: `GridDeploymentInfo#nodeId()`
now returns it, with the invariant written in its javadoc, and the call site
reads `depInfo.nodeId()`.
##########
modules/core/src/main/java/org/apache/ignite/internal/processors/cache/query/continuous/CacheContinuousQueryDeployableObject.java:
##########
@@ -88,11 +88,7 @@ protected CacheContinuousQueryDeployableObject(Object obj,
GridKernalContext ctx
<T> T unmarshal(UUID nodeId, GridKernalContext ctx) throws
IgniteCheckedException {
assert ctx != null;
- GridDeployment dep =
ctx.deploy().getGlobalDeployment(depInfo.deployMode(), clsName, clsName,
- depInfo.userVersion(), nodeId, depInfo.classLoaderId(),
depInfo.participants(), null);
-
- if (dep == null)
- throw new IgniteDeploymentCheckedException("Failed to obtain
deployment for class: " + clsName);
+ GridDeployment dep = ctx.deploy().globalDeployment(depInfo, clsName);
Review Comment:
No longer required - and it turned out to be true well past this method.
The deployment is now resolved from the descriptor the message carries, so
the node id has nothing to do here. Following it upwards, it had nothing to do
anywhere else either: it went through `GridContinuousHandler#p2pUnmarshal`, its
three implementations and the private helper in `CacheContinuousQueryHandler`,
and every one of them only fed it to an `assert nodeId != null`.
Deleted along the whole chain, so the signature is
`p2pUnmarshal(GridKernalContext)` now.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
