[jira] [Updated] (IGNITE-28940) Choose the marshaller by transport, not by message class
[
https://issues.apache.org/jira/browse/IGNITE-28940?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Ignite TC Bot updated IGNITE-28940:
---
Ignite Flags: (was: Release Notes Required)
> 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, ise, wire-format
> Fix For: 2.19
>
> Time Spent: 12h 20m
> Remaining Estimate: 0h
>
> *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,
[jira] [Updated] (IGNITE-28940) Choose the marshaller by transport, not by message class
[
https://issues.apache.org/jira/browse/IGNITE-28940?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Ignite TC Bot updated IGNITE-28940:
---
Ignite Flags: Release Notes Required (was: Docs Required,Release Notes
Required)
> 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, ise, wire-format
> Fix For: 2.19
>
> Time Spent: 12h 20m
> Remaining Estimate: 0h
>
> *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
[jira] [Updated] (IGNITE-28940) Choose the marshaller by transport, not by message class
[
https://issues.apache.org/jira/browse/IGNITE-28940?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Vladimir Steshin updated IGNITE-28940:
--
Labels: IEP-132 compatibility ise wire-format (was: IEP-132 compatibility
wire-format)
> 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, ise, wire-format
> Fix For: 2.19
>
> Time Spent: 12h 20m
> Remaining Estimate: 0h
>
> *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.
[jira] [Updated] (IGNITE-28940) Choose the marshaller by transport, not by message class
[
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
[jira] [Updated] (IGNITE-28940) Choose the marshaller by transport, not by message class
[
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 (was: 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
> 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
[jira] [Updated] (IGNITE-28940) Choose the marshaller by transport, not by message class
[
https://issues.apache.org/jira/browse/IGNITE-28940?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Anton Vinogradov updated IGNITE-28940:
--
Description:
*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.
was:
*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
[jira] [Updated] (IGNITE-28940) Choose the marshaller by transport, not by message class
[
https://issues.apache.org/jira/browse/IGNITE-28940?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Anton Vinogradov updated IGNITE-28940:
--
Description:
*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".
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.
was:
*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 may wait for a cluster-wide class registration, and this is not allowed
on the discovery thread. So the correct marshaller depends on the transport.
The call sites are already separated:
* communication: {{GridIoManager}}, {{GridCacheIoManager}},
{{IgniteTxManager}}, {{GridDhtPartitionsExchangeFuture}},
{{CacheContinuousQueryHandler}}, {{DataStreamProcessor}}, calcite
{{MessageServiceImpl}};
* discovery: {{TcpDiscoveryIoSession}} and ZooKeeper {{DiscoveryMessageParser}}.
Hand-written code already works this way:
{{MarshallableMessage#marshal(Marshaller)}} takes the marshaller as a
parameter. Only generated code keeps it in a field.
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
[jira] [Updated] (IGNITE-28940) Choose the marshaller by transport, not by message class
[
https://issues.apache.org/jira/browse/IGNITE-28940?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Anton Vinogradov updated IGNITE-28940:
--
Description:
*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 may wait for a cluster-wide class registration, and this is not allowed
on the discovery thread. So the correct marshaller depends on the transport.
The call sites are already separated:
* communication: {{GridIoManager}}, {{GridCacheIoManager}},
{{IgniteTxManager}}, {{GridDhtPartitionsExchangeFuture}},
{{CacheContinuousQueryHandler}}, {{DataStreamProcessor}}, calcite
{{MessageServiceImpl}};
* discovery: {{TcpDiscoveryIoSession}} and ZooKeeper {{DiscoveryMessageParser}}.
Hand-written code already works this way:
{{MarshallableMessage#marshal(Marshaller)}} takes the marshaller as a
parameter. Only generated code keeps it in a field.
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.
Four classes change their wire format on purpose:
* {{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}}.
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.
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.
was:
h3. Goal
Take the choice away from the message class and derive it from the place where
marshalling actually happens.
h3. Why
Whether binary may be used is decided by the call site, not by the message
type: binary blocks on a cluster-wide class registration, which is impossible
on the discovery thread. The call sites are already split by transport:
* 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 is already transport agnostic:
{{MarshallableMessage#marshal(Marshaller)}} takes the marshaller as a
parameter. Only the generated half keeps it in a field, which is exactly what
pins it to the class.
h3. How
* add a {{Marshaller}} parameter to {{MessageMarshaller}} and
{{MessageMarshalling}}; the generator stops storing it and passes it down into
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}}, {{TransactionsHashRecord}} - jdk
to binary. Safe for {{ErrorMessage}}: {{java.lang.Throwable}} declares
{{writeObject}}/{{readObject}}, so every exception matches
{{isCustomJavaSerialization}} and binary delegates to {{OptimizedMarshaller}} -
the format changes, the semantics do not.
* {{BinaryMetadataVersionInfo}} - binary to jdk, aligning it with
{{MetadataUpdateProposedMessage}}; moot if the blob-removal subtask lands first.
Note: {{MessageMarshaller}} lives in the public
{{org.apache.ignite.plugin.extensions.communication}} package; outside the
generator it is implemented only by three tests.
h3. Expected result
Zero places to annotate: the marshaller is a property of the send, not of the
type. The inherited-field trap and the nested-message override disappear by
construction, because one marshaller serves the whole message tree of a single
send.
h3. How to verify
Run All; targeted runs for continuous queries, idle_verify,
{{DistributedProcess}} and calcite; plus a test that
