[
https://issues.apache.org/jira/browse/IGNITE-28940?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Anton Vinogradov updated IGNITE-28940:
--------------------------------------
Labels: IEP-132 compatibility wire-format (was: IEP-132 compatibility)
> Choose the marshaller by transport, not by message class
> --------------------------------------------------------
>
> Key: IGNITE-28940
> URL: https://issues.apache.org/jira/browse/IGNITE-28940
> Project: Ignite
> Issue Type: Sub-task
> Components: messaging
> Reporter: Anton Vinogradov
> Assignee: Anton Vinogradov
> Priority: Major
> Labels: IEP-132, compatibility, wire-format
> Fix For: 2.19
>
>
> *Wire format change. Do this before 2.19 is released - after that it would
> break rolling upgrade.*
> h3. Goal
> Take the marshaller from the place where we marshal, not from the message
> class.
> h3. Why
> Binary cannot be used where the marshalling cannot afford a cluster-wide
> class registration, and that is a property of the call site.
> The registration itself is {{MarshallerContextImpl#registerClassName}} ->
> {{proposeMapping}} -> {{fut.get()}}, which waits for discovery. Ignite
> classes usually skip it, because 2276 names are pre-accepted from
> {{META-INF/classnames.properties}}. But that list is not closed under
> nesting: a plain {{CacheConfiguration}} needs 9 types registered and two of
> them are absent -
> {{javax.cache.configuration.FactoryBuilder$SingletonFactory}} and
> {{javax.cache.expiry.EternalExpiryPolicy}}, both coming from the default
> expiry policy. A discovery message carrying a cache configuration would
> therefore register types from inside the discovery thread.
> Measured on the same objects, binary is the better marshaller *when types are
> registered*: node attributes 2423 bytes / 5.2 us against 2725 / 30.0 for jdk,
> {{CacheConfiguration}} 857 bytes against 4539. So the split is not "binary is
> for user data" - it is "binary needs a place where registration is allowed".
> User classes travel with jdk in a dozen places today -
> {{TcpDiscoveryNode#attrs}} and {{#consistentId}}, {{StoredCacheData#ccfg}}
> and {{#qryEntities}}, {{DynamicCacheChangeRequest#startCfg}},
> {{LazyServiceConfigurationMessage#affKey}},
> {{QueryEntityMessage#dfltFieldValues}}, {{PluginsDataBagItem#data}},
> {{SerializableDataBagItemWrapper#data}},
> {{PartitionHashRecord#consistentId}}, {{TransactionsHashRecord}} and
> {{ErrorMessage#err}} - the full list is in IGNITE-28935.
> The call sites are already separated by transport, and transport is a
> reliable indicator of that:
> * communication - {{GridIoManager}} (marshal on send and the unmarshal
> paths), {{GridCacheIoManager}}, {{IgniteTxManager}},
> {{GridDhtPartitionsExchangeFuture}}, {{CacheContinuousQueryHandler}},
> {{DataStreamProcessor}}, calcite {{MessageServiceImpl}};
> * discovery - {{TcpDiscoveryIoSession}} and ZooKeeper
> {{DiscoveryMessageParser}}.
> The hand-written half of the API already works this way:
> {{MarshallableMessage#marshal(Marshaller)}} takes the marshaller as a
> parameter. Only generated code keeps it in a field, which is what pins it to
> the class.
> h3. How
> * add a {{Marshaller}} parameter to {{MessageMarshaller}} and
> {{MessageMarshalling}}; the generator stops storing it and passes it to
> nested messages;
> * communication call sites pass {{ctx.marshaller()}}, discovery call sites
> pass {{marshallerContext().jdkMarshaller()}};
> * delete {{@UseBinaryMarshaller}},
> {{AbstractMarshallableMessageFactoryProvider#init}} and both marshaller
> fields.
> The wire format changes for four classes, deliberately:
> * {{ErrorMessage}}, {{PartitionHashRecord}} and {{TransactionsHashRecord}}:
> jdk to binary. This is safe for {{ErrorMessage}}, because
> {{java.lang.Throwable}} declares {{writeObject}} and {{readObject}}. Every
> exception matches {{isCustomJavaSerialization}}, so binary passes it to
> {{OptimizedMarshaller}}: 709 bytes with jdk against 715 with binary, 6.7 us
> against 7.4. The bytes change, the behaviour does not.
> * {{BinaryMetadataVersionInfo}}: binary to jdk, the same as
> {{MetadataUpdateProposedMessage}}. Not needed if IGNITE-28939 is done first.
> Note: {{MessageMarshaller}} is in the public package
> {{org.apache.ignite.plugin.extensions.communication}}. Outside the generator
> only three tests implement it.
> h3. Expected result
> No annotations left. One marshaller is used for the whole message tree of one
> send, so a wrong marshaller from inheritance or from nesting is not possible.
> When the set of pre-registered types changes, or a self-contained binary mode
> appears, only the call sites have to be revisited - not sixty classes.
> h3. How to verify
> Run All. Also continuous queries, idle_verify, {{DistributedProcess}} and
> calcite. Add a test that sends the same message type over both transports.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
