anton-vinogradov commented on code in PR #13408:
URL: https://github.com/apache/ignite/pull/13408#discussion_r3683127586
##########
modules/core/src/main/java/org/apache/ignite/internal/processors/cache/distributed/near/GridNearGetRequest.java:
##########
@@ -67,15 +64,16 @@ public class GridNearGetRequest extends GridCacheIdMessage
implements GridCacheD
/** */
@GridToStringInclude
- private LinkedHashMap<KeyCacheObject, Boolean> keyMap;
+ @Marshalled(keys = "keys", values = "readersFlags")
+ Map<KeyCacheObject, Boolean> keyMap;
Review Comment:
No problem in practice, though the change is real — thanks for spotting it.
`keyMap()` has exactly one consumer:
`GridDhtCacheAdapter#processNearGetRequest:1292` → `getDhtAsync` →
`GridDhtGetFuture`. None of the steps there needs the order — `init()` passes
`keys.keySet()` to the preloader, reader registration is per-entry, and
`getDhtAllAsync` takes a key set. And the order dies at the next step anyway,
already in master: the local get result goes into `U.newHashMap`
(`GridDhtCacheAdapter:730`), `misses` into `new HashMap<>()` (:816), and
`GridDhtGetFuture#toEntryInfos` iterates that map to build the response. So the
entry order in `GridNearGetResponse` never matched the request order. The near
side matches by key, not by position
(`CacheDistributedGetFutureAdapter#onResult` → `createResultMap`,
`GridPartitionedGetFuture:600-620`).
`partition()` is unaffected: it runs on the NIO thread before unmarshal,
when `keyMap` is still null and `keys` is the wire-order `ArrayList`, so it
returns the same partition as master.
I also looked for the reason behind `LinkedHashMap`: `git log -S` traces it
to the very first import (`7016f154b20`), then it was carried over mechanically
in `832f114e8eb` when `keyBytes` was split into `keys`/`flags`. No ticket, no
comment, no test pins it.
One thing worth a follow-up rather than this PR: the `@Order` path *does*
derive the implementation from the declared type —
`MessageSerializerGenerator:542` emits `linked=true` for a `LinkedHashMap`
field and `readMap` then uses `U.newLinkedHashMap`.
`MessageMarshallerGenerator` hardcodes `U.newHashMap`/`U.newHashSet` instead,
so declaring the field `LinkedHashMap` here does not even compile (I tried:
`incompatible types: HashMap cannot be converted to LinkedHashMap`). Teaching
the marshaller generator to derive it the same way would remove the trap for
future `@Marshalled` fields where the order does matter. Happy to file that
separately if you agree.
--
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]
