[
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.
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.
was:
*No wire format change.*
h3. Goal
Check the rule "do not use the binary marshaller on the discovery path" in CI.
h3. Why
Today this rule is only a convention. In one place it works by luck:
{{BinaryMetadataVersionInfo}} uses binary inside the jdk discovery data bag. It
works because {{BinaryMetadata}}, {{BinarySchema}} and {{BinaryFieldMetadata}}
are listed in {{META-INF/classnames.properties}} (2276 names), so
{{registerClassName}} returns at once. A new class outside this file would
block the discovery thread in {{fut.get()}}.
h3. How
* walk the message tree that starts from discovery messages (message factory
and {{@Order}} fields), and check that no field uses the binary marshaller;
* optional: add an assert in {{MarshallerContextImpl#registerClassName}} that
it is not called from the discovery worker thread.
h3. Expected result
The rule is checked by CI, so a new message cannot bring the deadlock back.
h3. How to verify
The test must fail on a message marked wrong on purpose, and pass on master
after the other subtasks.
> 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.
> 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.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
