You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@ignite.apache.org by "anton-vinogradov (via GitHub)" <gi...@apache.org> on 2026/08/06 00:30:16 UTC
[PR] IGNITE-28528 Revise marshalling of the node filter in StartRequestData [ignite]
anton-vinogradov opened a new pull request, #13440:
URL: https://github.com/apache/ignite/pull/13440
The node filter in `StartRequestData` was marshalled by hand, and the receiver
had to build the deployment class loader itself before reading the filter.
### Why the code looked like that
A message that carries peer-deployed classes said nothing about it. It held the
deployment as four or five flat fields, so every receiver rebuilt the deployment
from those fields and passed the resulting loader into the unmarshal call. Eight
places did exactly that. `StartRequestData` also repeated a "peer class loading
is off" branch, although `GridDeploymentManager` already covers that case: with
peer class loading off it looks the deployment up locally.
### Change
* `DeploymentAware` lets a message state that its classes come with a deployment,
the same way `CacheIdAware` states the need for a cache object context.
* `GridDeploymentManager.classLoader(DeploymentAware)` is now the single place
that turns a carried deployment into a class loader.
* The generated marshaller asks for that loader when the caller passes none, so
receivers no longer resolve it by hand.
* Messages carry `GridDeploymentInfoBean` as a whole instead of the flat fields.
This changes the wire format.
`DeploymentAware` and `DeferredUnmarshalMessage` answer different questions and
stay independent: the first says which class loader reads the message, the
second says who starts the read. This matters for messages whose owner reports a
missing deployment back to the sender, and for messages read outside the
receiving path.
A message read by its owner also keeps its serialized bytes, because it can be
sent on: a discovery message travels the whole ring, and re-marshalling a user
object on that path is what those bytes are there to avoid.
### Verified
`GridEventStorageCheckAllEventsSelfTest`, `GridEventStorageManagerSelfTest`,
`GridEventConsumeSelfTest`, `GridMessageListenSelfTest`,
`GridP2PContinuousDeploymentSelfTest`, `P2PClassLoadingFailureHandlingTest`,
`IgniteNoCustomEventsOnNodeStart`.
๐ค Generated with [Claude Code](https://claude.com/claude-code)
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-28528 Revise marshalling of the node filter in StartRequestData [ignite]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #13440:
URL: https://github.com/apache/ignite/pull/13440#issuecomment-5199005039
<!-- ignite-rolling-upgrade-check -->
## Possible compatibility issues. Please, check rolling upgrade cases
This PR modifies protected classes (with **Order** annotation).
Changes to these classes can break rolling upgrade compatibility.
**Affected files:**
- `modules/codegen/src/main/java/org/apache/ignite/internal/MessageProcessor.java`
- `modules/core/src/main/java/org/apache/ignite/internal/managers/communication/GridIoUserMessage.java`
- `modules/core/src/main/java/org/apache/ignite/internal/managers/eventstorage/GridEventStorageRequest.java`
- `modules/core/src/main/java/org/apache/ignite/internal/processors/continuous/StartRequestData.java`
- `modules/core/src/main/java/org/apache/ignite/internal/processors/datastreamer/DataStreamerRequest.java`
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-28528 Revise marshalling of the node filter in StartRequestData [ignite]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #13440:
URL: https://github.com/apache/ignite/pull/13440#issuecomment-5199532647
<!-- ignite-rolling-upgrade-check -->
## Possible compatibility issues. Please, check rolling upgrade cases
This PR modifies protected classes (with **Order** annotation).
Changes to these classes can break rolling upgrade compatibility.
**Affected files:**
- `modules/codegen/src/main/java/org/apache/ignite/internal/MessageProcessor.java`
- `modules/core/src/main/java/org/apache/ignite/internal/managers/communication/GridIoUserMessage.java`
- `modules/core/src/main/java/org/apache/ignite/internal/managers/eventstorage/GridEventStorageRequest.java`
- `modules/core/src/main/java/org/apache/ignite/internal/processors/continuous/StartRequestData.java`
- `modules/core/src/main/java/org/apache/ignite/internal/processors/datastreamer/DataStreamerRequest.java`
- `modules/core/src/test/java/org/apache/ignite/internal/codegen/MessageProcessorTest.java`
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-28528 Revise marshalling of the node filter in StartRequestData [ignite]
Posted by "anton-vinogradov (via GitHub)" <gi...@apache.org>.
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-28528 Revise marshalling of the node filter in StartRequestData [ignite]
Posted by "anton-vinogradov (via GitHub)" <gi...@apache.org>.
anton-vinogradov commented on PR #13440:
URL: https://github.com/apache/ignite/pull/13440#issuecomment-5235085614
**[Ignite PR Checker](https://ignite-pr-checker.is-a.dev/?pr=13440)** verdict ยท RunAll build [9269815](https://ci2.ignite.apache.org/build/9269815) ยท 147 suites ran, 0 reused
โ ๏ธ **This run doesn't cover the PR fully:**
- 1 suite(s) have no reliable result (compilation error, timeout, crash)
- 1 suite(s) ran far fewer tests than the same suites on master
Everything below is what it did manage to say.
โ ๏ธ **1 broken suite(s)** (no reliable run):
- Platform C++ CMake (Linux): IgniteOdbcTest terminated abnormally - the test binary died, its remaining tests never ran ยท Number of tests 724 is 31% less than 1047 in build #42829 โ ran 724 of master's 1047 tests
๐ **1 suite(s) ran fewer tests than on master** (tests that never ran can't fail):
- Platform C++ CMake (Linux Clang): 728 tests vs 1047 on master (**โ30%**)
๐ **No blockers found โ but the run above can't prove the PR is clean.** 20 pre-existing/flaky tests filtered out. Re-run once the above is sorted out.
โณ _Auto re-run **#1** in progress โ 1 broken suite(s) re-queued (attempt 1/2), **โ settled by 05:25 MSK**. This comment updates when they settle._
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-28528 Revise marshalling of the node filter in StartRequestData [ignite]
Posted by "anton-vinogradov (via GitHub)" <gi...@apache.org>.
anton-vinogradov commented on PR #13440:
URL: https://github.com/apache/ignite/pull/13440#issuecomment-5260878699
@anton-vinogradov ๐ **RunAll queued** โ [build 9275828](https://ci2.ignite.apache.org/build/9275828) ยท live progress & verdict: [Ignite PR Checker](https://ignite-pr-checker.is-a.dev/?pr=13440). The verdict lands here when the run finishes.
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-28528 Revise marshalling of the node filter in StartRequestData [ignite]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #13440:
URL: https://github.com/apache/ignite/pull/13440#issuecomment-5226685128
<!-- ignite-rolling-upgrade-check -->
## Possible compatibility issues. Please, check rolling upgrade cases
This PR modifies protected classes (with **Order** annotation).
Changes to these classes can break rolling upgrade compatibility.
**Affected files:**
- `modules/codegen/src/main/java/org/apache/ignite/internal/MessageProcessor.java`
- `modules/core/src/main/java/org/apache/ignite/internal/managers/communication/GridIoUserMessage.java`
- `modules/core/src/main/java/org/apache/ignite/internal/managers/eventstorage/GridEventStorageRequest.java`
- `modules/core/src/main/java/org/apache/ignite/internal/processors/continuous/StartRequestData.java`
- `modules/core/src/main/java/org/apache/ignite/internal/processors/continuous/StartRoutineDiscoveryMessage.java`
- `modules/core/src/main/java/org/apache/ignite/internal/processors/datastreamer/DataStreamerRequest.java`
- `modules/core/src/test/java/org/apache/ignite/internal/codegen/MessageProcessorTest.java`
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-28528 Revise marshalling of the node filter in StartRequestData [ignite]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #13440:
URL: https://github.com/apache/ignite/pull/13440#issuecomment-5234156557
<!-- ignite-rolling-upgrade-check -->
## Possible compatibility issues. Please, check rolling upgrade cases
This PR modifies protected classes (with **Order** annotation).
Changes to these classes can break rolling upgrade compatibility.
**Affected files:**
- `modules/core/src/main/java/org/apache/ignite/internal/managers/communication/GridIoUserMessage.java`
- `modules/core/src/main/java/org/apache/ignite/internal/managers/deployment/GridDeploymentInfoBean.java`
- `modules/core/src/main/java/org/apache/ignite/internal/managers/deployment/GridDeploymentInfoMessage.java`
- `modules/core/src/main/java/org/apache/ignite/internal/managers/eventstorage/GridEventStorageRequest.java`
- `modules/core/src/main/java/org/apache/ignite/internal/processors/cache/GridCacheMessage.java`
- `modules/core/src/main/java/org/apache/ignite/internal/processors/continuous/StartRequestData.java`
- `modules/core/src/main/java/org/apache/ignite/internal/processors/datastreamer/DataStreamerRequest.java`
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-28528 Revise marshalling of the node filter in StartRequestData [ignite]
Posted by "anton-vinogradov (via GitHub)" <gi...@apache.org>.
anton-vinogradov commented on PR #13440:
URL: https://github.com/apache/ignite/pull/13440#issuecomment-5229157476
**[Ignite PR Checker](https://ignite-pr-checker.is-a.dev/?pr=13440)** verdict ยท RunAll build [9266790](https://ci2.ignite.apache.org/build/9266790) ยท 147 suites ran, 0 reused
โ **23 blocker(s) in 3 suite(s):**
- Continuous Query 3: `org.apache.ignite.testsuites.IgniteCacheQuerySelfTestSuite5: org.apache.ignite.internal.processors.cache.query.continuous.ContinuousQueryRemoteFilterMissingInClassPathSelfTest.testServerMissingClassFailsRegistration`
- Continuous Query 3: `org.apache.ignite.testsuites.IgniteCacheQuerySelfTestSuite5: org.apache.ignite.internal.processors.cache.query.continuous.ContinuousQueryRemoteFilterMissingInClassPathSelfTest.testNodeFilterServerMissingClassDoesNotFailRegistration`
- ZooKeeper (Discovery) 3: `org.apache.ignite.spi.discovery.zk.ZookeeperDiscoverySpiTestSuite3: org.apache.ignite.internal.processors.continuous.GridEventConsumeSelfTest.testNodeJoinWithProjection`
- ZooKeeper (Discovery) 3: `org.apache.ignite.spi.discovery.zk.ZookeeperDiscoverySpiTestSuite3: org.apache.ignite.internal.processors.continuous.GridEventConsumeSelfTest.testNodeJoin`
- ZooKeeper (Discovery) 3: `org.apache.ignite.spi.discovery.zk.ZookeeperDiscoverySpiTestSuite3: org.apache.ignite.internal.processors.cache.query.continuous.CacheContinuousQueryOperationP2PTest.testTxClient`
- ZooKeeper (Discovery) 3: `org.apache.ignite.spi.discovery.zk.ZookeeperDiscoverySpiTestSuite3: org.apache.ignite.internal.processors.cache.query.continuous.CacheContinuousQueryOperationP2PTest.testTxReplicated`
- ZooKeeper (Discovery) 3: `org.apache.ignite.spi.discovery.zk.ZookeeperDiscoverySpiTestSuite3: org.apache.ignite.internal.processors.cache.query.continuous.CacheContinuousQueryOperationP2PTest.testTx`
- ZooKeeper (Discovery) 3: `org.apache.ignite.spi.discovery.zk.ZookeeperDiscoverySpiTestSuite3: org.apache.ignite.internal.processors.cache.query.continuous.CacheContinuousQueryOperationP2PTest.testMultithreadedUpdatesNodeJoin`
- ZooKeeper (Discovery) 3: `org.apache.ignite.spi.discovery.zk.ZookeeperDiscoverySpiTestSuite3: org.apache.ignite.internal.processors.cache.query.continuous.CacheContinuousQueryOperationP2PTest.testAtomicClient`
- ZooKeeper (Discovery) 3: `org.apache.ignite.spi.discovery.zk.ZookeeperDiscoverySpiTestSuite3: org.apache.ignite.internal.processors.cache.query.continuous.CacheContinuousQueryOperationP2PTest.testTxReplicatedClient`
โฆ and 13 more
โณ _Auto re-run **#1** in progress โ 3 blocker suite(s) re-queued (attempt 1/2), **โ settled by 05:03 MSK**. This comment updates when they settle._
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-28528 Revise marshalling of the node filter in StartRequestData [ignite]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #13440:
URL: https://github.com/apache/ignite/pull/13440#issuecomment-5234028821
<!-- ignite-rolling-upgrade-check -->
## Possible compatibility issues. Please, check rolling upgrade cases
This PR modifies protected classes (with **Order** annotation).
Changes to these classes can break rolling upgrade compatibility.
**Affected files:**
- `modules/codegen/src/main/java/org/apache/ignite/internal/MessageProcessor.java`
- `modules/core/src/main/java/org/apache/ignite/internal/managers/communication/GridIoUserMessage.java`
- `modules/core/src/main/java/org/apache/ignite/internal/managers/deployment/GridDeploymentInfoBean.java`
- `modules/core/src/main/java/org/apache/ignite/internal/managers/deployment/GridDeploymentInfoMessage.java`
- `modules/core/src/main/java/org/apache/ignite/internal/managers/eventstorage/GridEventStorageRequest.java`
- `modules/core/src/main/java/org/apache/ignite/internal/processors/cache/GridCacheMessage.java`
- `modules/core/src/main/java/org/apache/ignite/internal/processors/continuous/StartRequestData.java`
- `modules/core/src/main/java/org/apache/ignite/internal/processors/datastreamer/DataStreamerRequest.java`
- `modules/core/src/test/java/org/apache/ignite/internal/codegen/MessageProcessorTest.java`
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-28528 Revise marshalling of the node filter in StartRequestData [ignite]
Posted by "anton-vinogradov (via GitHub)" <gi...@apache.org>.
anton-vinogradov commented on PR #13440:
URL: https://github.com/apache/ignite/pull/13440#issuecomment-5234247732
Good catch, and it was a symptom of the same thing your inline question uncovered - that generated code cannot pick the class loader for a message.
Overwriting the parameter was deliberate but wrong. It came from `DeploymentAware`: a message declared that it carries its own deployment, and the generated marshaller resolved the loader from it. I made the assignment unconditional because the convenience overloads pass a loader rather than `null`:
```java
default void unmarshal(M msg, GridKernalContext kctx) {
unmarshal(msg, kctx, null, U.resolveClassLoader(kctx.config()));
}
```
so a caller using them would silently read peer-deployed classes with the local loader. Ignoring the argument fixed that symptom and hid a worse problem: the resolution needs the sending node, which the generated code has no way to know, and the sender is not always the owner of the class loader.
So the whole thing is gone now. `DeploymentAware`, the resolution in the generator and its test are removed - `modules/codegen` is no longer part of this PR at all - and the resolution takes the node as a parameter, passed by the callers that know it.
What is left is what the ticket asked for: the deployment travels as one descriptor instead of four flat fields, and the eight places that used to assemble it by hand now make a single call. The diff went from 35 files to 25, and from +577/-497 to +248/-475.
Since the change is materially different from what the last RunAll saw, a fresh run is needed.
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-28528 Revise marshalling of the node filter in StartRequestData [ignite]
Posted by "anton-vinogradov (via GitHub)" <gi...@apache.org>.
anton-vinogradov commented on PR #13440:
URL: https://github.com/apache/ignite/pull/13440#issuecomment-5260875221
/runall
---
๐จ _Checkstyle autofix: fixed 2 of 15 violation(s) in 2 file(s) โ commit 7454525e. 13 remain (javadoc/naming/wrapping need a human) โ the Checkstyle suite may still fail._
๐ **RunAll queued** โ [build 9266790](https://ci2.ignite.apache.org/build/9266790) ยท live progress & verdict: [Ignite PR Checker](https://ignite-pr-checker.is-a.dev/?pr=13440). The verdict lands here when the run finishes.
๐ _Run finished โ analysing; the verdict comment follows._
โ ๏ธ _GitHub rejected your personal access token, so this is narrated from the checker's own account. Save a fresh PAT in the checker's settings to get your own back._
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-28528 Revise marshalling of the node filter in StartRequestData [ignite]
Posted by "anton-vinogradov (via GitHub)" <gi...@apache.org>.
anton-vinogradov commented on code in PR #13440:
URL: https://github.com/apache/ignite/pull/13440#discussion_r3745492413
##########
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(),
Review Comment:
It cannot be null today, but only by an invariant that is nowhere stated - you are right not to trust it.
On the sending side both fields come from the same deployment, and the send fails outright when there is none:
```java
dep = ctx.deploy().deploy(cls0, U.detectClassLoader(cls0));
if (dep == null)
throw new IgniteDeploymentCheckedException(...);
depClsName = cls0.getName();
```
So `deploymentClassName() != null` implies `deploymentInfo() != null`, and the guard on the receiving side happens to check the first while the code dereferences the second.
Rather than document that, I made it structural: the resolution now accepts a null deployment and returns null for it, so nothing to resolve gives nothing back, and the caller reports it with the message it already has. The throwing overload turns the same case into its usual `IgniteDeploymentCheckedException`.
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-28528 Revise marshalling of the node filter in StartRequestData [ignite]
Posted by "anton-vinogradov (via GitHub)" <gi...@apache.org>.
anton-vinogradov commented on PR #13440:
URL: https://github.com/apache/ignite/pull/13440#issuecomment-5228554950
/runall
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-28528 Revise marshalling of the node filter in StartRequestData [ignite]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #13440:
URL: https://github.com/apache/ignite/pull/13440#issuecomment-5228312025
<!-- ignite-rolling-upgrade-check -->
## Possible compatibility issues. Please, check rolling upgrade cases
This PR modifies protected classes (with **Order** annotation).
Changes to these classes can break rolling upgrade compatibility.
**Affected files:**
- `modules/codegen/src/main/java/org/apache/ignite/internal/MessageProcessor.java`
- `modules/core/src/main/java/org/apache/ignite/internal/managers/communication/GridIoUserMessage.java`
- `modules/core/src/main/java/org/apache/ignite/internal/managers/eventstorage/GridEventStorageRequest.java`
- `modules/core/src/main/java/org/apache/ignite/internal/processors/continuous/StartRequestData.java`
- `modules/core/src/main/java/org/apache/ignite/internal/processors/continuous/StartRoutineDiscoveryMessage.java`
- `modules/core/src/main/java/org/apache/ignite/internal/processors/datastreamer/DataStreamerRequest.java`
- `modules/core/src/test/java/org/apache/ignite/internal/codegen/MessageProcessorTest.java`
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-28528 Revise marshalling of the node filter in StartRequestData [ignite]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #13440:
URL: https://github.com/apache/ignite/pull/13440#issuecomment-5234124933
<!-- ignite-rolling-upgrade-check -->
## Possible compatibility issues. Please, check rolling upgrade cases
This PR modifies protected classes (with **Order** annotation).
Changes to these classes can break rolling upgrade compatibility.
**Affected files:**
- `modules/core/src/main/java/org/apache/ignite/internal/managers/communication/GridIoUserMessage.java`
- `modules/core/src/main/java/org/apache/ignite/internal/managers/deployment/GridDeploymentInfoBean.java`
- `modules/core/src/main/java/org/apache/ignite/internal/managers/deployment/GridDeploymentInfoMessage.java`
- `modules/core/src/main/java/org/apache/ignite/internal/managers/eventstorage/GridEventStorageRequest.java`
- `modules/core/src/main/java/org/apache/ignite/internal/processors/cache/GridCacheMessage.java`
- `modules/core/src/main/java/org/apache/ignite/internal/processors/continuous/StartRequestData.java`
- `modules/core/src/main/java/org/apache/ignite/internal/processors/datastreamer/DataStreamerRequest.java`
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-28528 Revise marshalling of the node filter in StartRequestData [ignite]
Posted by "petrov-mg (via GitHub)" <gi...@apache.org>.
petrov-mg commented on code in PR #13440:
URL: https://github.com/apache/ignite/pull/13440#discussion_r3745237499
##########
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:
It's confusing that `depInfo.classLoaderId().globalId()` is used as `sndNodeId`. Why is that?
##########
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:
Was the `nodeId` method parameter lost during refactoring, or is it no longer required and should be deleted?
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-28528 Revise marshalling of the node filter in StartRequestData [ignite]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #13440:
URL: https://github.com/apache/ignite/pull/13440#issuecomment-5228450212
<!-- ignite-rolling-upgrade-check -->
## Possible compatibility issues. Please, check rolling upgrade cases
This PR modifies protected classes (with **Order** annotation).
Changes to these classes can break rolling upgrade compatibility.
**Affected files:**
- `modules/codegen/src/main/java/org/apache/ignite/internal/MessageProcessor.java`
- `modules/core/src/main/java/org/apache/ignite/internal/managers/communication/GridIoUserMessage.java`
- `modules/core/src/main/java/org/apache/ignite/internal/managers/deployment/GridDeploymentInfoBean.java`
- `modules/core/src/main/java/org/apache/ignite/internal/managers/deployment/GridDeploymentInfoMessage.java`
- `modules/core/src/main/java/org/apache/ignite/internal/managers/eventstorage/GridEventStorageRequest.java`
- `modules/core/src/main/java/org/apache/ignite/internal/processors/cache/GridCacheMessage.java`
- `modules/core/src/main/java/org/apache/ignite/internal/processors/continuous/StartRequestData.java`
- `modules/core/src/main/java/org/apache/ignite/internal/processors/continuous/StartRoutineDiscoveryMessage.java`
- `modules/core/src/main/java/org/apache/ignite/internal/processors/datastreamer/DataStreamerRequest.java`
- `modules/core/src/test/java/org/apache/ignite/internal/codegen/MessageProcessorTest.java`
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-28528 Revise marshalling of the node filter in StartRequestData [ignite]
Posted by "anton-vinogradov (via GitHub)" <gi...@apache.org>.
anton-vinogradov commented on PR #13440:
URL: https://github.com/apache/ignite/pull/13440#issuecomment-5230131194
**[Ignite PR Checker](https://ignite-pr-checker.is-a.dev/?pr=13440)** verdict ยท RunAll build [9267428](https://ci2.ignite.apache.org/build/9267428) ยท 147 suites ran, 0 reused
๐ **6 test(s) started failing on this code** โ not proven blockers yet: too few runs of this revision to tell a break from a flake, so a re-run of the suite decides it.
- PDS 10: `org.apache.ignite.testsuites.IgnitePdsTestSuite10: org.apache.ignite.internal.processors.cache.distributed.dht.preloader.HistoricalRebalanceCheckpointTest.testDelayedToBackupsRequests1BackupMorePuts`
- Disk Page Compressions 3: `org.apache.ignite.testsuites.IgnitePdsCompressionTestSuite3: org.apache.ignite.internal.processors.localtask.DurableBackgroundTasksProcessorSelfTest.testDontDeleteTaskIfItsRestart`
- Cache 19: `org.apache.ignite.testsuites.IgniteCacheTestSuite17: org.apache.ignite.internal.processors.cache.transactions.TxPartitionCounterStateOnePrimaryTwoBackupsHistoryRebalanceTest.testPartialCommit_2TX_4`
- Client Nodes: `org.apache.ignite.testsuites.IgniteClientReconnectTestSuite: org.apache.ignite.internal.IgniteClientReconnectAtomicsWithLostPartitionsTest.testAtomicSequenceAddAngGet`
- Client Nodes: `org.apache.ignite.testsuites.IgniteClientReconnectTestSuite: org.apache.ignite.internal.IgniteClientReconnectAtomicsWithLostPartitionsTest.testAtomicStampedCompareAndSet`
- Client Nodes: `org.apache.ignite.testsuites.IgniteClientReconnectTestSuite: org.apache.ignite.internal.IgniteClientReconnectAtomicsWithLostPartitionsTest.testAtomicLongGet`
โ ๏ธ **No proven blocker yet** โ this is not an all-clear: see the tests above. 19 pre-existing/flaky filtered out.
โณ _Auto re-run **#1** in progress โ 4 watch suite(s) re-queued (attempt 1/2), **โ settled by 09:49 MSK**. This comment updates when they settle._
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-28528 Revise marshalling of the node filter in StartRequestData [ignite]
Posted by "anton-vinogradov (via GitHub)" <gi...@apache.org>.
anton-vinogradov commented on PR #13440:
URL: https://github.com/apache/ignite/pull/13440#issuecomment-5239417345
The wire format change is intentional and is the point of this ticket, so here is what actually changes and what does not.
**Changed** - four messages carry the deployment as one descriptor instead of four flat fields (`deployMode`, `userVersion`, `classLoaderId`, `participants`):
| message | `@Order` fields |
|---|---|
| `GridJobExecuteRequest` | 25 โ 22 |
| `DataStreamerRequest` | 16 โ 13 |
| `GridIoUserMessage` | 7 โ 4 |
| `GridEventStorageRequest` | 7 โ 4 |
**Not changed**, though the check lists them:
- `StartRequestData` - 8 fields before and after; only field visibility and the code reading them moved.
- `GridCacheMessage` - the type of an existing field was renamed, the field itself stays.
- `GridDeploymentInfoBean` / `GridDeploymentInfoMessage` - one class renamed under IGNITE-28967. Its own fields and its `directType` are untouched: the registration keeps its place in `CoreMessagesProvider`, and `META-INF/classnames.properties` is updated.
Rolling upgrade: this lands before 2.19 is released, which is the window such changes are meant for - the same reasoning as in IGNITE-28940. Between 2.18 and 2.19 these four messages are not compatible, deliberately.
One note on the check itself: `GridJobExecuteRequest` changes the most here and is **missing** from the list above, while three classes that do not change the format are listed. That matches the detection defects filed as IGNITE-28966 - the check is blind to messages in its own package and reports files whose change is not on the wire.
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-28528 Revise marshalling of the node filter in StartRequestData [ignite]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #13440:
URL: https://github.com/apache/ignite/pull/13440#issuecomment-5199439174
<!-- ignite-rolling-upgrade-check -->
## Possible compatibility issues. Please, check rolling upgrade cases
This PR modifies protected classes (with **Order** annotation).
Changes to these classes can break rolling upgrade compatibility.
**Affected files:**
- `modules/codegen/src/main/java/org/apache/ignite/internal/MessageProcessor.java`
- `modules/core/src/main/java/org/apache/ignite/internal/managers/communication/GridIoUserMessage.java`
- `modules/core/src/main/java/org/apache/ignite/internal/managers/eventstorage/GridEventStorageRequest.java`
- `modules/core/src/main/java/org/apache/ignite/internal/processors/continuous/StartRequestData.java`
- `modules/core/src/main/java/org/apache/ignite/internal/processors/continuous/StartRoutineDiscoveryMessage.java`
- `modules/core/src/main/java/org/apache/ignite/internal/processors/datastreamer/DataStreamerRequest.java`
- `modules/core/src/test/java/org/apache/ignite/internal/codegen/MessageProcessorTest.java`
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-28528 Revise marshalling of the node filter in StartRequestData [ignite]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #13440:
URL: https://github.com/apache/ignite/pull/13440#issuecomment-5199679009
<!-- ignite-rolling-upgrade-check -->
## Possible compatibility issues. Please, check rolling upgrade cases
This PR modifies protected classes (with **Order** annotation).
Changes to these classes can break rolling upgrade compatibility.
**Affected files:**
- `modules/codegen/src/main/java/org/apache/ignite/internal/MessageProcessor.java`
- `modules/core/src/main/java/org/apache/ignite/internal/managers/communication/GridIoUserMessage.java`
- `modules/core/src/main/java/org/apache/ignite/internal/managers/eventstorage/GridEventStorageRequest.java`
- `modules/core/src/main/java/org/apache/ignite/internal/processors/continuous/StartRequestData.java`
- `modules/core/src/main/java/org/apache/ignite/internal/processors/continuous/StartRoutineDiscoveryMessage.java`
- `modules/core/src/main/java/org/apache/ignite/internal/processors/datastreamer/DataStreamerRequest.java`
- `modules/core/src/test/java/org/apache/ignite/internal/codegen/MessageProcessorTest.java`
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-28528 Revise marshalling of the node filter in StartRequestData [ignite]
Posted by "petrov-mg (via GitHub)" <gi...@apache.org>.
petrov-mg commented on PR #13440:
URL: https://github.com/apache/ignite/pull/13440#issuecomment-5234006485
Generated code for `GridEventStorageRequestMarshaller` looks confusing:
```
/** */
@Override public void unmarshal(GridEventStorageRequest msg, GridKernalContext kctx, CacheObjectContext cacheObjCtx, ClassLoader clsLdr) throws IgniteCheckedException {
CacheObjectContext ctx = cacheObjCtx;
clsLdr = kctx.deploy().classLoader(msg);
```
Why is `clsLdr` overwritten here?
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-28528 Revise marshalling of the node filter in StartRequestData [ignite]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #13440:
URL: https://github.com/apache/ignite/pull/13440#issuecomment-5228103732
<!-- ignite-rolling-upgrade-check -->
## Possible compatibility issues. Please, check rolling upgrade cases
This PR modifies protected classes (with **Order** annotation).
Changes to these classes can break rolling upgrade compatibility.
**Affected files:**
- `modules/codegen/src/main/java/org/apache/ignite/internal/MessageProcessor.java`
- `modules/core/src/main/java/org/apache/ignite/internal/managers/communication/GridIoUserMessage.java`
- `modules/core/src/main/java/org/apache/ignite/internal/managers/eventstorage/GridEventStorageRequest.java`
- `modules/core/src/main/java/org/apache/ignite/internal/processors/continuous/StartRequestData.java`
- `modules/core/src/main/java/org/apache/ignite/internal/processors/continuous/StartRoutineDiscoveryMessage.java`
- `modules/core/src/main/java/org/apache/ignite/internal/processors/datastreamer/DataStreamerRequest.java`
- `modules/core/src/test/java/org/apache/ignite/internal/codegen/MessageProcessorTest.java`
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-28528 Revise marshalling of the node filter in StartRequestData [ignite]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #13440:
URL: https://github.com/apache/ignite/pull/13440#issuecomment-5229339734
<!-- ignite-rolling-upgrade-check -->
## Possible compatibility issues. Please, check rolling upgrade cases
This PR modifies protected classes (with **Order** annotation).
Changes to these classes can break rolling upgrade compatibility.
**Affected files:**
- `modules/codegen/src/main/java/org/apache/ignite/internal/MessageProcessor.java`
- `modules/core/src/main/java/org/apache/ignite/internal/managers/communication/GridIoUserMessage.java`
- `modules/core/src/main/java/org/apache/ignite/internal/managers/deployment/GridDeploymentInfoBean.java`
- `modules/core/src/main/java/org/apache/ignite/internal/managers/deployment/GridDeploymentInfoMessage.java`
- `modules/core/src/main/java/org/apache/ignite/internal/managers/eventstorage/GridEventStorageRequest.java`
- `modules/core/src/main/java/org/apache/ignite/internal/processors/cache/GridCacheMessage.java`
- `modules/core/src/main/java/org/apache/ignite/internal/processors/continuous/StartRequestData.java`
- `modules/core/src/main/java/org/apache/ignite/internal/processors/datastreamer/DataStreamerRequest.java`
- `modules/core/src/test/java/org/apache/ignite/internal/codegen/MessageProcessorTest.java`
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-28528 Revise marshalling of the node filter in StartRequestData [ignite]
Posted by "anton-vinogradov (via GitHub)" <gi...@apache.org>.
anton-vinogradov commented on PR #13440:
URL: https://github.com/apache/ignite/pull/13440#issuecomment-5231047611
**[Ignite PR Checker](https://ignite-pr-checker.is-a.dev/?pr=13440)** verdict ยท RunAll build [9268108](https://ci2.ignite.apache.org/build/9268108) ยท 147 suites ran, 0 reused
๐ **3 test(s) started failing on this code** โ not proven blockers yet: too few runs of this revision to tell a break from a flake, so a re-run of the suite decides it.
- Cache 7: `org.apache.ignite.testsuites.IgniteCacheTestSuite7: org.apache.ignite.internal.processors.cache.CacheGroupMetricsTest.testCacheGroupMetrics`
- Cache 8: `org.apache.ignite.testsuites.IgniteCacheTestSuite8: org.apache.ignite.internal.processors.cache.distributed.near.GridCacheNearAtomicMetricsSelfTest.testNearRead`
- Snapshots 1: `org.apache.ignite.testsuites.IgniteSnapshotTestSuite: org.apache.ignite.internal.processors.cache.persistence.snapshot.IgniteClusterSnapshotSelfTest.testClientHandlesSnapshotFailOnStartStage[encryption=true, onlyPrimay=false]`
โ ๏ธ **No proven blocker yet** โ this is not an all-clear: see the tests above. 15 pre-existing/flaky filtered out.
โณ _Auto re-run **#1** in progress โ 3 watch suite(s) re-queued (attempt 1/2). This comment updates when they settle._
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-28528 Revise marshalling of the node filter in StartRequestData [ignite]
Posted by "petrov-mg (via GitHub)" <gi...@apache.org>.
petrov-mg commented on code in PR #13440:
URL: https://github.com/apache/ignite/pull/13440#discussion_r3745411191
##########
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(),
Review Comment:
Is `depInfo` guaranteed to be non-null here? In some places it seems that it can be null, for example in `org.apache.ignite.internal.managers.communication.GridIoUserMessage#deploymentInfo`.
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-28528 Revise marshalling of the node filter in StartRequestData [ignite]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #13440:
URL: https://github.com/apache/ignite/pull/13440#issuecomment-5229378442
<!-- ignite-rolling-upgrade-check -->
## Possible compatibility issues. Please, check rolling upgrade cases
This PR modifies protected classes (with **Order** annotation).
Changes to these classes can break rolling upgrade compatibility.
**Affected files:**
- `modules/codegen/src/main/java/org/apache/ignite/internal/MessageProcessor.java`
- `modules/core/src/main/java/org/apache/ignite/internal/managers/communication/GridIoUserMessage.java`
- `modules/core/src/main/java/org/apache/ignite/internal/managers/deployment/GridDeploymentInfoBean.java`
- `modules/core/src/main/java/org/apache/ignite/internal/managers/deployment/GridDeploymentInfoMessage.java`
- `modules/core/src/main/java/org/apache/ignite/internal/managers/eventstorage/GridEventStorageRequest.java`
- `modules/core/src/main/java/org/apache/ignite/internal/processors/cache/GridCacheMessage.java`
- `modules/core/src/main/java/org/apache/ignite/internal/processors/continuous/StartRequestData.java`
- `modules/core/src/main/java/org/apache/ignite/internal/processors/datastreamer/DataStreamerRequest.java`
- `modules/core/src/test/java/org/apache/ignite/internal/codegen/MessageProcessorTest.java`
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-28528 Revise marshalling of the node filter in StartRequestData [ignite]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #13440:
URL: https://github.com/apache/ignite/pull/13440#issuecomment-5234224594
<!-- ignite-rolling-upgrade-check -->
## Possible compatibility issues. Please, check rolling upgrade cases
This PR modifies protected classes (with **Order** annotation).
Changes to these classes can break rolling upgrade compatibility.
**Affected files:**
- `modules/core/src/main/java/org/apache/ignite/internal/managers/communication/GridIoUserMessage.java`
- `modules/core/src/main/java/org/apache/ignite/internal/managers/deployment/GridDeploymentInfoBean.java`
- `modules/core/src/main/java/org/apache/ignite/internal/managers/deployment/GridDeploymentInfoMessage.java`
- `modules/core/src/main/java/org/apache/ignite/internal/managers/eventstorage/GridEventStorageRequest.java`
- `modules/core/src/main/java/org/apache/ignite/internal/processors/cache/GridCacheMessage.java`
- `modules/core/src/main/java/org/apache/ignite/internal/processors/continuous/StartRequestData.java`
- `modules/core/src/main/java/org/apache/ignite/internal/processors/datastreamer/DataStreamerRequest.java`
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-28528 Revise marshalling of the node filter in StartRequestData [ignite]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #13440:
URL: https://github.com/apache/ignite/pull/13440#issuecomment-5199210923
<!-- ignite-rolling-upgrade-check -->
## Possible compatibility issues. Please, check rolling upgrade cases
This PR modifies protected classes (with **Order** annotation).
Changes to these classes can break rolling upgrade compatibility.
**Affected files:**
- `modules/codegen/src/main/java/org/apache/ignite/internal/MessageProcessor.java`
- `modules/core/src/main/java/org/apache/ignite/internal/managers/communication/GridIoUserMessage.java`
- `modules/core/src/main/java/org/apache/ignite/internal/managers/eventstorage/GridEventStorageRequest.java`
- `modules/core/src/main/java/org/apache/ignite/internal/processors/continuous/StartRequestData.java`
- `modules/core/src/main/java/org/apache/ignite/internal/processors/datastreamer/DataStreamerRequest.java`
- `modules/core/src/test/java/org/apache/ignite/internal/codegen/MessageProcessorTest.java`
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-28528 Revise marshalling of the node filter in StartRequestData [ignite]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #13440:
URL: https://github.com/apache/ignite/pull/13440#issuecomment-5199859897
<!-- ignite-rolling-upgrade-check -->
## Possible compatibility issues. Please, check rolling upgrade cases
This PR modifies protected classes (with **Order** annotation).
Changes to these classes can break rolling upgrade compatibility.
**Affected files:**
- `modules/codegen/src/main/java/org/apache/ignite/internal/MessageProcessor.java`
- `modules/core/src/main/java/org/apache/ignite/internal/managers/communication/GridIoUserMessage.java`
- `modules/core/src/main/java/org/apache/ignite/internal/managers/eventstorage/GridEventStorageRequest.java`
- `modules/core/src/main/java/org/apache/ignite/internal/processors/continuous/StartRequestData.java`
- `modules/core/src/main/java/org/apache/ignite/internal/processors/continuous/StartRoutineDiscoveryMessage.java`
- `modules/core/src/main/java/org/apache/ignite/internal/processors/datastreamer/DataStreamerRequest.java`
- `modules/core/src/test/java/org/apache/ignite/internal/codegen/MessageProcessorTest.java`
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-28528 Revise marshalling of the node filter in StartRequestData [ignite]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #13440:
URL: https://github.com/apache/ignite/pull/13440#issuecomment-5199121207
<!-- ignite-rolling-upgrade-check -->
## Possible compatibility issues. Please, check rolling upgrade cases
This PR modifies protected classes (with **Order** annotation).
Changes to these classes can break rolling upgrade compatibility.
**Affected files:**
- `modules/codegen/src/main/java/org/apache/ignite/internal/MessageProcessor.java`
- `modules/core/src/main/java/org/apache/ignite/internal/managers/communication/GridIoUserMessage.java`
- `modules/core/src/main/java/org/apache/ignite/internal/managers/eventstorage/GridEventStorageRequest.java`
- `modules/core/src/main/java/org/apache/ignite/internal/processors/continuous/StartRequestData.java`
- `modules/core/src/main/java/org/apache/ignite/internal/processors/datastreamer/DataStreamerRequest.java`
- `modules/core/src/test/java/org/apache/ignite/internal/codegen/MessageProcessorTest.java`
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-28528 Revise marshalling of the node filter in StartRequestData [ignite]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #13440:
URL: https://github.com/apache/ignite/pull/13440#issuecomment-5228386330
<!-- ignite-rolling-upgrade-check -->
## Possible compatibility issues. Please, check rolling upgrade cases
This PR modifies protected classes (with **Order** annotation).
Changes to these classes can break rolling upgrade compatibility.
**Affected files:**
- `modules/codegen/src/main/java/org/apache/ignite/internal/MessageProcessor.java`
- `modules/core/src/main/java/org/apache/ignite/internal/managers/communication/GridIoUserMessage.java`
- `modules/core/src/main/java/org/apache/ignite/internal/managers/deployment/GridDeploymentInfoBean.java`
- `modules/core/src/main/java/org/apache/ignite/internal/managers/deployment/GridDeploymentInfoMessage.java`
- `modules/core/src/main/java/org/apache/ignite/internal/managers/eventstorage/GridEventStorageRequest.java`
- `modules/core/src/main/java/org/apache/ignite/internal/processors/cache/GridCacheMessage.java`
- `modules/core/src/main/java/org/apache/ignite/internal/processors/continuous/StartRequestData.java`
- `modules/core/src/main/java/org/apache/ignite/internal/processors/continuous/StartRoutineDiscoveryMessage.java`
- `modules/core/src/main/java/org/apache/ignite/internal/processors/datastreamer/DataStreamerRequest.java`
- `modules/core/src/test/java/org/apache/ignite/internal/codegen/MessageProcessorTest.java`
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-28528 Revise marshalling of the node filter in StartRequestData [ignite]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #13440:
URL: https://github.com/apache/ignite/pull/13440#issuecomment-5228560520
<!-- ignite-rolling-upgrade-check -->
## Possible compatibility issues. Please, check rolling upgrade cases
This PR modifies protected classes (with **Order** annotation).
Changes to these classes can break rolling upgrade compatibility.
**Affected files:**
- `modules/codegen/src/main/java/org/apache/ignite/internal/MessageProcessor.java`
- `modules/core/src/main/java/org/apache/ignite/internal/managers/communication/GridIoUserMessage.java`
- `modules/core/src/main/java/org/apache/ignite/internal/managers/deployment/GridDeploymentInfoBean.java`
- `modules/core/src/main/java/org/apache/ignite/internal/managers/deployment/GridDeploymentInfoMessage.java`
- `modules/core/src/main/java/org/apache/ignite/internal/managers/eventstorage/GridEventStorageRequest.java`
- `modules/core/src/main/java/org/apache/ignite/internal/processors/cache/GridCacheMessage.java`
- `modules/core/src/main/java/org/apache/ignite/internal/processors/continuous/StartRequestData.java`
- `modules/core/src/main/java/org/apache/ignite/internal/processors/continuous/StartRoutineDiscoveryMessage.java`
- `modules/core/src/main/java/org/apache/ignite/internal/processors/datastreamer/DataStreamerRequest.java`
- `modules/core/src/test/java/org/apache/ignite/internal/codegen/MessageProcessorTest.java`
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-28528 Revise marshalling of the node filter in StartRequestData [ignite]
Posted by "anton-vinogradov (via GitHub)" <gi...@apache.org>.
anton-vinogradov commented on code in PR #13440:
URL: https://github.com/apache/ignite/pull/13440#discussion_r3745472326
##########
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:
I was wrong here, and your question is what led me to it - thanks.
The two nodes are not the same in general. `GridCacheDeploymentManager` states it directly:
```java
assert sndId.equals(ldrId.globalId()) || participants != null;
```
and `GridDeploymentManager#deploy` hands out the deployment of the *original* owner when the class arrived by peer loading - the comment there calls it a nested execution:
```java
// Check for nested execution. In that case, if task
// is available locally by name, then we should ignore class loader ID.
dep = checkDeployment(ldrStore.getDeployment(ldr.classLoaderId()), "perLoader");
```
So a node that received classes from another one passes them further as a participant of the same deployment, while being the sender itself.
And the receiving side needs the sender, not the owner: `GridDeploymentPerVersionStore` asks it for the classes, records it with `addParticipant`, and refuses the deployment when `discovery().node(senderNodeId) == null`. Deriving the node from the loader id would break exactly the case where the owner has left and the sender still holds the classes.
Fixed: the resolution takes the node as a parameter again. Since generated code has no way to know the sender, the `DeploymentAware` interface is gone with it, and the callers - which do know the sender - pass it in.
##########
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:
Correction to my previous reply: the parameter is back.
The deployment resolution needs the sending node after all - it is not derivable from the loader id, see the other thread. So `p2pUnmarshal(UUID nodeId, GridKernalContext ctx)` keeps its signature, and the node is passed down to the resolution instead of only feeding an assert.
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-28528 Revise marshalling of the node filter in StartRequestData [ignite]
Posted by "anton-vinogradov (via GitHub)" <gi...@apache.org>.
anton-vinogradov commented on PR #13440:
URL: https://github.com/apache/ignite/pull/13440#issuecomment-5239537904
> There are some messages, that have `GridDeploymentInfoMessage` field, but doesn't implement `DeploymentAware`:
> - `GridJobExecuteRequest`
> - `GridIoUserMessage`
> - `GridCacheMessage`
> - `DataStreamerRequest`
>
> Is it planned, that these messages (or any other) become `DeploymentAware` in the future?
Thanks for the review. The answer turned out to be no, and `DeploymentAware` is gone from this PR entirely - it was removed a few hours before your review, so you were looking at the earlier version.
It let a message declare "my classes come with this deployment", and the generated marshaller resolved the class loader from it. @petrov-mg asked why `depInfo.classLoaderId().globalId()` was used as the sender node, and digging into that showed the interface was promising something a message cannot deliver.
Resolving a deployment needs the **sending** node, and it is not the owner of the class loader in general:
```java
assert sndId.equals(ldrId.globalId()) || participants != null; // GridCacheDeploymentManager
```
A node that received classes by peer loading passes them on as a participant of the same deployment - `GridDeploymentManager#deploy` hands it the deployment of the original owner, which the comment there calls a nested execution. The receiving side then asks the sender for the classes, records it with `addParticipant`, and drops the deployment when that node has left. Deriving it from the loader id breaks exactly the case participants exist for.
Generated code cannot supply the sender - `unmarshal` only gets a `GridKernalContext` - so the interface had no way to be correct. It is deleted, together with the resolution in the generator and its test; `modules/codegen` is no longer part of this PR.
What all these messages do share is the descriptor itself: they carry `GridDeploymentInfoMessage` instead of four flat fields, and each receiver makes one call - `globalDeployment(depInfo, clsName, sndNodeId)` - instead of assembling the deployment by hand. That was the point of the ticket, and it holds without the marker interface.
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-28528 Revise marshalling of the node filter in StartRequestData [ignite]
Posted by "anton-vinogradov (via GitHub)" <gi...@apache.org>.
anton-vinogradov merged PR #13440:
URL: https://github.com/apache/ignite/pull/13440
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org