Repository navigation
ReliableDelivery: MessagePack V2 serializer fork (id 36 -> 76, additive, writes unchanged) - #8409
Open
Aaronontheweb wants to merge 7 commits into
Open
Aaronontheweb wants to merge 7 commits into
Aaronontheweb wants to merge 7 commits into
Conversation
…d MessagePack (id 36 -> 76) First subsystem migration under the internal-serializers-messagepack-v2 plan (design.md Decisions 1/4/5/8/10): additive, read-side-only registration. * New Akka.Cluster.Serialization.ReliableDeliveryMessagePackSerializer (id 76 = legacy 36 + 40, reserved 40-79 block) translating the Akka.Delivery domain messages to non-generic [AkkaSerializable] wire mirrors encoded by the source-generated ReliableDeliveryMessagePackCodec; manifest tokens "a".."i" reused verbatim from the protobuf ReliableDeliverySerializer. * User payloads ride through [AkkaEnvelopePayload] (serializerId+manifest+bytes, the WrappedPayloadSupport analog); chunked payloads ride through as raw bytes. * Write path is reflection-free: the internal ISequencedMessage / IMessageSent / IState / IRegisterConsumer serialization interfaces in core Akka now expose non-generic member access; the read path caches one generic domain factory per payload type instead of per-call MakeGenericMethod. * Dual registration in Cluster.conf: the legacy protobuf serializer stays fully registered and bound (writes unchanged); the MessagePack serializer is registered additively so every v1.6 node can READ both formats by id. The write-side flip attaches later via akka.actor.serialization.v2.write-bindings.reliable-delivery (feature/serialization-v2-write-flag). * Parity specs cover every legacy message type + manifest: round-trip through the new serializer, legacy no-regression, manifest parity, dual-id resolution from one ActorSystem, and writes-stay-protobuf; plus a protobuf-vs-MessagePack A/B benchmark (CPU, allocations, and per-message payload size).
Aaronontheweb
added
akka-delivery
akkadotnet#8518; object-typed payloads are the boundary akkadotnet#8518 (6775212) removed AkkaEnvelopePayloadAttribute from Akka.Serialization.V2 - a property whose static type is object (or object?) is now the envelope-payload boundary by itself, with no attribute required. This left two sites in ReliableDeliveryMessagePackSerializer.cs referencing the deleted attribute: SequencedMessageWire.Payload and MessageSentWire.Payload, both already typed object?. Drop the attribute, keep the object? typing, and update the two doc comments that described the old attribute-based mechanism. No wire-format or generated-code change; only the source-gen input annotation moved. Verified: Akka.Cluster and Akka.Cluster.Tests build clean under -warnaserror with no new AKKASGxxx diagnostics; ReliableDeliveryMessagePackSerializerSpecs (82 tests) and the full Akka.Cluster.Tests Serialization filter (154 tests, includes the id 36 -> 76 classic-serializer fork) pass; Akka.Benchmarks builds clean.
to11mtm
approved these changes
Sep 20, 2026
to11mtm
left a comment
to11mtm
left a comment
Member
There was a problem hiding this comment.
Looks good to me, added some comments for potential polishing before commit.
| private const string DurableQueueStateManifest = "h"; | ||
| private const string DurableQueueCleanupManifest = "i"; | ||
|
|
||
| private static readonly ConcurrentDictionary<Type, IDomainFactory> DomainFactories = new(); |
Member
There was a problem hiding this comment.
Not sure if Juice is worth the squeeze or not, but I saw this recently and at least wanted to mention https://x.com/neuecc/status/2097265061282943252 .
| { | ||
| var wire = ToWire(obj); | ||
| var sizeHint = _codec.SizeHint(wire); | ||
| var writer = sizeHint > 0 ? new ArrayBufferWriter<byte>(sizeHint) : new ArrayBufferWriter<byte>(); |
Member
There was a problem hiding this comment.
Any specific reason to not bother with pooling a bufferwriter here? (Might be some good ones but sanity checking...)
Comment on lines
+404
to
+405
| [property: AkkaField(3)] bool SupportResend, | ||
| [property: AkkaField(4)] bool ViaTimeout) : IReliableDeliveryWireMessage; |
Member
There was a problem hiding this comment.
Per your comments on PR itself, IDK whether the performance difference is a concern or not for this, but for perf (maybe a little?) size here I wonder if it would be better to just have a Byte for the bool flags on the wire type and handle the ser/deser that way...
(Although, maybe that's overkill regardless?)
| return null; | ||
|
|
||
| return new ChunkedMessageWire( | ||
| chunkedMessage.SerializedMessage.ToArray(), |
Member
There was a problem hiding this comment.
Is the ToArray here really necessary? Messagepack should be able to handle that for round-trip unless there's some other shenanigans I'm somehow forgetting...
This was referenced Sep 24, 2026
Open
Experimental: integrating the V2 serialization POC into Akka.Remote (research / not for merge)
#8203
Closed
Aaronontheweb
added a commit
that referenced
this pull request
Oct 2, 2026
…2 serialization decisions (#8720) One global opt-in V2 switch (off by default), V2 serializers as read-only rows in the C# module tables, no .conf rows, native same-bytes route for Primitive/PersistenceMessage/PersistenceSnapshot, Sharding as a unit, Remote core in scope, PR-body breaking-change sections. Unticks the tasks that claim PR #8409 merged.
This was referenced Oct 2, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
ReliableDelivery: MessagePack V2 serializer fork (id 36 → 76)
First subsystem migration of the
internal-serializers-messagepack-v2OpenSpec change (#8402, amended by #8408): a source-generated MessagePack fork ofReliableDeliverySerializer, registered additively at id 76 (legacy + 40). No binding changes — writes stay on protobuf (id 36). Every v1.6 node gains the ability to read the MessagePack format; nothing writes it until the #8403 flag re-points theIDeliverySerializablebinding (theCluster.confTODO documents the exactwrite-bindings.reliable-deliveryattachment).Architecture (per design.md Decisions 5/8/10)
SerializerV2(ReliableDeliveryMessagePackSerializer, internal) translating domain types ↔ internal[AkkaSerializable]wire mirrors; byte work is delegated to the source-generatedReliableDeliveryMessagePackCodec(never itself registered). Two classes because the domain messages are generic core-Akka types that can't referenceAkka.Serialization.V2.a–ireused verbatim from the protobuf serializer — intra-serializer dispatch mirrors it 1:1.[AkkaEnvelopePayload]— the (serializerId, manifest, bytes) triple, the direct analog ofWrappedPayloadSupport; verified with both V1 and V2 inner serializers. Chunked payloads ride as raw bytes.ISequencedMessage/IMessageSent/IState/IRegisterConsumerinterfaces in core Akka gained non-generic accessors — internal only, API approval 18/18 unchanged); read path caches one generic factory per payload type vs. the legacy per-callMakeGenericMethod.Parity
All 9 manifests round-trip through the new serializer AND still round-trip through the legacy serializer (no regression), with edge shapes covered (chunked ×2, qualifiers, POCO envelopes, empty/populated
State/Cleanup): 82/82 new specs + 16/16 legacy specs. Also pinned: manifest parity, both ids resolvable from oneActorSystem(the dual-registration read contract),FindSerializerForTypestill resolves protobuf, andNobody-ref resolution parity (neither serializer restoresNobody.Instanceidentity — legacy-equivalent).Benchmark A/B (i9-9900K, .NET 10, ShortRun — rerun a full job before publishing gate numbers)
SequencedMessage(128-char payload)SequencedMessage(1 KiB chunk)StateRequest(tiny control)Verdict: deserialize clears the "very noticeable improvement" bar decisively (1.2–3.3× faster, lower allocations, on the receive half of every delivery); serialize is 15–33% faster on real messages; payload sizes are within 1% except the 7-byte
Request(+3 bytes absolute). Per the amended Decision 9 (#8408) the subsystem migrates as a unit — theRequestserialize regression and write-side allocation overhead are optimization targets, with root causes already identified: theToBinarybyte[]-bridge (Artery'sIBufferWriterpath skips two of its three arrays and is where flag-era traffic actually flows),SizeHintdegrading toUnknownSizewhen the envelope's inner serializer is V1, one avoidable buffer-doubling on chunked writes, and a fixed ~40–60 ns two-hop dispatch tax that only matters at 7-byte scale.Verification
ReliableDeliverySerializerSpecs16/16, core Delivery specs 57/57, API approval 18/18 (no baseline change)dotnet formatclean on all new files