Skip to content
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

Custom Serializer setup not propagating to Persistence #127

Closed
bauersystems opened this issue Oct 5, 2022 · 9 comments · Fixed by akkadotnet/akka.net#6528
Closed

Custom Serializer setup not propagating to Persistence #127

bauersystems opened this issue Oct 5, 2022 · 9 comments · Fixed by akkadotnet/akka.net#6528
Labels
akka-persistence bug Something isn't working wontfix This will not be worked on

Comments

@bauersystems
Copy link

bauersystems commented Oct 5, 2022

Version Information
Akka - v1.4.43
Akka.Hosting - v0.4.3
Akka.Persistence.SqlServer.Hosting - v0.4.3
Akka.Serialization.Hyperion - v1.4.43

Describe the bug
When setting up Hyperion as the default serializer via Akka.Hosting configuration, persistence still uses Json as the default serializer and has to be configured separately to use Hyperion.

To Reproduce
I used a configuration similar to the following:

        services.AddAkka("actor-system", (configurationBuilder, _) =>
        {
            configurationBuilder
                .WithRemoting("localhost", 7910)
                .WithClustering(new ClusterOptions() { Roles = new[] { "Coordinator-Role" }, SeedNodes = Address[] { Address.Parse("akka.tcp://actor-system@localhost:7910") } })
                .WithSqlServerPersistence("Server=localhost,1433; Database=Akka; User Id=sa; Password=[password];")
                .WithCustomSerializer("hyperion", new[] { typeof(object) }, system => new HyperionSerializer(system));
        {

Expected behavior
When setting up Hyperion as the default serializer, this setting should be propagated through-out the Akka.net modules.

Actual behavior
Json was still the default serializer for persistence.

Screenshots
image

Environment
Windows 11
.Net 6.0

Additional context

@Aaronontheweb
Copy link
Member

Thanks @bauersystems - we'll look into this

@Arkatufus
Copy link
Contributor

@bauersystems
Can you tell us what value you get when you run this SQL script on your journal database?

SELECT ej.SerializerId FROM EventJournal ej WHERE ej.PersistenceId = 'client_bauer-systems' AND ej.SequenceNr = 1

@Aaronontheweb Aaronontheweb added bug Something isn't working akka-persistence labels Oct 31, 2022
@Arkatufus
Copy link
Contributor

The proper SerializerId for Hyperion is -5, the SerializerId for the default JSON serializer is 1.

@Arkatufus
Copy link
Contributor

Arkatufus commented Nov 1, 2022

@bauersystems
It appears that this is a big design flaw with the Akka.Persistence package that never got found until today, I think we might not be able to fix it because it will break a lot of existing implementations.

The workaround is to add these custom HOCON configuration in:

builder
    .AddHocon(@"
akka.persistence.journal.sql-server.serializer = hyperion
akka.persistence.snapshot-store.sql-server.serializer = hyperion", HoconAddMode.Prepend))

These settings are a bit obscure, it overrides the object type serializer for that specific journal/snapshot store configuration.

@Arkatufus
Copy link
Contributor

You can also override the default behaviour by using these HOCON settings:

builder
    .AddHocon(@"
akka.persistence.journal-plugin-fallback.serializer = null
akka.persistence.snapshot-store-plugin-fallback.serializer = null
", HoconAddMode.Prepend)

@ismaelhamed
Copy link
Member

@bauersystems It appears that this is a big design flaw with the Akka.Persistence package that never got found until today, I think we might not be able to fix it because it will break a lot of existing implementations.

Actually, this is rather old: akkadotnet/akka.net#3133 (comment)

@Arkatufus Arkatufus added the wontfix This will not be worked on label Nov 21, 2022
@Arkatufus
Copy link
Contributor

Changing this would potentially break and/or change the behavior of systems that is already in the wild since it requires changes to be made on Akka.NET core. We will leave this as is.

@Arkatufus Arkatufus reopened this Nov 21, 2022
@Arkatufus
Copy link
Contributor

@Aaronontheweb Maybe we should add a new extension to Akka.Hosting that specifically target this?
Something like .WithDefaultPersistenceSerializer()?
We also need to add this specific peculiarity to the documentation.

@Aaronontheweb
Copy link
Member

@bauersystems @Arkatufus I'm thinking this is a bug with Akka.Persistence and it needs to be fixed akkadotnet/akka.net#6389

Arkatufus added a commit to Arkatufus/akka.net that referenced this issue Mar 16, 2023
Aaronontheweb pushed a commit to akkadotnet/akka.net that referenced this issue Mar 17, 2023
* Reproduction unit test for akkadotnet/Akka.Hosting#127

(cherry picked from commit 75e24b3)

* Clarify the unit test

(cherry picked from commit a0b5e8f)

* Remove persistence default serializer feature

(cherry picked from commit 3cf2eee)

* Revert changes on EventRead, add legacy data read test

(cherry picked from commit b1daa66)

* Revert persistence.conf changes, modify sqlite database to mimic legacy data

(cherry picked from commit f3fd0e9)

* Resolve conflicts

* Update API Verify list

* Add legacy data test

* Add sqlite data and fix tests
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
akka-persistence bug Something isn't working wontfix This will not be worked on
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants