IGNITE-28203 Use MarshalableMessage for ErrorMessage by anton-vinogradov · Pull Request #12878 · apache/ignite

anton-vinogradov · 2026-03-11T14:44:03Z

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

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

The Contribution Checklist

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

Notes

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

modules/core/src/main/java/org/apache/ignite/internal/managers/communication/GridIoManager.java

github-actions · 2026-03-11T16:34:38Z

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/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/message/CalciteErrorMessage.java
modules/core/src/main/java/org/apache/ignite/internal/managers/communication/ErrorMessage.java
modules/core/src/main/java/org/apache/ignite/spi/discovery/tcp/messages/TcpDiscoveryClientReconnectMessage.java
modules/core/src/main/java/org/apache/ignite/spi/discovery/tcp/messages/TcpDiscoveryJoinRequestMessage.java
modules/core/src/main/java/org/apache/ignite/spi/discovery/tcp/messages/TcpDiscoveryNodeAddFinishedMessage.java

...re/src/main/java/org/apache/ignite/internal/managers/communication/GridIoMessageFactory.java

modules/core/src/main/java/org/apache/ignite/internal/managers/communication/GridIoManager.java

...ore/src/main/java/org/apache/ignite/internal/managers/discovery/DiscoveryMessageFactory.java

Vladsz83 · 2026-03-13T14:26:29Z

...in/java/org/apache/ignite/internal/processors/query/calcite/message/CalciteErrorMessage.java

+
+    /** {@inheritDoc} */
+    @Override public void prepareMarshal(GridCacheSharedContext<?, ?> ctx) throws IgniteCheckedException {
+        if (err != null)


Usually, we do && errBytes == null

Yes, and the reason is not clear for me.
This MUST happen once.

Vladsz83 · 2026-03-13T14:27:04Z

...in/java/org/apache/ignite/internal/processors/query/calcite/message/CalciteErrorMessage.java

+
+    /** {@inheritDoc} */
+    @Override public void prepareUnmarshal(GridCacheSharedContext<?, ?> ctx) throws IgniteCheckedException {
+        if (errBytes != null)


Usually w no && err == null

Yes, and the reason is not clear for me.
This MUST happen once.

Vladsz83 · 2026-03-13T14:27:32Z

...in/java/org/apache/ignite/internal/processors/query/calcite/message/CalciteErrorMessage.java

+    /** {@inheritDoc} */
+    @Override public void prepareUnmarshal(GridCacheSharedContext<?, ?> ctx) throws IgniteCheckedException {
+        if (errBytes != null)
+            err = U.unmarshal(ctx.marshaller(), errBytes, U.resolveClassLoader(ctx.cache().context().gridConfig()));


no keed to keep the bytes any more. Less memory consumption

Just let gc do it work.
This message has short life. Everything will be cleared after use.

GC won't deal with alive not nulls. My suggestion is not to consider how long message lives. Just do not keep obsoletes. WDYT?

I'ts a preoptimisation.
Don't like such things.

modules/codegen/src/main/java/org/apache/ignite/internal/MessageSerializerGenerator.java

Vladsz83 · 2026-03-13T14:59:47Z

modules/core/src/main/java/org/apache/ignite/internal/managers/communication/ErrorMessage.java

-        }
+    /** {@inheritDoc} */
+    @Override public void finishUnmarshal(Marshaller marsh, ClassLoader clsLdr) throws IgniteCheckedException {
+        if (errBytes != null)


Usually we do && err != null. The same in the other places

modules/core/src/main/java/org/apache/ignite/internal/managers/communication/ErrorMessage.java

...ore/src/main/java/org/apache/ignite/internal/managers/discovery/DiscoveryMessageFactory.java

Vladsz83 · 2026-03-13T15:17:38Z

...in/java/org/apache/ignite/spi/discovery/tcp/messages/TcpDiscoveryClientReconnectMessage.java

-        }
+    @Override public void finishUnmarshal(Marshaller marsh, ClassLoader clsLdr) throws IgniteCheckedException {
+        if (msgsBytes != null)
+            msgs = U.unmarshal(marsh, msgsBytes, clsLdr);


Let's do not keep msgsBytes any more.

Vladsz83 · 2026-03-13T15:21:15Z

modules/core/src/main/java/org/apache/ignite/internal/managers/communication/GridIoManager.java

        List<MessageFactoryProvider> compMsgs = new ArrayList<>();

-        compMsgs.add(new GridIoMessageFactory());
+        compMsgs.add(new GridIoMessageFactory(marsh, U.gridClassLoader()));


Why not ctx.marshallerContext().jdkMarshaller() ? Is widely used.

Has no answer, unfortunatelly

sonarqubecloud · 2026-03-13T16:30:42Z

Quality Gate failed

Failed conditions
18.9% Duplication on New Code (required ≤ 5%)

See analysis details on SonarQube Cloud

anton-vinogradov added 3 commits March 11, 2026 17:43

WIP

7ca597b

WIP

e0996f4

WIP

3e1b383

Vladsz83 reviewed Mar 11, 2026

View reviewed changes

modules/core/src/main/java/org/apache/ignite/internal/managers/communication/GridIoManager.java Outdated Show resolved Hide resolved

Merge remote-tracking branch 'origin/master' into ignite-28203

41d86b5

github-actions bot added the compatibility label Mar 11, 2026

Vladsz83 reviewed Mar 12, 2026

View reviewed changes

...re/src/main/java/org/apache/ignite/internal/managers/communication/GridIoMessageFactory.java Outdated Show resolved Hide resolved

Vladsz83 reviewed Mar 12, 2026

View reviewed changes

modules/core/src/main/java/org/apache/ignite/internal/managers/communication/GridIoManager.java Outdated Show resolved Hide resolved

anton-vinogradov added 4 commits March 12, 2026 15:58

WIP

4d1d8f7

WIP

5271be4

WIP

1311312

WIP

10064ba

Vladsz83 reviewed Mar 12, 2026

View reviewed changes

...ore/src/main/java/org/apache/ignite/internal/managers/discovery/DiscoveryMessageFactory.java Outdated Show resolved Hide resolved

WIP

9349d55

Vladsz83 reviewed Mar 13, 2026

View reviewed changes

anton-vinogradov added 4 commits March 13, 2026 18:56

WIP

5ffcb22

WIP

769bd08

WIP

2aae0b5

WIP

f2ccbe7

Vladsz83 approved these changes Mar 13, 2026

View reviewed changes

anton-vinogradov merged commit 81dda22 into apache:master Mar 13, 2026
9 of 10 checks passed

Conversation

anton-vinogradov commented Mar 11, 2026

The Contribution Checklist

Notes

Uh oh!

Uh oh!

github-actions bot commented Mar 11, 2026

Possible compatibility issues. Please, check rolling upgrade cases

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sonarqubecloud bot commented Mar 13, 2026

Quality Gate failed

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants