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

Akka.Cluster.DistributedData #2261

Merged
merged 3 commits into from
Nov 23, 2016
Merged

Conversation

Horusiath
Copy link
Contributor

@Horusiath Horusiath commented Aug 23, 2016

This PR includes @bruinbrown work on ddata module with code updated by me. Basically the core module is ported - not sure if I didn't miss something, I was sitting late on that yesterday - missing part I'm going to work on is test suite (especially multinode).

NOTE: originally ddata messages use protobuf for serialization, however I've found out that we use some generics there and originally we used reflection to make serialization possible. I've erased generic types where I could, however I think, Wire would be a better choice here. @akkadotnet/core aggree?

Status:

  • Core project:
    • Flag
    • GCounter
    • GSet
    • LWWDictionary
    • LWWRegister
    • ORDictionary
    • ORMultiDictionary
    • ORSet
    • PNCounter
    • PNCounterDictionary
    • PruningState
    • Replicator
    • VersionVector
  • Tests:
    • ReplicatedDataSerializerSpec (used Wire instead of custom protobuf-based)
    • ReplicatorMessageSerializerSpec (used Wire instead of custom protobuf-based)
    • FlagSpecsSpec (passing)
    • GCounterSpec (passing)
    • GSetSpec (passing)
    • LWWDictionarySpec (passing)
    • LWWRegisterSpec (passing)
    • ORDictionarySpec (passing)
    • ORMultiDictionarySpec (passing)
    • ORSetSpec (passing)
    • PNCounterSpec (passing)
    • PNCounterDictionarySpec (passing)
    • PruningStateSpec (passing)
    • VersionVectorSpec (passing)
    • LocalConcurrencySpec (passing)
    • WriteAggregatorSpec (passing)
    • LotsOfDataBot - to be implemented as part of performance spec
  • MultiNode tests - can be implemented in a separate PR:
    • JepsenInspiredInsertSpec (ported, not verified)
    • PerformanceSpec
    • ReplicatorSpec (ported, not verified)
    • ReplicatorChaosSpec (ported, not verified)
    • ReplicatorPruningSpec (ported, not verified)

@Horusiath
Copy link
Contributor Author

Horusiath commented Aug 27, 2016

All tests pass locally. Once our CI will prove that, it should be safe to merge. Remaining work, that still needs to be done, but as we're not going to publish that package yet, we can do it in a separate PRs:

  • Port and fix MNTK specs.
  • Port LotsOfDataBot into some sort of NBench performance spec.
  • Performance optimizations.
  • Serialization tests - currently tests are only asserting local replication. We need to check if replicated collections are serialized/deserialized correctly.
  • Replicated collections API - right now it's almost 1-1 copy of JVM model, with some changes mostly in naming conventions. I think, that replicated collections should follow design closer to what we have in System.Collections.Immutable (i.e. extracting ORSet and GSet into some kind of IReplicatedSet interface). Also pretty much everything is exposed public right now. We need to check what needs to be internal or private.

@Aaronontheweb
Copy link
Member

Need to approve the API changes made to Akka.Cluster.

@Horusiath Horusiath force-pushed the distributed-data branch 2 times, most recently from 20a113b to 29dfcfd Compare August 31, 2016 07:28
@Horusiath
Copy link
Contributor Author

I don't know why it broke on the MNTK specs. @Aaronontheweb could you verify?

@alexvaluyskiy
Copy link
Contributor

@alexvaluyskiy
Copy link
Contributor

It would be better to make all protobuff serializer classes as internal

@Horusiath
Copy link
Contributor Author

I need to make sure, that JVM protobuf definitions can be used here. JVM version of replicated collection assumes, that all dictionaries use string keys. I've assumed, that they may be generic. Also bruinbrown introduced collection keys, which are generic types - I don't know how protobuf will behave in this situation.

Those were the reasons, why I opted to use Wire instead.

@Horusiath
Copy link
Contributor Author

UPDATE: I've implemented serialization specs - they pass under Wire akka serializer. My proposition is to stay with that, and if necessary (and it may be necessary, as custom serializer provided compression and optimizations), make custom serializer based on Wire anyway.

@Horusiath Horusiath force-pushed the distributed-data branch 3 times, most recently from c123104 to ee69fb1 Compare September 4, 2016 18:02
@Horusiath
Copy link
Contributor Author

UPDATE: I've fixed failing tests and added Akka.DistributedData.Local versions for each CRDT - they are surrogated wrappers that always operate in the context of the current cluster. This way we may avoid passing node address each time we want to update current CRDT.

@Horusiath Horusiath force-pushed the distributed-data branch 3 times, most recently from 0e5ed37 to d2b0d19 Compare November 11, 2016 21:19
@Horusiath Horusiath changed the title [WIP] Akka.Cluster.DistributedData Akka.Cluster.DistributedData Nov 23, 2016
@Aaronontheweb Aaronontheweb merged commit 84cafff into akkadotnet:dev Nov 23, 2016
@Aaronontheweb
Copy link
Member

Nice work @Horusiath !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants