anton-vinogradov commented on code in PR #13095:
URL: https://github.com/apache/ignite/pull/13095#discussion_r3596183185
##########
modules/core/src/main/java/org/apache/ignite/internal/processors/cache/GridCacheIoManager.java:
##########
@@ -1136,11 +1138,13 @@ public void send(ClusterNode node, GridCacheMessage
msg, byte plc) throws Ignite
if (log.isDebugEnabled())
log.debug("Sending cache message [msg=" + msg + ", node=" +
U.toShortString(node) + ']');
+ GridIoMessage ioMsg = cctx.gridIO().prepare(TOPIC_CACHE, msg, plc,
false, 0, false);
Review Comment:
It already is, for the normal path: `GridIoManager.send()` marshals
internally (`MessageMarshalling.marshal` → `sendMarshalled`), so regular
callers never see marshalling. `prepare`/`sendPrepared` surfaces in exactly two
places — this retry loop and the ordered one below — because there the
marshal-once contract (`MessageMarshalOnceTest`) must span the caller's retry
loop, and the retry policy (what's recoverable, topology re-check) is
cache-level knowledge. Pushing it deeper while retries re-enter `send()` would
need an "already marshalled" flag on the message itself — replied on that in
your comment below.
##########
modules/core/src/main/java/org/apache/ignite/internal/processors/cache/GridCacheIoManager.java:
##########
@@ -1197,11 +1201,19 @@ public void sendOrderedMessage(ClusterNode node, Object
topic, GridCacheMessage
msg.lastAffinityChangedTopologyVersion(cctx.exchange().lastAffinityChangedTopologyVersion(msg.topologyVersion()));
+ if (node.isLocal()) {
+ cctx.gridIO().sendOrderedMessage(node, topic, msg, plc, timeout,
false);
+
+ return;
+ }
+
+ GridIoMessage ioMsg = cctx.gridIO().prepare(topic, msg, plc, true,
timeout, false);
Review Comment:
The wrap variant is in place: `GridIoMessage` carries a transient `prepared`
flag set by `prepare()` and asserted by `sendPrepared()` (your earlier
suggestion). Going further — retry re-entering plain `send(msg)` — doesn't work
through the wrap: `prepare` is what creates the wrap, so each attempt would get
a fresh one and lose the flag; marshal-once across attempts then needs the flag
on the inner message plus abstract `prepare/prepare0` on a generated base.
That's the exact shape this PR removes: master's
`GridCacheMessage.prepareMarshal()`/`finishUnmarshal()` put transport state and
marshal logic on the message, and the extraction into external marshallers
(enforced by `MessageSerializationArchitectureTest` — messages stay passive
DTOs) is the core of the change. Mechanically it's also invasive:
`MarshallableMessage` is an interface, so the flag means a generated field in
every message or a new base class wedged into existing hierarchies — for the
benefit of these two re
try call sites.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
