anton-vinogradov opened a new pull request, #13462:
URL: https://github.com/apache/ignite/pull/13462
The marshaller used for a message's `@Marshalled` fields was a property of
the
message class: `@UseBinaryMarshaller` decided it, and the factory bound the
chosen marshaller into the generated companion at registration time.
### Why that is the wrong place
Binary cannot be used where marshalling cannot afford a cluster-wide class
registration, and that is a property of the call site. The registration is
`MarshallerContextImpl#registerClassName` -> `proposeMapping` -> `fut.get()`,
which waits for discovery, so it must never happen on a discovery thread.
Ignite
classes usually skip it thanks to `META-INF/classnames.properties`, but that
list
is not closed under nesting - a plain `CacheConfiguration` needs types that
are
not on it.
The call sites are already separated by transport, and the hand-written half
of
the API works this way: `MarshallableMessage#marshal(Marshaller)` takes the
marshaller as a parameter. Only generated code kept it in a field.
### Change
* `Marshaller` is now a parameter of `MessageMarshaller` and
`MessageMarshalling`;
the generator stops storing it and passes it down to nested messages.
* Communication call sites pass `ctx.marshaller()`, discovery call sites pass
`marshallerContext().jdkMarshaller()`.
* `@UseBinaryMarshaller` (60 classes), `AbstractMessageFactoryProvider#init`
and
both marshaller fields are gone, as is `initProvider` in `IgniteKernal`.
### Wire format
The format is now a function of the transport, not of the class, so a class
that
travels both transports is marshalled differently on each. Deliberate
changes:
| class | change |
|---|---|
| `ErrorMessage`, `PartitionHashRecord`, `TransactionsHashRecord` | jdk ->
binary, on the communication leg |
| `BinaryMetadataVersionInfo` | binary -> jdk, on the discovery leg |
| `StoredCacheData` | jdk -> binary, on the communication leg (snapshot
restore) |
`StoredCacheData` is not in the ticket's list: it was found while reviewing
this
change. It carries `CacheConfiguration` and `QueryEntity` blobs and reaches
communication through `SnapshotRestoreOperationResponse` ->
`SingleNodeMessage`,
so the transport rule moves it to binary. Registration is allowed there, and
binary is the smaller of the two for a `CacheConfiguration`.
`BinaryMetadataVersionInfo` needed one more fix: the same long-lived instance
from the local metadata cache travels discovery in the data bag and
communication
in `MetadataResponseMessage`, and a marshalled instance keeps its serialized
form. `MetadataResponseMessage#metadataVersionInfo` now stores a copy, so
bytes
of one transport cannot leak into the other.
### Verified
`MessageProcessorTest` (codegen goldens regenerated),
`IgniteCoreMessagesSerializationTest`,
`DirectMarshallingMessagesTest`, `MessageMarshalOnceTest`,
`CompressedMessageTest`,
`GridIoManagerOrderedUnmarshalFailureTest`,
`DiscoveryUnmarshalVulnerabilityTest`,
`QueryEntityMessageSerializationTest`,
`LazyServiceConfigurationMessageSerializationTest`,
`SecurityBasicPermissionSetSerializationTest`,
`TxDeadlockDetectionMessageMarshallingTest`,
`GridCacheQueryResponseUnmarshalTest`,
`BinaryMetadataRegistrationInsideEntryProcessorTest`,
`GridCacheAtomicFullApiSelfTest`, the continuous-query and p2p suites, and
the
calcite `PlanExecutionTest` / `ContinuousExecutionTest` /
`CalciteCommunicationMessageSerializationTest`.
Full build of all modules and the strict checkstyle profile are clean.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
