-
Notifications
You must be signed in to change notification settings - Fork 11
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Serialization pipeline Fixes/improvements #22
Serialization pipeline Fixes/improvements #22
Conversation
…lizer`, remove perf numbers till we make new ones
0bbd829
to
3748fcf
Compare
deserialized = | ||
Akka.Serialization.Serialization.WithTransport( | ||
_serializer.System, | ||
() => deserializer.FromBinary(t.message, type)); | ||
|
||
return new Try<(IPersistentRepresentation, IImmutableSet<string> | ||
, long)>(( | ||
new Persistent( Akka.Serialization.Serialization.WithTransport( | ||
_serializer.System, (_serializer.FindSerializerForType(type,_journalConfig.DefaultSerializer),t.message,type), | ||
(state) => | ||
{ | ||
return state.Item1.FromBinary( | ||
state.message, state.type); | ||
}), t.sequenceNumber, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we look closely here, we are using FindSerializerForType
here where we weren't previously. I think this was an oopsie in moving over the Serialization dance from SqlJournal
, and it wasn't caught before because it would ONLY trip as a case when recovering from a very old version of the journal where we did not include an Identifier.
@Aaronontheweb Looks like Docker tests failing somehow =/. Confused cause I'm not sure what I broke? |
NumericRangeEntry
behavior to use ayield
for it's Enumerator instead of allocating a bunch of lists.TState
overloads For Seriallizer.WithTransport
methods to improve copies just a little