Skip to content
This repository has been archived by the owner on Mar 16, 2022. It is now read-only.

Some food-for-thought for supporting Key-Value API, this shows we can… #143

Closed
wants to merge 2 commits into from

Conversation

viktorklang
Copy link
Contributor

… implement it as a pure lang-support alternative.

Related to #51

@viktorklang viktorklang added enhancement New feature or request user platform Issues related to the different user target platforms labels Nov 14, 2019
@viktorklang viktorklang self-assigned this Nov 14, 2019
final KVEntity.Builder builder = KVEntity.newBuilder();
unparsed.forEach(
(k, v) -> {
builder.putEntries(k, com.google.protobuf.ByteString.copyFrom(v.asByteBuffer()));
Copy link
Contributor

Choose a reason for hiding this comment

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

@viktorklang
This is the serialization for the user choosen for his values in the KVS, right?
Does this conflict or is it compatible with the stable serialization rules by Cloudstate? Is this relevant here too? I think about how, if done, a separate/different user lib, language wise, would parse the values back is they're are marshalled as ByteStrings here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, this needs work. I am not sure how to integrate it though—I'd need to go through AnySupport I think.

Copy link
Contributor

Choose a reason for hiding this comment

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

Could be a good example how to use AnySupport.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Indeed. I'll have a look at what that could look like.

@viktorklang
Copy link
Contributor Author

viktorklang commented Nov 15, 2019 via email

@marcellanz
Copy link
Contributor

marcellanz commented Nov 15, 2019

It’s just food-for-thought, but you’re right—for this to be correct we’d have to use the same serialization scheme as for the rest. I just did it like this for illustrative purposes 🙂

Cheers, √

@viktorklang no worries. I just wanted to be sure I got the idea right. cool stuff.

@viktorklang
Copy link
Contributor Author

Thought: Since this is more of a pattern than a distinct different storage mode, should we make this a documentation feature only?

@marcellanz
Copy link
Contributor

Thought: Since this is more of a pattern than a distinct different storage mode, should we make this a documentation feature only?

In #144 @jroper argues, that the user should not have to know that any of the CRDTs he uses is a CRDT, by using one. Similarly a user also should not have to know a "Cloudstate KVS" he uses is an event sourced entity underneath?

// We'll most likely want to add a specific CommandContext for KeyValueEntity
// so you'd instead of state().set(…) would do ctx.setState(…), and then it'd be automatically
// flushing ctx.emit(state.toProtoModification()) if there are any changes
@CommandHandler
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jroper What do you think about the comments above, James? Do you think this is something we could put together rather quickly?

Copy link
Contributor

Choose a reason for hiding this comment

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

I like that idea.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jroper ping :)

Choose a reason for hiding this comment

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

@viktorklang Any progress here? This seems to be stopped for a long time

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@sleipnir No, unfortunately not. I guess the question is: Should we go for an API-only approach or support KV natively in the proxy as a different way of storing data.

Copy link
Member

Choose a reason for hiding this comment

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

The CRUD support will provide the underlying native KV support discussed in this thread, and @ralphlaude is making progress there. (Side note: not sure if "CRUD" entities is the best name, maybe we can improve the naming there too.) Until that's complete, language supports could also experiment with including a setState or setValue style API directly on event sourced entities, setting a snapshot value, if we want to try this out earlier.

Also agree that it would be good to try out some use cases, to see how useful a KV API like in this PR might be, since this would need to be added to all the language supports. This currently gives a more generic Map-like API in place of defining protobuf message objects directly (which can also be map-like as needed). The shopping cart samples also use a map for state — would that example be improved with a generic key-value API, compared with the types it uses now for event sourcing? In these kinds of examples, would just setting a map as the value (and using CRUD support) already be clean enough?

Entity services themselves provide a KV-style store, keyed by entity id, and can be backed by event sourced, or the upcoming CRUD for a simple set-value approach, or using CRDTs for distributed, in-memory, eventually consistent storage. Within an entity the state can be a map (be key-value-based) like in this PR.

And then there's the concept of a KV store that's available across Entity or Action instances — like the Cloudflare Workers KV, which is an eventually-consistent LWW (last writer wins) key-value store distributed across the worker nodes. For Cloudstate, the pattern that we've demonstrated that's similar to this is having multiple entity types. Like the "hot items" CRDT entity used alongside the shopping cart. We have forwarding and side effects to combine entities. Maybe we should look at supporting an "ask pattern", an async get on another entity type, where values can be fetched from other entity types and then continue processing with the responses.

Copy link
Member

Choose a reason for hiding this comment

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

@jroper and I discussed this further. Some things we discussed:

We don't currently see clear use cases for a key-value API like this in the language supports. Especially when compared with simply using an object or map as the value (with CRUD or event sourcing support). For persisting deltas/changes only, there's the event sourcing already, and having an additional KV API increases the surface area that language supports need to implement.

For CRUD support naming, we're suggesting to refer to this as a value-based state model, and it could be the default state model for entities. From the user perspective, we could simply call these Entities. And then there are Event Sourced Entities and Replicated Entities (CRDTs). Within code or discussions, we could use value entities or value-based entities or persisted value entities as needed. And for services, entities themselves provide key-value storage (with the entity key and its value), which can be backed by simple key-value storage, event sourcing, or in-memory replication.

I think the main thing to do here is to complete the basic entities with value-based state model (CRUD support). In terms of roadmap, we also discussed other forward-looking things, like replicated and active-active event-sourcing, durable actions (as in long-lasting, that can fetch asynchronously, implement workflows), durable CRDTs, and product CRDTs.

So suggesting that we close this PR and put the idea of KV APIs in language supports on hold, and focus on finishing the value entities (CRUD support) and making those the default starting point. Interested to hear more thoughts on this, and it can be a topic for the next contributors' call.

Copy link
Contributor

Choose a reason for hiding this comment

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

@pvlugter thanks for summing everything up. It is a clear roadmap and the proposed names are fine for me.

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree that this is the best path forward.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Having a good "CRUD" solution might obviate the need for a KV solution, especially if the proxy can diff.

@pvlugter
Copy link
Member

Closing this, as discussed.

@pvlugter pvlugter closed this Oct 14, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request user platform Issues related to the different user target platforms
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants