Repository navigation
Serialization: module tables carry aliases and bindings (#8676, part 2) - #8698
Conversation
, part 2) Replace ModuleSerializer with SerializerRegistration(Alias, Type, Create, Bindings) in all 8 first-party module tables (Remote, Streams, Cluster, Cluster.Tools, Cluster.Sharding, DistributedData, Cluster.Metrics, Persistence), filled in exactly from each module's reference.conf. The tables now hold everything HOCON holds for these modules; HOCON still drives Serialization - no behavior change. BoundTypes is gone; LoadedModule derives its bound-type index from the union of every registration's Bindings, with identical lookup behavior. Extend ModuleSerializerSpecs.AssertTableMatchesConfig with an overload that checks alias-to-type and binding-to-alias parity in both directions, and update all 8 module specs to use it.
Akka.Serialization.V2 already exposes a public SerializerRegistration. Any assembly that sees Akka's internals and imports both namespaces hit CS0104, which broke the Akka.Benchmarks build.
Aaronontheweb
left a comment
There was a problem hiding this comment.
Author walkthrough: each comment below explains why that change is needed.
| /// null skips the alias (a safety net, not a feature). | ||
| /// </summary> | ||
| internal sealed record ModuleSerializer(Type Type, Func<ExtendedActorSystem, Config, Serializer> Create); | ||
| internal sealed record BuiltInSerializer( |
There was a problem hiding this comment.
Each row now carries what a HOCON serializer row and its binding rows say: the alias, the type, a factory and the bound types. PR 3 needs all four to register built-in serializers from code with no HOCON. The old ModuleSerializer(Type, Create) couldn't do that, because it had no alias to register under and no bindings to apply.
I first called this SerializerRegistration, but Akka.Serialization.V2 already has a public type with that name. Any assembly that sees Akka's internals and imports both namespaces hit CS0104, and Akka.Benchmarks failed to build. Hence BuiltInSerializer.
| public abstract IReadOnlyList<ModuleSerializer> Serializers { get; } | ||
|
|
||
| public abstract IReadOnlyList<Type> BoundTypes { get; } | ||
| public abstract IReadOnlyList<BuiltInSerializer> Serializers { get; } |
There was a problem hiding this comment.
BoundTypes is gone because a flat list of types can't say which serializer each type binds to. PR 3 must know that to replace the binding rows. With the bindings on each row there's one source, so the two lists can't drift apart.
This also makes the V2 cut-over a one-line move: moving a type from a V1 row's Bindings to a V2 row's.
| _serializers[KeyOf(entry.Type)] = (entry, entry.Type.Assembly.GetName().Name, IsAkka(entry.Type)); | ||
| foreach (var type in module.BoundTypes) | ||
| _boundTypes[KeyOf(type)] = (type, type.Assembly.GetName().Name, IsAkka(type)); | ||
| foreach (var type in entry.Bindings) |
There was a problem hiding this comment.
Nothing changes at runtime. _boundTypes is the same dictionary with the same KeyOf and Accepts rules, now filled from each row's Bindings instead of a separate list. HOCON still decides which serializer a type binds to. This index only turns the type name in a binding row into a Type without reflection, as before.
| try | ||
| { | ||
| // reads both lists here, so a member missing from either also lands in the catch | ||
| // reads the list here, so a missing member also lands in the catch |
There was a problem hiding this comment.
There's now one list instead of two, so I updated the comment. The point stands: reading Serializers inside the try means a module built against an older Akka that lacks the member throws MissingMethodException here. The version-skew catch handles that, and the module falls back to HOCON plus reflection. SkewedModule and StaticSkewedModule in ModuleSerializersSpec cover it.
| typeof(string), | ||
| typeof(int), | ||
| typeof(long), | ||
| new BuiltInSerializer("akka-containers", typeof(MessageContainerSerializer), (system, _) => new MessageContainerSerializer(system), |
There was a problem hiding this comment.
Each row mirrors one alias in Remote.conf, with that alias's bindings under it. The bindings are the same as the old flat list, regrouped. The parity test in ModuleSerializerSpecs checks both directions, so a type added or dropped by mistake would fail it.
PoisonPill is bound twice in Remote.conf (lines 40 and 43, both to akka-misc). HOCON keeps one, and so does this table. PR 3 removes the duplicate line.
| typeof(IDeliverySerializable), | ||
| new BuiltInSerializer("akka-cluster", typeof(ClusterMessageSerializer), (system, _) => new ClusterMessageSerializer(system), | ||
| new[] { typeof(IClusterMessage), typeof(ClusterRouterPool) }), | ||
| new BuiltInSerializer("reliable-delivery", typeof(ReliableDeliverySerializer), (system, _) => new ReliableDeliverySerializer(system), |
There was a problem hiding this comment.
This shows why bindings belong to rows. In #8409, cutting IDeliverySerializable over to the V2 serializer means moving it from this row to a reliable-delivery-v2 row. Reads still dispatch by serializer id, so old payloads still decode. The flat BoundTypes list couldn't express that move.
| public static void AssertTableMatchesConfig(Config moduleConfig, IEnumerable<Type> serializerTypes, IEnumerable<Type> boundTypes) | ||
| /// <param name="moduleConfig">The module's own reference.conf (or the combined config of its files).</param> | ||
| /// <param name="registrations">Each registration's alias, serializer type, and the types it binds.</param> | ||
| public static void AssertTableMatchesConfig( |
There was a problem hiding this comment.
The parity check is stricter now. The old check only compared type sets, so a row with the right type and the wrong alias passed. #8678 and #8679 shipped exactly that bug: DData's akka-data-replication and akka-replicated-data were swapped, and we caught it by hand. Now each alias must name the same type it names in config, both ways. I removed the old three-argument overload because nothing calls it any more.
|
|
||
| // bindings match in both directions: every table binding is a config row under the same alias, and | ||
| // every config row is bound by exactly one registration under that alias | ||
| tableBindings.Should().BeEquivalentTo(configuredBindings); |
There was a problem hiding this comment.
Comparing (Type, Alias) pairs catches a type bound under the wrong alias, a binding missing from the table, an extra binding in the table, and a type bound under two rows. That's what PR 4 needs before it deletes the HOCON binding rows: proof that the table says exactly what they say.
| { | ||
| var table = new MetricsSerializers(); | ||
| ModuleSerializerSpecs.AssertTableMatchesConfig(MetricsRows, table.Serializers.Select(s => s.Type), table.BoundTypes); | ||
| ModuleSerializerSpecs.AssertTableMatchesConfig(MetricsRows, table.Serializers.Select(s => (s.Alias, s.Type, s.Bindings))); |
There was a problem hiding this comment.
Each module spec makes the same one-line change to pass (Alias, Type, Bindings). All eight modules (Remote, Streams, Cluster, Tools, Sharding, DData, Metrics, Persistence) now check alias and binding parity against their own reference.conf.
| { | ||
| new ModuleSerializer(typeof(FakeSerializer), | ||
| (system, config) => config.IsNullOrEmpty() ? new FakeSerializer(system) : new FakeSerializer(system, config)) | ||
| new BuiltInSerializer("fake-module", typeof(FakeSerializer), |
There was a problem hiding this comment.
The fake module carries its bindings on its row, like the real tables. The alias only needs to be unique here, because this spec tests loading and lookup, not config parity.
Delete the bespoke BuiltInSerializer record and have each module's ModuleSerializers.Create(system) build an ImmutableHashSet<SerializerDetails> directly, the same shape SerializationSetup already uses. Serialization now builds each module's serializers at most once per instance (a new ModuleResolver), indexing by the built serializer's own type instead of a separate type+factory pair. PrimitiveSerializers is the only table that reads a settings block, and it now reads akka.actor.serialization-settings.primitive itself instead of being fed a per-alias Config. ModuleSerializerTable keeps caching the loaded ModuleSerializers object process-wide; building its serializers needs the ActorSystem, so that part is cached per Serialization instance instead. A missing member thrown from Create is caught the same way a missing member from the module's constructor already was, so the module still counts as absent under version skew. Update every module's table (Remote, Streams, Cluster, Cluster.Tools, Cluster.Sharding, DistributedData, Cluster.Metrics, Persistence), the shared parity helper, and the module and loader test specs to match.
…not eagerly Create(system) built every serializer a module has, including ones no HOCON row ever asked for - triggered both by a genuine serializer row and by a binding row's lookup against the module. Some of those serializers need their own module's config and throw when it is missing, so a config that only ever touches one of a module's rows (or only binds one of its types, with no serializer row at all) could fail to start, even though the same config worked before this table rework. Build a module only from a serializer row that names it; a binding row now only reads modules this config already built, never builds one itself. A binding-only row for a module type with no matching serializer row falls back to reflection (or throws NotBuiltIn, same as today, with the switch off). Make every built-in serializer safe to build without its own module config: PrimitiveSerializers gets an empty settings block instead of null, and ReplicatorMessageSerializer falls back to its reference.conf's own default cache TTL instead of scheduling a repeating timer with a zero interval. Replace the ModuleResolver class with two static helpers and a local dictionary in the Serialization constructor - smaller, and keeps the module lookups where they were before this rework. When a module's Create throws a version-skew exception and dynamic type loading is off, that exception now rides along on the ConfigurationException instead of being hidden behind a plain "not built in, enable the switch" message. Add one spec per module asserting Create does not throw against a system that never loaded that module's own config, plus two Remote-specific regression specs reproducing the exact failures this fixes.
… never builds a module PrimitiveSerializers now falls back to Remote.conf's use-legacy-behavior = on, and the DData cache TTL only falls back when the setting is missing, so an explicit bad value still fails at startup as on dev. Adds a switch-off spec for a binding-only Remote row and checks that a skew hit in Create is kept as the inner exception.
Add rows for akkadotnet#8222 (Serializer overloads, ByteArraySerializer manifest), akkadotnet#8465 (explicit TLS hostname check), akkadotnet#8132 (TCP connects bypass akka.io.dns) and akkadotnet#7557 (object handler without predicate blocks later Receive calls). State net10.0-only once, folding in akkadotnet#8594 and the dependency bumps; fix the ByteString row's write types; narrow the akkadotnet#8698 row to module serializers; drop the .Internal AddOrSet row and say why in the preamble; shorten rows; use asterisk list markers.
…8698 (#8700) * Audit BREAKING_CHANGES_V1.6.md against v1.5.71 and catch up through #8698 Restate the definition (binary / source / behavioral compatibility against the last stable v1.5 release), drop rows that are not breaking under it (Artery-only, DynamicTypeLoading-off-only, fixes, dev-only comparisons), fold the TargetInvocationException and trim-annotation rows into one row each, shorten the rest, mark every row Merged with its PR, and add the breaking changes from #8694 and #8698. * Add pre-ledger breaking changes to BREAKING_CHANGES_V1.6.md Cover changes on dev since the v1.5 line diverged that landed before the ledger existed: net10.0-only targeting and ByteString removal (#8132), App.config HOCON loading removal (#7456), and the AddOrSet removal (#7622). * Apply review feedback to BREAKING_CHANGES_V1.6.md Add rows for #8222 (Serializer overloads, ByteArraySerializer manifest), #8465 (explicit TLS hostname check), #8132 (TCP connects bypass akka.io.dns) and #7557 (object handler without predicate blocks later Receive calls). State net10.0-only once, folding in #8594 and the dependency bumps; fix the ByteString row's write types; narrow the #8698 row to module serializers; drop the .Internal AddOrSet row and say why in the preamble; shorten rows; use asterisk list markers. * Drop the #7557 ReceiveActor row from BREAKING_CHANGES_V1.6.md The Receive(typeof(object), Func<object, bool>) regression is being fixed in a separate PR instead of documented.
Purpose
Part 2 of #8676. The C# module tables (
RemoteSerializers,StreamsSerializers,ClusterSerializers,ToolsSerializers,ShardingSerializers,DistributedDataSerializers,MetricsSerializers,PersistenceSerializers) now hold everything their HOCON rows hold: alias, serializer, and bindings. HOCON still drivesSerialization. Building a module's serializers now happens only when one of that module's ownserializersrows asks for it - see the startup-regression fix below for why that distinction matters. PR 3 will make the tables the source and load them from code.SerializerDetails
The bespoke
BuiltInSerializer(Alias, Type, Create, Bindings)record is gone. Each module's table now builds the publicSerializerDetailstype instead - the same typeSerializationSetupand V2'sSerializerRegistration.CreateDetailsalready use:A table fills it in directly, e.g.
ClusterSerializers:Every alias and bound type is unchanged from the previous
BuiltInSerializertables.PrimitiveSerializersis the only built-in that takes a settings block. It used to be handed a per-aliasConfigbySerialization; now its table readsakka.actor.serialization-settings.primitiveitself, falling back to Remote.conf's own default (use-legacy-behavior = on) on a system that never loaded Remote.conf - see below.LoadedModulenow indexes by the built serializer's ownGetType()and byUseFor, instead of by a separateTypefield.Serializationbuilds a module's serializers - callingmodule.Create(system)- at most once per module, the first time one of that module's ownserializersrows names it, caching the result (or the lack of one) in a local dictionary in its constructor.ModuleSerializerTablestill caches the loadedModuleSerializersobject once per table instance (that part needs noActorSystem); building its serializers does, so that part lives in the constructor's own dictionary instead.Version skew:
Create(system)can now throw too, since it builds real serializer instances (constructor, not justtypeof). It's wrapped in the same catch that already handled a module's constructor throwing, so the module still counts as absent rather than failing the wholeActorSystem. When dynamic type loading is off and that's the only reason a row didn't resolve, the skew exception now rides along as theConfigurationException's inner exception, instead of being hidden behind a plain "not built in, enable the switch" message - a missingGoogle.Protobuf.dllnow says so. Added a loader test for skew thrown fromCreate, alongside the existing constructor/static-constructor cases.Dropped the old "factory returns null skips the alias" safety net for module tables - no module factory ever returned null, that was only ever a safety net. Core's own
BuiltInSerializerstable inSerialization.cs(thejsonnull-skip underAkka.DynamicTypeLoading) is untouched.Fixed: a config that only touched one module row could fail to start
Review of the first version of this change (commit
e95eebf34) caught a startup regression:Create(system)builds every serializer a module has in one shot, and it ran for (a) any serializer row naming the module, and (b) a binding row's lookup against the module - even one with no matching serializer row at all. Some of those serializers need their own module's config and throw when it is missing (PrimitiveSerializerson a null config,ReplicatorMessageSerializerscheduling a repeating timer with a zero interval). So a config that worked fine ondev- one row namingproto, say, with no other Remote rows - could fail to start on this branch, because answering that one row builtprimitivetoo, andprimitivethrew.Two changes fix this:
NotBuiltIn(off) - see Breaking changes.PrimitiveSerializersgets Remote.conf's default block (use-legacy-behavior = on) instead of null.ReplicatorMessageSerializerfalls back to its reference.conf's own 10-second cache TTL when the setting is missing. An explicit bad value (0s,infinite) still fails at startup, as ondev.Also replaced the
ModuleResolverclass with two static helpers (FindModuleSerializer,FindModuleBoundType) plus a local dictionary in theSerializationconstructor - smaller, and it keeps the module lookups where they lived before this rework instead of in a new class.Added one spec per module asserting
new XSerializers().Create(system)does not throw against a system that never loaded that module's own config, plus Remote-specific regression specs for the failures above (a loneprotorow; a binding-onlyRemoteWatcher+Heartbeatrow with the switch on, and the same row with the switch off, which must throw because a binding row never builds a module). The Remote and DData no-config specs and the lone-protospec fail one95eebf34; the switch-off spec fails if a binding row is allowed to build its module again (checked by mutation).Cross-module bindings
None found. Every binding row in every module's config (Remote, Cluster, Cluster.Tools' three files, Cluster.Sharding, DistributedData, Cluster.Metrics, Persistence, Streams) binds to an alias defined in that same module's own
serializersblock. Remote.conf'sSystem.String/System.Int32/System.Int64→primitiveand severalAkka.Actor.*types →akka-miscrows look like they could be "someone else's" alias because the bound types live in CoreLib/Akka.dll rather than Akka.Remote, but the alias itself is Remote's own - this is the existing fallback-lookup case (nowFindModuleBoundType), not a cross-module binding, and needed no special handling.Parity spec
ModuleSerializerSpecs.AssertTableMatchesConfignow takesIEnumerable<SerializerDetails>instead of the old tuple shape, and compares(Alias, Serializer.GetType())againstakka.actor.serializersrows and(UseFor type, Alias)pairs againstserialization-bindingsrows, in both directions - same strictness as before. Every module spec now calls it asModuleSerializerSpecs.AssertTableMatchesConfig(ModuleRows, new XSerializers().Create((ExtendedActorSystem)Sys)).Mutation checks
DaemonMsgCreate's binding fromdaemon-createtoakka-system-msginRemoteSerializers- the parity spec failed (alias mismatch) as expected, then reverted.akka-clusterasakka-clusterrinClusterSerializers- the parity spec failed (alias mismatch) as expected, then reverted.Testing
dotnet build Akka.slnx -c Release -warnaserror: 0 warnings, 0 errors, whole solution (includingAkka.Benchmarks).dotnet test -c Release --no-build --framework net10.0 --filter "FullyQualifiedName~Serializ"forAkka.Tests(139 passed, 1 skipped, pre-existing/unrelated),Akka.Remote.Tests(183 passed, 1 skipped, pre-existing/unrelated),Akka.Streams.Tests(10),Akka.Cluster.Tests(78),Akka.Persistence.Tests(22),Akka.Cluster.Tools.Tests(56),Akka.Cluster.Sharding.Tests(22),Akka.DistributedData.Tests(28),Akka.Cluster.Metrics.Tests(7) - all passed.Akka.Remote.TestsRemoteDeathWatchSpec: 5 passed.dotnet test -c Release src/core/Akka.API.Tests: 24 passed - confirms no public API change (everything touched isinternal;SerializerDetailswas already public).git grep -n BuiltInSerializer -- src: no match for the exactBuiltInSerializeridentifier (two unrelated, pre-existing spec classes -BuiltInSerializerIdentifierSpec,BuiltInSerializerDefaultsSpec- and core's own long-standingBuiltInSerializersdictionary inSerialization.cs, which this PR deliberately leaves alone, still match the bare substring).src/aot/Akka.AOT.App, matchingbuild-system/pr-validation.yaml'sAotCanaryjob): unrooted publish + run printed[canary] OK, exit 0; rooted publish compared againstaot-warnings.baseline.txtviascripts/CheckAotWarnings.cs- 8 warnings in scope, all baselined, 0 new. Re-run after the regression fix below (same result).e95eebf34(or under the mutation above) and pass now.Breaking changes
Serializerinstance, rather than a fresh one constructed against that alias's ownserialization-settings.<alias>block. This can change wire output: ondev, bindingmy-primitivetoPrimitiveSerializerswith its ownserialization-settings.my-primitive.use-legacy-behavior = offmanifests alongas"L"; on this branch,my-primitiveshares the built-inprimitiveinstance, which (absent its ownserialization-settings.primitiveoverride) manifests the same value as"System.Int64, System.Private.CoreLib". OnlyPrimitiveSerializersis affected in practice, since it's the only built-in that reads a settings block.serialization-bindingsrow for a module type that has no matchingserializersrow from that same module no longer resolves through the module table. With dynamic type loading on (the default) it still resolves through reflection, unchanged; with the switch off, it now throws the ordinaryNotBuiltInconfiguration error instead of silently building the module. A config that binds a module type without also declaring that module's own serializer alias is unusual but not invalid HOCON, so this is called out here even though no projects we could find rely on it.Next
PR 3 will load these tables from code as defaults, applied before HOCON and
SerializationSetup, per D3 in the plan.