[
https://issues.apache.org/jira/browse/IGNITE-28941?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Anton Vinogradov updated IGNITE-28941:
--------------------------------------
Description:
*No wire format change.*
h3. Goal
Check in CI that nothing on the discovery path needs a cluster-wide class
registration, and produce the map of that path as a by-product.
h3. Why
The rule is kept by convention only, and the trap is not "someone used binary"
but "someone added a type that is not pre-registered".
* {{BinaryMetadataVersionInfo}} is marshalled with binary inside the jdk
discovery data bag {{CacheBinaryDataBagItem}}, and works only because
{{BinaryMetadata}}, {{BinarySchema}} and {{BinaryFieldMetadata}} are listed in
{{META-INF/classnames.properties}}.
* That list is not closed under nesting: a plain {{CacheConfiguration}} pulls 9
types, and two of them are missing.
A new type outside the list would block the discovery thread in {{fut.get()}}.
Note for the implementer: this map cannot be built by grepping sources -
message trees are assembled at run time, and a static approximation gives false
positives on almost every class. Walk the registered message types and their
{{@Order}} fields instead.
h3. How
* walk the message tree reachable from discovery messages (message factory and
{{@Order}} fields) and collect the types that would be marshalled there;
* assert that each of them either avoids the schema-aware marshaller, or is
pre-accepted, so that no cluster-wide registration can be triggered;
* optional: add an assert in {{MarshallerContextImpl#registerClassName}} that
it is never called from the discovery worker thread.
h3. Expected result
The rule is checked by CI, so neither a new message nor a new nested type can
bring the deadlock back. The collected map also tells which message types
travel over both transports, which is useful for IGNITE-28940.
h3. How to verify
The test must fail on a message deliberately marked wrong, and on a type
deliberately removed from the pre-accepted list. It must pass on master once
the other subtasks are in.
was:
*No wire format change.*
h3. Goal
Check in CI that nothing on the discovery path needs a cluster-wide class
registration.
h3. Why
The rule is kept by convention only, and the convention is easy to break
without noticing, because the trap is not "someone used binary" but "someone
added a type that is not pre-registered".
Two facts show how thin the ice is:
* {{BinaryMetadataVersionInfo}} is marshalled with binary inside the jdk
discovery data bag {{CacheBinaryDataBagItem}}, and it works only because
{{BinaryMetadata}}, {{BinarySchema}} and {{BinaryFieldMetadata}} are listed in
{{META-INF/classnames.properties}};
* that list is not closed under nesting. A plain {{CacheConfiguration}} pulls 9
types, and {{javax.cache.configuration.FactoryBuilder$SingletonFactory}} and
{{javax.cache.expiry.EternalExpiryPolicy}} are not in it.
A new type outside the list would block the discovery thread in {{fut.get()}}.
h3. How
* walk the message tree reachable from discovery messages (message factory and
{{@Order}} fields) and collect the types that would be marshalled there;
* assert that each of them either avoids the schema-aware marshaller, or is
pre-accepted, so that no cluster-wide registration can be triggered;
* optional: add an assert in {{MarshallerContextImpl#registerClassName}} that
it is never called from the discovery worker thread.
h3. Expected result
The rule is checked by CI, so neither a new message nor a new nested type can
bring the deadlock back.
h3. How to verify
The test must fail on a message deliberately marked wrong, and on a type
deliberately removed from the pre-accepted list. It must pass on master once
the other subtasks are in.
> Add a test: discovery messages must not need cluster class registration
> -----------------------------------------------------------------------
>
> Key: IGNITE-28941
> URL: https://issues.apache.org/jira/browse/IGNITE-28941
> Project: Ignite
> Issue Type: Sub-task
> Components: messaging
> Reporter: Anton Vinogradov
> Assignee: Anton Vinogradov
> Priority: Major
> Fix For: 2.19
>
>
> *No wire format change.*
> h3. Goal
> Check in CI that nothing on the discovery path needs a cluster-wide class
> registration, and produce the map of that path as a by-product.
> h3. Why
> The rule is kept by convention only, and the trap is not "someone used
> binary" but "someone added a type that is not pre-registered".
> * {{BinaryMetadataVersionInfo}} is marshalled with binary inside the jdk
> discovery data bag {{CacheBinaryDataBagItem}}, and works only because
> {{BinaryMetadata}}, {{BinarySchema}} and {{BinaryFieldMetadata}} are listed
> in {{META-INF/classnames.properties}}.
> * That list is not closed under nesting: a plain {{CacheConfiguration}} pulls
> 9 types, and two of them are missing.
> A new type outside the list would block the discovery thread in {{fut.get()}}.
> Note for the implementer: this map cannot be built by grepping sources -
> message trees are assembled at run time, and a static approximation gives
> false positives on almost every class. Walk the registered message types and
> their {{@Order}} fields instead.
> h3. How
> * walk the message tree reachable from discovery messages (message factory
> and {{@Order}} fields) and collect the types that would be marshalled there;
> * assert that each of them either avoids the schema-aware marshaller, or is
> pre-accepted, so that no cluster-wide registration can be triggered;
> * optional: add an assert in {{MarshallerContextImpl#registerClassName}} that
> it is never called from the discovery worker thread.
> h3. Expected result
> The rule is checked by CI, so neither a new message nor a new nested type can
> bring the deadlock back. The collected map also tells which message types
> travel over both transports, which is useful for IGNITE-28940.
> h3. How to verify
> The test must fail on a message deliberately marked wrong, and on a type
> deliberately removed from the pre-accepted list. It must pass on master once
> the other subtasks are in.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
