Skip to content

feat: add specialised replicated maps#161

Merged
pvlugter merged 5 commits intolightbend:mainfrom
pvlugter:replicated-maps
Aug 16, 2021
Merged

feat: add specialised replicated maps#161
pvlugter merged 5 commits intolightbend:mainfrom
pvlugter:replicated-maps

Conversation

@pvlugter
Copy link
Copy Markdown
Member

@pvlugter pvlugter commented Jul 20, 2021

Refs #48. Test the specialised replicated maps using the new TCK tests from https://github.com/lightbend/akkaserverless-framework/pull/802 (needs to be published first, draft PR here). Support the new replicated maps from https://github.com/lightbend/akkaserverless-framework/pull/803. Resolves #49.

@pvlugter pvlugter marked this pull request as draft July 20, 2021 06:55
Comment on lines +228 to +231
ReplicatedRegisterMap<String, String> registerMap =
(ReplicatedRegisterMap<String, String>) replicatedData;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I may be missing something, but why both ReplicatedCounterMap and ReplicatedRegisterMap have type parameters?

The proto definitions define the (K, V) to be (String, Long) and (String, String) respectively.

True that I still need to wrap my mind around the whole replicate entity feature and get a better view on how that will be used, but at first sight my understanding is that we are already bound to (String, Long) and (String, String)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The key can be anything serializable — protobuf message, json, or primitive. Same for register values. The proto definitions here are just for the TCK tests, which are simplified to only string keys and string values for registers.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understood this was the TCK, but the map entry in the proto is defined as

message ReplicatedRegisterMapEntryUpdate {
  string key = 1;
  string value = 2;
}

How could we send anything other then String? Are we sending the bytes encoded as base64?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's the protocol for the TCK — which sends messages on how the entities should be updated, and only uses strings.

https://github.com/lightbend/akkaserverless-framework/blob/master/protocols/tck/src/main/protobuf/akkaserverless/tck/model/replicatedentity/replicated_entity.proto

The actual proxy protocol for replicated maps uses protobuf Any:

message ReplicatedMapEntryDelta {
  // The entry key.
  google.protobuf.Any key = 1;
  ReplicatedEntityDelta delta = 2;
}

message ReplicatedRegisterDelta {
  google.protobuf.Any value = 1;
  ReplicatedEntityClock clock = 2;
  int64 custom_clock_value = 3;
}

https://github.com/lightbend/akkaserverless-framework/blob/613d873529f8e0785218b0357c281a30a99c9d31/protocols/proxy/src/main/protobuf/akkaserverless/component/replicatedentity/replicated_entity.proto#L132-L136

https://github.com/lightbend/akkaserverless-framework/blob/613d873529f8e0785218b0357c281a30a99c9d31/protocols/proxy/src/main/protobuf/akkaserverless/component/replicatedentity/replicated_entity.proto#L119-L123

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, that explains everything. Thanks for clarifying.

@pvlugter pvlugter changed the title tck: add tests for specialised replicated maps feat: add specialised replicated maps Jul 27, 2021
Copy link
Copy Markdown
Contributor

@patriknw patriknw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@pvlugter
Copy link
Copy Markdown
Member Author

Rebased and updated to framework release 0.7.0-beta.16.

@pvlugter pvlugter marked this pull request as ready for review August 16, 2021 03:44
@pvlugter pvlugter merged commit 4fb0c58 into lightbend:main Aug 16, 2021
@pvlugter pvlugter deleted the replicated-maps branch August 16, 2021 03:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add ReplicatedMultiMap

3 participants