Anton Vinogradov created IGNITE-28935:
-----------------------------------------
Summary: Simplify marshaller selection for message object fields
Key: IGNITE-28935
URL: https://issues.apache.org/jira/browse/IGNITE-28935
Project: Ignite
Issue Type: Task
Components: messaging
Reporter: Anton Vinogradov
Assignee: Anton Vinogradov
Fix For: 2.19
h3. Goal
Make it obvious - and mechanically checkable - which marshaller every message
field is serialized with, and cut the number of places where that decision is
taken from 60 to zero.
h3. Why
Object fields of messages are turned into bytes either by the schema-less
{{JdkMarshaller}} or by the schema-aware {{BinaryMarshaller}}. The marshaller
is bound to the message *class*: the {{@UseBinaryMarshaller}} annotation plus
the registration call in {{CoreMessagesProvider}}.
Measured on master over the generated companions:
* 297 messages are registered in core (58 binary, 239 jdk), 195 {{*Marshaller}}
companions are generated;
* only 30 companions hold a {{Marshaller}} at all, and the choice affects 24
classes;
* of 60 {{@UseBinaryMarshaller}} annotations only 13 mean anything - the other
47 are noise.
The annotation is meant to say "the message carries user classes", but that is
not what it does. User data travels with the jdk marshaller in
{{TcpDiscoveryNode#attrs}}, {{LazyServiceConfigurationMessage#affKey}},
{{StoredCacheData#ccfg}}, {{QueryEntityMessage#dfltFieldValues}},
{{PartitionHashRecord#consistentId}}. What actually decides is whether the
marshalling may block on a cluster-wide class registration: binary - and
{{OptimizedMarshaller}} inside it - goes
{{MarshallerContextImpl#registerClassName}} -> {{proposeMapping}} ->
{{fut.get()}}, which is impossible on the discovery thread, during join, and
for on-disk formats read by offline tools. That is a property of the call site,
not of the class.
Binding it to the class produces three defects:
# The annotation is not {{@Inherited}}, while the generator builds a companion
from all fields including inherited ones
({{MessageCompanionGenerator#enclosedFields}}). Nine parent/child pairs are
registered in different categories. Verified by adding a blob field to
{{GridDistributedTxPrepareRequest}} (binary): in the generated
{{GridNearTxPrepareRequestMarshaller}} (jdk) the very same inherited field is
marshalled with jdk. No such field exists today, so this is a latent trap
rather than a live bug.
# A nested message overrides the enclosing one, because the marshaller is
resolved per {{directType}}: {{ErrorMessage}} (jdk) travels inside binary
messages, and {{BinaryMetadataVersionInfo}} (binary) travels inside the jdk
discovery data bag {{CacheBinaryDataBagItem}} - while the same
{{BinaryMetadata}} in {{MetadataUpdateProposedMessage}} is marshalled with jdk.
It works only because those classes are pre-registered in
{{META-INF/classnames.properties}} and need no ring round-trip.
# {{MarshallableMessage}} conflates "needs a pre-marshal hook" with "needs a
marshaller": 8 of its 21 implementations use neither the marshaller nor the
class loader.
h3. Plan
Each subtask is independently mergeable and useful on its own.
# (pending) - *Remove dead marshalling code from
{{GridDhtAtomicUpdateRequest}}.* {{forceTransformBackups}} is {{false}} at both
creation sites, so the hand-written marshalling and the fields it fills are
unreachable. Result: one boolean and three always-null collection fields leave
the wire, and one of the 11 meaningful annotations goes away.
# (pending) - *Register core messages uniformly.*
{{withSchema}}/{{withNoSchema}} are identical after IGNITE-28929, and the
marshaller is threaded into serializer and deployer lookups that never take
one. Result: a single registration form, the marshaller stated once, generated
code byte-identical.
# (pending) - *Split {{MarshallableMessage}} into a hook and a marshalling
contract.* 8 of its 21 implementations use neither the marshaller nor the class
loader - they only reshape their own fields. Result: 'needs a marshaller'
becomes visible in the type, and 8 companions lose the field.
# (pending) - *Replace marshaller blobs of internal types with regular message
fields* - {{BinaryMetadata}} (two messages), {{BaselineTopology}}, job
siblings, query entities. Binary yields no schema for them anyway, since
{{Externalizable}} types fall back to {{OptimizedMarshaller}} inside it.
Result: blob fields 24 -> ~19, and the {{BinaryMetadata}} inconsistency
disappears.
# (pending) - *Select the marshaller by transport instead of per message class*
- the core change. Communication call sites pass binary, discovery call sites
pass jdk, and the generator passes it down the message tree. Result: zero
annotations, the inherited-field trap and the nested-message override become
impossible, and the wire format changes for four classes deliberately.
# (pending) - *Test that discovery messages never require cluster-wide class
registration.* Today the rule holds by luck in at least one place - only
because the classes involved are pre-registered in {{classnames.properties}}.
Result: the rule is checked in CI instead of remembered.
Order: the first two are cheap and independent of everything else; the hook
split and the blob removal both shrink the set of classes the transport change
has to touch, so they come before it; the test closes the loop last.
h3. Expected result
|| || before || after ||
| places where the marshaller is chosen | 60 annotations, two registration
helpers | 0 annotations, marshaller comes from the call site |
| companions holding a {{Marshaller}} | 30 | ~12, hand-written marshalling only
|
| classes given a marshaller they never use | 6 | 0 |
| blob fields of internal types | 24 | ~19 |
| inherited-field and nested-message mismatches | possible | impossible by
construction |
The number of marshaller implementations does not change - all three are
load-bearing: jdk serializes without any cluster context, binary gives field
access without the class, and optimized is the fallback binary itself relies on
for {{Externalizable}} and {{writeObject}}/{{readObject}} types (and the REST
client protocol).
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
