[ 
https://issues.apache.org/jira/browse/IGNITE-28939?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18101420#comment-18101420
 ] 

Anton Vinogradov commented on IGNITE-28939:
-------------------------------------------

Closing this: what it was worth doing is covered by IGNITE-28940, and most of 
what it promised turned out to be impossible.

*Checked before closing:*

# {{BinaryMetadata}} cannot become a {{Message}} without moving modules around. 
Classes with {{@Order}} exist only in core (500), indexing (27), calcite (12) 
and zookeeper (5) - none in {{modules/binary}}. And it is not a matter of 
adding the annotation processor: {{Message}} and {{MessageSerializer}} live in 
{{ignite-nio}}, while {{ignite-nio}} already depends on {{ignite-binary-api}}. 
Making {{binary-api}} depend on {{nio}} closes a dependency cycle. It would 
take extracting the message API into a lower-level module first.
# {{BaselineTopology}} and {{GridJobSiblingImpl}} hold plain {{Object}} fields 
- {{Map<Object, Map<String, Object>> nodeMap}}, {{Map<Object, Short>}}, 
{{Object taskTopic}}, {{Object jobTopic}}. The direct protocol has no item type 
for an arbitrary object (see {{MessageCollectionItemType}}), so the blob would 
only move one level down.
# What is actually convertible is two fields - {{StoredCacheData#qryEntities}} 
and {{DynamicCacheChangeRequest#schema}} - because {{QueryEntityMessage}} 
already exists. That is "24 blob fields -> 22", not "-> 19", and it has nothing 
to do with marshaller selection.

*Why IGNITE-28940 covers the point of this ticket:* the reason these fields 
were listed here was the inconsistency - the same {{BinaryMetadata}} marshalled 
with binary in {{BinaryMetadataVersionInfo}} and with jdk in 
{{MetadataUpdateProposedMessage}}. Once the marshaller comes from the call 
site, both travel over discovery and get the same one, so the mismatch 
disappears without touching the fields.

The two convertible fields are left out on purpose: it is a small independent 
cleanup, not part of this umbrella.

> Turn blob fields of internal types into normal message fields
> -------------------------------------------------------------
>
>                 Key: IGNITE-28939
>                 URL: https://issues.apache.org/jira/browse/IGNITE-28939
>             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
> Remove the question "which marshaller" for fields that hold internal classes.
> h3. Why
> Some fields are sent as blobs only because an internal class was never made a 
> {{Message}}. Binary gives no schema for them anyway: 
> {{BinaryUtils#isCustomJavaSerialization}} checks the whole class hierarchy, 
> so {{Externalizable}} classes fall back to {{OptimizedMarshaller}} inside 
> binary. One of these fields is also a mismatch: {{BinaryMetadata}} uses 
> binary in {{BinaryMetadataVersionInfo}} and jdk in 
> {{MetadataUpdateProposedMessage}}.
> h3. How
> Convert to normal message fields:
> * {{BinaryMetadataVersionInfo#metadata}} and 
> {{MetadataUpdateProposedMessage#metadata}} - {{BinaryMetadata}} has simple 
> fields: type id, type name, field map, schemas, flags;
> * {{ChangeGlobalStateMessage#baselineTopology}} - {{BaselineTopology}};
> * {{GridJobSiblingsResponse#siblings}} - the implementation is the internal 
> {{GridJobSiblingImpl}};
> * {{StoredCacheData#qryEntities}} and {{DynamicCacheChangeRequest#schema}} - 
> {{QueryEntityMessage}} already exists.
> Fields that really hold user classes stay blobs: node attributes, service 
> affinity key, query entity default values, entry processors, invoke 
> arguments, query filters, task results, message topic, cache configuration, 
> plugin data and {{Throwable}}.
> h3. On-disk compatibility
> Three of these types also live on disk, so the conversion must not change how 
> they are stored:
> * {{BinaryMetadata}} - written by {{BinaryMetadataFileStore#writeMetadata}}; 
> the format comes from its
> {{Externalizable}} methods;
> * {{BaselineTopology}} - kept in the metastore under 
> {{METASTORE_CURR_BLT_KEY}}; {{MetaStorage}} marshals values with jdk;
> * {{QueryEntity}} inside {{StoredCacheData}} - {{cache_data.dat}} files and 
> snapshots, jdk again.
> Rules that keep the stored format byte-identical:
> # keep the explicit {{serialVersionUID}}. All of these classes declare one, 
> so adding the {{Message}} interface does
> not change the computed id. Without an explicit id, adding an interface 
> changes it and old files stop loading.
> # make new wire companion fields {{transient}}, the way {{ccfgBytes}} and 
> {{qryEntitiesBytes}} are done in
> {{StoredCacheData}}. Non-transient fields are what the disk format is made of.
> # do not add or remove non-transient fields, and do not touch 
> {{writeExternal}}/{{readExternal}}. For
> {{BinaryMetadata}} the {{Externalizable}} methods *are* the on-disk format: 
> adding {{Message}} on top is fine,
> dropping {{Externalizable}} is not.
> Precedent: {{StoredCacheData}}, {{CacheConfigurationEnrichment}} and 
> {{SnapshotMetadata}} are already both
> {{Serializable}} and {{Message}}, and that coexists with the disk today.
> Verification for this ticket: read a {{cache_data.dat}} and a binary metadata 
> file written by the previous build,
> and restore a snapshot taken by it.
> h3. Expected result
> Blob fields: 24 now, about 19 after. The {{BinaryMetadata}} mismatch is gone. 
> These fields no longer depend on a marshaller, and usually become smaller on 
> the wire.
> h3. How to verify
> Binary metadata tests, node join tests, cluster state change tests, dynamic 
> cache start with query entities, job siblings tests. Compare message size 
> before and after.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

Reply via email to