IGNITE-27631 Use MessageSerializer for SingleNodeMessage#12827
IGNITE-27631 Use MessageSerializer for SingleNodeMessage#12827NSAmelchev wants to merge 2 commits intoapache:masterfrom
Conversation
NSAmelchev
force-pushed
the
ignite-27631
branch
7 times, most recently
from
b3cb9e7 to
c128a8e
Compare
modules/core/src/main/java/org/apache/ignite/internal/util/distributed/SingleNodeMessage.java
Show resolved
Hide resolved
| factory.register((short)22, TcpDiscoveryServerOnlyCustomEventMessage::new, | ||
| new TcpDiscoveryServerOnlyCustomEventMessageSerializer()); | ||
| factory.register((short)23, FullMessage::new, new FullMessageSerializer()); | ||
|
|
There was a problem hiding this comment.
Why spaced and exactly 86? No 24? Or 100? No comment on this?
There was a problem hiding this comment.
This message already exists - just use it as is, without changing the direct type. The current file's compactness is good — comments wouldn't help. Must be a framework-specific thing.
| factory.register((short)510, UserAcceptedMessage::new, new UserAcceptedMessageSerializer()); | ||
| factory.register((short)511, UserProposedMessage::new, new UserProposedMessageSerializer()); | ||
|
|
||
| factory.register((short)520, SnapshotOperationResponse::new, new SnapshotOperationResponseSerializer()); |
There was a problem hiding this comment.
Are they custom messages? I believe we've met a problem of the messages sharing between Discovery and Communication. This wasn't significant until the shared messages number exceeds 5 maybe. The messages sharing should be discussed. We might introduce a message number replacement at the message registering. Or even use a message-content-generated hash/id to exclude the manual-processed numbers.
There was a problem hiding this comment.
If we do smth. like this (the messages reuse) , TcpDiscoveryNodeMetricsMessage, TcpDiscoveryCacheMetricsMessage, TcpDiscoveryNodeFullMetricsMessage should be reverted too. Look for "We cannot directly reuse {@link".
There was a problem hiding this comment.
I think it's normal when a message is forwarded through different communication protocols. To me, serialization is a more high-level abstraction. Especially for RU purposes.
There was a problem hiding this comment.
Consider the case when id of a shared message is already occupied in Discovery and you can't reuse. I belive that using of ids from one domain should not be "as is" in other domain where messages and their numeration are different by nature.
|
3 participants
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
The description explains WHAT and WHY was made instead of HOW.
The following pattern must be used:
IGNITE-XXXX Change summarywhereXXXX- number of JIRA issue.(see the Maintainers list)
the
green visaattached to the JIRA ticket (see tabPR Checkat 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.