Skip to content

IGNITE-28901 Split GridEventStorageMessage into a request and a response - #13428

Merged
anton-vinogradov merged 1 commit into
apache:masterfrom
anton-vinogradov:ignite-28901
Aug 4, 2026
Merged

IGNITE-28901 Split GridEventStorageMessage into a request and a response#13428
anton-vinogradov merged 1 commit into
apache:masterfrom
anton-vinogradov:ignite-28901

Conversation

@anton-vinogradov

@anton-vinogradov anton-vinogradov commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

IGNITE-28901

GridEventStorageMessage served both directions, and the two halves never overlapped: the request constructor nulled evts and errMsg, the response constructor nulled the six request fields. Every response carried six always-null fields on the wire, every request carried two.

That conflation is also what made the filter awkward. The filter is a user class, so it can only be unmarshalled once the deployment class loader is known, which makes the message deferred. Deferral applies to a whole message, so the response was dragged along and had to be unmarshalled by hand in its listener, for no reason of its own.

So the message is split in two:

  • GridEventStorageRequest - deferred, carries the filter as a normal @Marshalled field. The receiver unmarshals it right after it resolves the deployment, with the same class loader the old finishUnmarshalFilters was given. GridJobExecuteRequest works this way since IGNITE-28356;
  • GridEventStorageResponse - a plain Message again, unmarshalled by the generic inbound pass. Its listener is back to what it was.

The response topic also stops being a marshalled blob. It was a GridTopic tuple wrapped in a GridTopicMessage, built in one place out of the sender's node id, so the request now carries that id and rebuilds TOPIC_EVENT.topic(id) on the other side. T1.equals/hashCode compare the topic and the id, so the listener still matches. Without it, deferring the message would have hidden the response topic behind an unmarshal that has to happen before the deployment is resolved - a failure to resolve one still has to be answered, and the answer goes to that topic.

Gone: the MarshallableMessage interface, marshal, unmarshal, finishUnmarshalFilters, the GridTopicMessage wrapper and the marsh field of the manager. One MessageMarshalling.unmarshal call is left, and it is the one that has a reason to exist.

Wire format

This changes it: two message types instead of one, with the fields split between them and the response topic sent as an IgniteUuid. It has to be merged before 2.19 is released.

The extra registration also shifts direct type ids, but only inside the block it belongs to: CoreMessagesProvider resets msgIdx per block, so the 14 messages that follow in [13000 - 13300] move by one and nothing else does. The block holds 15 of its 301 slots, and no block in the provider is close to full.

One unrelated line

GridIoManager gets a TODO and nothing else. While checking why removing the deferral made a test hang instead of fail, it turned out the regular receive path drops a message that cannot be unmarshalled without writing anything to the log, while the ordered path catches, logs and skips it on purpose. Filed as IGNITE-28950; the marker is here so the next reader of that method finds it.

Checks

  • GridP2PTimeoutSelfTest - 8 of 8. The one that matters: it runs a remoteQuery with a filter loaded by an external class loader, so the filter really travels and really needs the deployment class loader on the other side;
  • GridP2PMissedResourceCacheSizeSelfTest - 8 of 8;
  • GridEventStorageSelfTest - 6 of 6;
  • GridEventStorageDefaultExceptionTest - 2 of 2;
  • IgniteCoreMessagesSerializationTest - green;
  • checkstyle with -Pcheckstyle - no violations.

🤖 Generated with Claude Code

@anton-vinogradov anton-vinogradov changed the title IGNITE-28901 Use the generated marshalling for the GridEventStorageMessage filter IGNITE-28901 Split GridEventStorageMessage into a request and a response Aug 3, 2026
@anton-vinogradov
anton-vinogradov force-pushed the ignite-28901 branch 2 times, most recently from bac4ba0 to 97d95f6 Compare August 3, 2026 22:54
@anton-vinogradov

anton-vinogradov commented Aug 3, 2026

Copy link
Copy Markdown
Contributor Author

/runall


🚀 RunAll queuedbuild 9250889 · live progress & verdict: Ignite PR Checker. The verdict lands here when the run finishes.
🏁 Run finished — the verdict comment has the full story.

@anton-vinogradov

anton-vinogradov commented Aug 4, 2026

Copy link
Copy Markdown
Contributor Author

Ignite PR Checker verdict · RunAll build 9250889 · 147 suites ran, 0 reused

No blockers — nothing in this run looks caused by this PR. 60 pre-existing/flaky tests filtered out.

♻️ Settled after 1 auto re-run wave(s): #1 — 1 blocker suite(s).

@wernerdv wernerdv left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GridEventStorageMessage needs to be removed from modules/core/src/main/resources/META-INF/classnames.properties.

@anton-vinogradov

Copy link
Copy Markdown
Contributor Author

Done, thanks — the line is gone.

I did not add the two new classes in its place, and I want to say why, in case you meant that too. The list pre-registers names for classes that are marshalled by name. A message with a generated serializer travels by its direct type id, so its class name never reaches the wire. None of the messages written since the serializer migration are in the file:

class in classnames.properties
ErrorMessage no
GridTopicMessage no
CompressedMessage no
QueryEntityMessage no
IgniteFeatureSet no
RollingUpgradeClusterData no

The old entry looks like a leftover from the time GridEventStorageMessage was marshalled as an object. Its neighbour GridJobExecuteRequest is there for the same historical reason, and there are probably more — worth a sweep in a separate ticket, but I did not want to widen this PR.

Happy to add both names if you think they are needed for a case I am missing.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Aug 4, 2026

Copy link
Copy Markdown

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/eventstorage/GridEventStorageMessage.java
  • modules/core/src/main/java/org/apache/ignite/internal/managers/eventstorage/GridEventStorageRequest.java
  • modules/core/src/main/java/org/apache/ignite/internal/managers/eventstorage/GridEventStorageResponse.java

@anton-vinogradov

Copy link
Copy Markdown
Contributor Author

Ignite PR Checker verdict · RunAll build 9253516 · 147 suites ran, 0 reused

No blockers — nothing in this run looks caused by this PR. 60 pre-existing/flaky tests filtered out.

@anton-vinogradov
anton-vinogradov merged commit 4b92f71 into apache:master Aug 4, 2026
7 of 8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants