He-Pin opened a new issue, #3694:
URL: https://github.com/apache/pekko/issues/3694
## Summary
`AGENTS.md:41` states "Wire protocol changes must consider rolling upgrade
compatibility", but nothing in the build verifies it. Every existing
serialization test is a same-version round trip: serialize and deserialize
inside one JVM, then compare object equality. There is no test anywhere
asserting that bytes produced by one release can be decoded by another, so an
accidental wire change is only caught by a production rolling upgrade.
## Current coverage
`actor-testkit-typed/src/main/scala/org/apache/pekko/actor/testkit/typed/scaladsl/SerializationTestKit.scala:38,50,63`:
```scala
def verifySerialization[M](obj: M): M = ...
def verifySerialization[M](obj: M, assertEquality: Boolean): M = { ... }
private def roundtrip[M](obj: M): M = { ... }
```
All 22 `*Serialization*Spec.scala` files under `src/test` follow this
pattern. A round trip through the *same* serializer instance cannot detect an
encoding change, a renumbered field, a changed manifest string or a reordered
collection — it only proves the current code is self-consistent.
The repo contains no committed byte fixtures of any kind:
```
find . \( -name '*.approved.*' -o -name '*.verified.*' -o -name '*.golden'
\) -not -path './target/*' | wc -l
0
```
The two existing rolling-upgrade tests cover different concerns —
`cluster/src/test/scala/org/apache/pekko/cluster/JoinConfigCompatCheckerRollingUpdateSpec.scala`
checks *configuration* compatibility at join time, and
`cluster-sharding/src/multi-jvm/scala/org/apache/pekko/cluster/sharding/RollingUpdateShardAllocationSpec.scala`
checks *allocation behaviour* during a rolling update. Neither touches encoded
bytes.
## What is at stake
The internal protocol is carried by 21 `.proto` files under
`*/src/main/protobuf/`:
```
remote/src/main/protobuf/{ArteryControlFormats,ContainerFormats,SystemMessageFormats,WireFormats}.proto
cluster/src/main/protobuf/ClusterMessages.proto
cluster-typed/src/main/protobuf/{ClusterMessages,ReliableDelivery}.proto
cluster-sharding/src/main/protobuf/ClusterShardingMessages.proto
cluster-sharding-typed/src/main/protobuf/ShardingMessages.proto
cluster-tools/src/main/protobuf/{ClusterClientMessages,DistributedPubSubMessages}.proto
cluster-metrics/src/main/protobuf/ClusterMetricsMessages.proto
distributed-data/src/main/protobuf/{ReplicatedDataMessages,ReplicatorMessages}.proto
persistence/src/main/protobuf/MessageFormats.proto
persistence-query/src/main/protobuf/QueryMessages.proto
persistence-typed/src/main/protobuf/ReplicatedEventSourcing.proto
stream/src/main/protobuf/StreamRefMessages.proto
multi-node-testkit/src/main/protobuf/TestConductorProtocol.proto
```
plus roughly 21 hand-written `SerializerWithStringManifest` implementations
that carry manifests and serializer identifiers not described by any schema —
among them `remote/.../serialization/ArteryMessageSerializer.scala`,
`MiscMessageSerializer.scala`, `DaemonMsgCreateSerializer.scala`,
`cluster/.../protobuf/ClusterMessageSerializer.scala`,
`cluster-sharding/.../protobuf/ClusterShardingMessageSerializer.scala`,
`distributed-data/.../protobuf/{ReplicatedDataSerializer,ReplicatorMessageSerializer}.scala`,
`actor-typed/.../internal/MiscMessageSerializer.scala`,
`stream/.../serialization/StreamRefSerializer.scala`.
For the manifest-based ones there is no schema at all: the manifest strings
and the `identifier` values *are* the contract, and they live in code and in
`serialization-identifiers` entries spread across many `reference.conf` files.
Nothing fails a build if one of them changes.
## Proposal
Commit golden byte fixtures for the internal protocol messages and assert
two things in CI:
1. **Encode stability** — the current code produces byte-identical output
for a fixed input.
2. **Decode compatibility** — the current code can decode the committed
bytes.
(2) is the one that actually protects a rolling upgrade, since it is the
direction a mixed-version cluster exercises when an old node talks to a new one.
A readable fixture format matters more than a compact one, because the whole
point is that a diff is reviewable in a PR. An annotated hex dump with a small
header works well:
```
# manifest: <manifest string>
# serializer-identifier: <id>
# bytes: <n>
00000000 0a 05 70 65 6b 6b 6f 12 0b 08 c0 a8 01 0a 10 d9
|..pekko.........|
...
```
so a reviewer can see immediately that field 2 changed from a `fixed64` uid
to something else, rather than facing an opaque binary diff.
Details worth settling up front:
- **Hash-ordered collections must be snapshotted with a single element.**
`Gossip.members`, `ORSet` dots and similar are emitted in an order that depends
on iteration of a hash structure, which is not part of the cross-version
contract. A multi-element fixture would fail spuriously.
- **Pin line endings** for the fixture directory in `.gitattributes`, or the
fixtures will churn across platforms.
- **Keep the fixtures under the module that owns the message**, next to its
existing test sources, so they are discovered by the same review.
- **Cover the manifest and the serializer identifier in the header**, not
just the payload — those are the parts with no schema behind them and are the
easiest to change by accident.
A sensible first slice, in decreasing order of traffic on the wire: Artery
control messages, system messages, cluster gossip, shard coordinator messages.
Those four cover most of what a mixed-version cluster actually exchanges.
## Scope
This is test-only. No `src/main` change, no public API change, no MiMa
impact.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
