[
https://issues.apache.org/jira/browse/IGNITE-28941?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Anton Vinogradov resolved IGNITE-28941.
---------------------------------------
Resolution: Done
Closing: IGNITE-28940 closes this by construction, so the test this task asked
for would guard a rule that the API already keeps.
Every point that marshals a discovery message now goes through one facade,
which speaks jdk:
{noformat}
TcpDiscoveryIoSession:203 DiscoveryMarshalling.unmarshal(msg, kctx)
TcpDiscoveryIoSession:249 DiscoveryMarshalling.marshal(m, kctx, null)
zk/DiscoveryMessageParser:97 DiscoveryMarshalling.marshal(m, kctx, null)
zk/DiscoveryMessageParser:144 DiscoveryMarshalling.unmarshal(msg, kctx)
{noformat}
The direct uses of a marshaller left in the discovery packages are
{{spi.marshaller()}} for node attributes and credentials, and that one is the
jdk marshaller as well (TcpDiscoverySpi:606).
So both traps from the description are gone: writing binary on the discovery
path now takes an explicitly wrong line ({{CommunicationMarshalling}} inside
discovery code), and the class registration it feared belongs to binary, which
no longer runs there. The example in the description has changed too -
{{BinaryMetadataVersionInfo}} carries {{@JdkMarshalled}} now.
One thing from here is still worth having: the map of the discovery path, which
this task listed as a by-product. That is documentation rather than a test, so
it deserves its own ticket if someone wants it.
> 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
> Labels: IEP-132
> 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)
