[
https://issues.apache.org/jira/browse/IGNITE-26631?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18132785#comment-18132785
]
Reza Taroosheh commented on IGNITE-26631:
-----------------------------------------
Hi [~vladsz83], thanks for the review and the context on the rolling upgrade.
I'd like to understand what we'd gain by combining the two messages. My concern
is that {{GridCacheIdMessage}} and {{GridCacheGroupIdMessage}} carry different
IDs with different meanings, so a merged message would have one field with two
meanings, and the two could no longer evolve separately. The current PR keeps
them separate and follows the open/closed principle: a new message type
overrides {{descriptor()}} instead of adding another {{instanceof}} branch in
{{GridCacheIoManager}}.
Is this refactoring useful on its own, or would you prefer to close it in favor
of the merge? If you prefer the merge, one option might avoid the rolling
upgrade concern: as far as I can see, both messages serialize a single int at
the same position ({{cacheId}} / {{grpId}}). So we could move that int into a
common base class at the same position and remove the two old fields. The bytes
sent between nodes should then stay the same, and only the wire-compatibility
check files would need to be regenerated. This is based on reading the code, so
we would need to confirm it with a compatibility test between the old and new
versions. Did you mean that only the wire-compatibility check would flag the
change, or that the bytes sent between nodes would actually be different?
> Combine or refactor GridCacheGroupIdMessage and GridCacheIdMessage
> ------------------------------------------------------------------
>
> Key: IGNITE-26631
> URL: https://issues.apache.org/jira/browse/IGNITE-26631
> Project: Ignite
> Issue Type: Improvement
> Reporter: Vladimir Steshin
> Assignee: Reza Taroosheh
> Priority: Minor
> Labels: IEP-132, ise, wire-format
> Fix For: 2.19
>
> Time Spent: 40m
> Remaining Estimate: 0h
>
> These messages are similar and are only for providing a cache or a group id
> to find proper cache message handler. Currently, we use `{_}instance of{_}`.
> Instead, we should revise these messages and probably combine them.
>
> Also, check in {_}GridCacheIoManager{_}:
> {code:java}
> private Object descriptorForMessage(GridCacheMessage msg) {
> if (msg instanceof GridCacheIdMessage)
> return
> cctx.cache().cacheDescriptor(((GridCacheIdMessage)msg).cacheId());
> else if (msg instanceof GridCacheGroupIdMessage)
> return
> cctx.cache().cacheGroupDescriptors().get(((GridCacheGroupIdMessage)msg).groupId());
> return null;
> } {code}
>
--
This message was sent by Atlassian Jira
(v10.3.26#10030026)
