cloudstateio / cloudstate

Distributed State Management for Serverless
https://cloudstate.io
Apache License 2.0
764 stars 97 forks source link

Evolve the TCK #316

Closed marcellanz closed 3 years ago

marcellanz commented 4 years ago

After a short Gitter discussion with @pvlugter and @sleipnir, I open this issue for discussion (attached is the gitter exchanges for context).

In short, the issue is to evolve the TCK so that a language support library implementation can be verified.

I think the topic is too broad to capture it in one issue and I propose to discuss it here and then create actionable issues out of it. I list here suggested topics and existing related issues.

Suggested Objectives

  1. Evolve the TCK for all state models defined by the Cloudstate Protocol.
  2. Enhance or adapt the TCK Shopping Cart Example to include other state models as explanatory as possible. This might not be exhaustive in terms of TCK completeness, but still have a use case that is easy to understand, and also can be an example how to use the state models. I'm not sure if this is that important for the TCK.
  3. Define synthetic entities and interactions so that the Cloudstate Protocol can be verified by the TCK. These entities have to be provided then by each support library so that the TCK can run against them.
  4. Let support libraries execute the TCK within their repositories, preferable without the need of a JVM to be installed or a SBT dependency => a Docker based approach might be feasible.
  5. Maybe, existing tests of the proxy can be used to guide the synthetic tests defined.
  6. Perhaps for the future and a low priority for now: the TCK currently runs as "Akka + '"; how would the TCK test a reference implementation of the proxy.

Related Issues


Related discussion (Gitter, May 11th)

@marcellanz : I think it may be time to extend the TCK for other state models like CRDTs, Stateless, KV, Eventing, for Cloudstate. I come to a point where I'd like to verify an implementation and can't. The TCK so far was a great enabler getting started with EventSourcing.

There is tck/src/ with the Shopping Cart example. How should we model the TCK around other state models? While the proxy itself has many synthetic test cases (AFAIKS) the TCK shopping cart is a nice and illustrative example, should we find such an example for other types too, or perhaps expand on the shopping cart? With a few language support libraries in different states existing, and new coming, we also might better have the TCK before they are complete.

@sleipnir: I think we should do a lot of things in several steps (parallel or not) at TCK. I have illustrated this need several times and I agree with you. So my thinking is divided like this?

  1. The TCK must be less than a selected docker image and accept the necessary parameters to run the tests easily, without depending on the entire Cloudstate project.
  2. We must expand the TCK and the shopping cart example, there is a reference somewhere in the list that can be used (HotItems) for example to test "forwards and effects" that are not currently supported in the TCK.
  3. We must include (with shoppingcart or another) a test for statelessness (a simple request/response would be enough in this case).
  4. A basic example for CRDT should be included at first and then expanded to more complex examples. I also think it would be a good way to test the various types of CRDT independently, that way we could have a compatibility matrix supporting in supported idioms more granular.

@pvlugter: Yes, please open an issue, if it doesn't exist already, with everything we need for the TCK. And we have cloudstateio/cloudstate#216 for supporting the TCK in the language supports. I'm personally not sure whether having the TCK based on examples, like shopping cart, or just artificial tests is better. Possibly a combination in the end? Simple synthetic test cases to get full coverage, plus examples that demonstrate the main use cases.

marcellanz commented 4 years ago

Discussion Notes:

CRDT Similar to the ShoppingCartClient that the TCK is using, there is a io.cloudstate.samples.CrdtsClient. Also similar to the shopping cart example, there is a crdt-example.proto that defines the corresponding io.cloudstate.samples.CrdtExample service.

The CrdtExample Service covers

with missing

How to implement Implement the io.cloudstate.samples.CrdtExample and execute its service interface similar to the shopping cart TCK. This service can be the synthetic use case for the CRDT TCK. The synthetic methods used and possibly expanded can be similar or even the same as the language supports unit tests. There is a difference to eventsourcing as CRDTs can have streamed commands. The io.cloudstate.samples.CrdtExample service has already two methods from samples-java-chat where streamed commands are used.

Stateless TBD

KV TBD

marcellanz commented 4 years ago

@sleipnir this can be perhaps done similarly for the stateless model? wdyt?

marcellanz commented 4 years ago

I bootstrapped myself a bit and started with the extension of the existing TCK implementing synthetic tests for the CRDT support.

Screenshot 2020-05-18 at 01 05 00

I currently use the existing io.cloudstate.samples.CrdtExample service from the crdt-example.proto and wired its service client into io.cloudstate.tck.CloudStateTCK so that it gets setup and called similar to existing event sourcing tests. I'm not yet sure how to test replication of CRDTs this way.

WIP branch Cloudstate: https://github.com/marcellanz/cloudstate/tree/feature/tck_crdt_support

marcellanz commented 4 years ago

Testing CRDT state replication within the TCK was surprisingly easy to do, at least to trigger an init message with a state applied by the proxy:

I'm sure there is perhaps a better way to force the proxy to replicate a CRDTs state, but this way, a full circle of an crashed and re-initialized user functions state is involved, which might be a use-case of its own.

sequence: tck_crdt_support_001.json – Verify GCounter

sleipnir commented 4 years ago

Beautiful work

Em segunda-feira, 18 de maio de 2020, Marcel Lanz notifications@github.com escreveu:

Testing CRDT state replication within the TCK was surprisingly easy to do, at least to trigger an init message with a state applied by the proxy:

I'm sure there is perhaps a better way to force the proxy to replicate a CRDTs state, but this way, a full circle of an crashed and re-initialized user functions state is involved, which might be a use-case of its own.

sequence: tck_crdt_support_001.json – Verify GCounter https://gist.github.com/marcellanz/f9dfc651941e08666260ab21517f4137

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/cloudstateio/cloudstate/issues/316#issuecomment-630445962, or unsubscribe https://github.com/notifications/unsubscribe-auth/AACTTZXBJP4L3SNXPTM7RDLRSGSJZANCNFSM4M7DLGGQ .

-- Adriano P. Santos

"O homem erudito é um descobridor de fatos que já existem - mas o homem sábio é um criador de valores que não existem e que ele faz existir." Albert Einstein

pvlugter commented 4 years ago

👍 great work

marcellanz commented 4 years ago

I'm coming to the point where crdt-example.proto limits its use to TCK-verify a CRDT implementation. Also because crdt-examples.proto lacks a few CRDT types and I'd like to verify streamed commands for the CRDT types.

I came to the conclusion that the CRDT TCK for sure has a completely synthetic part to capture all CRDT types. A useful synthetic service would not serve any other purpose than operate on CRDT types' API for verification. Anything with a practical use-case like the shopping cart would be supplementary.

There are uses for crdt-examples.proto right now for:

Two a bit older and CrdtStressSpec.scala heavily used.

Options

I think I'd probably go for the latter option and having the crdt-examples.proto use consolidated in the future or even leave it as it is.

@pvlugter @viktorklang @jroper wdyt?

viktorklang commented 4 years ago

I vote for creating a new service as well. The original thought of the shopping-cart being both a tck validator and an example was a good idea initially but as features grow I feel the need for dedicated tck protos increases. -- Cheers, √

pvlugter commented 4 years ago

Agree on adding a new service for synthetic tests.

marcellanz commented 4 years ago

Thanks for voting :)

I have a few points for the service design and the TCKs behaviour for the synthetic tests. Let me know wdyt, as the last point is a bit of work:

New service is under protocols/tck/tck_crdt.proto. I think protocols/example is for examples, even if the evensourcing TCK shoppingcart service is currently under example.

1:1 mapping of TCK-Service <> CRDT API There is no implicit protocol or story of things like mutating domain data and then error on that. Each CRDT type will have gRPC service method that is in line with their API. There will be most of the time a 1:1 mapping of what the TCK calls on the support library. This also simplifies the implementation on the user support library.

Errors There is room "to error", but the support library has to do it explicitly by doing so in part as we can't enforce invalid data to be sent most of the time. An example is to decrement a GCounter, we can't enforce that as, GCounter state and delta are by design uint64 and, depending on the language, the implementation would have to type-cast values to an incompatible type.

If we really like to decrement a GCounter, we would have a dedicated TCK rpc method where we instruct the support library to decrement the GCounter. And as said above, this would lead to an unusual typecast to call GCounter.Increment(-1) if that is even possible with the libraries types API. (JS and Python are exceptions of that, the two most common languages currently used in the field3.4.4, so its perhaps still a good idea to do so.)

Force errors / Replicate state To test error handling, we could send a flag that the user support should error. This way we could test the operation would error and with it, that the user support would propagate this error properly. Also, having an entity being crashed right now, is the preferred way to let the proxy send an init message with an initial state and lets us test applying state to a CRDT. (I currently don't know how to have the proxy sending delta-messages, @jroper do you have an idea how to do that?)

Test Coverage The TCK will cover the complete set of unit tests like we have already for user support in JS and Go. It makes little sense only to cover a few smoke-like tests I think.

This applies especially for synthetic tests and might be different for a TCK use-case like the shoppingcart that we have right now for eventsourcing. There, the user support library proves to implement that usecase and can be used for demonstration purposes. This can be useful as one can replace the JS based shoppingcart implementation with any of the other ones implemented for the TCK, and the shopping cart demo would work.

sleipnir commented 4 years ago

Hello everyone! I think having a TCK with specific services to cover tests with all types is a great idea. But I think that support libraries should make some individual effort in implementing language-specific coverage tests. In the spring support, for example, the shopping cart is tested through unit tests, so that I know that the change to pass the TCK is highly likely before I even put it in the TCK test.

marcellanz commented 4 years ago

I agree; every language support implementation should have their own unit tests covering, the TCK does not replace that. I'm sure, many language specific tests can't be done by the TCK and it was not my intent to propose that.

sleipnir commented 4 years ago

Yes @marcellanz I understood that it was not your intention. Is that I believe that a mix of the two approaches (TCK, Coverage tests) should be desired. I say this because in your GCounter negative example I see this being tested in the support libraries because it is a very specific case and it is very related to the unfortunate path of a request. What I mean is that in addition to a good TCK we have to have a good specification on the expected behaviors, this must be defined clearly and publicly, so that the implementers have the ability to validate the idiosyncrasies of each language against the expected behaviors that were defined by the protocol.

marcellanz commented 4 years ago

in addition to a good TCK we have to have a good specification on the expected behaviors, this must be defined clearly and publicly, so that the implementers have the ability to validate the idiosyncrasies

An excellent point! This expected behaviour is, from the implementers point of view, not always clear by solely reading the Cloudstate protocol spec, which is the *.proto file. To implement the CRDT types and the CRDT state model service for Go, I had to read existing implementations (Node, Scala) to understand what is expected.

To have a relatively simple API with the TCK here, the 1:1 API mapping, helps so in the sense that implementing the user language part of the TCK service can be kept simple. But it does not help to understand how to implement a state model like CRDT. Things like what does "applying delta" state mean, or why and how does an implementation handle "changes", streamed commands and everything a protocol.CrdtStreamedMessage transports back to the proxy is out of scope of that.

The aspects of the CRDT state model and its accurate implementation will emerge, as soon as the TCK enforces the proxy to replicate and re-establish state to an entity, and then the expected behaviour or better said state is being verified or not. In this regard the TCK is a functional test of the state model, it does not describe how the user support library should implement that.

marcellanz commented 4 years ago

What I mean is that in addition to a good TCK we have to have a good specification on the expected behaviors, this must be defined clearly and publicly

… re-reading this and in short, this means we need a clear spec. What we have right now is not enough to describe what is accurately needed to implement the CRDT state model as an example.

We have a Draft-PR for that https://github.com/cloudstateio/cloudstate/pull/119 and a few discussions. One of my tasks of the Go CRDT support is to give input and feedback of how the spec can describe the state model better in this regard.

sleipnir commented 4 years ago

@marcellanz I would go further and say that today we have no specifications. We have a technical contract defined in a series of .proto files but I don't think this is a formal protocol specification. It certainly wouldn't pass as a draft at the IETF or anything like that. What I mean is that .proto files don't explain behavior, they just indicate what are the inputs and outputs that implementers need to infer behaviors from.

marcellanz commented 4 years ago

An example is to decrement a GCounter, we can't enforce that as, GCounter state and delta are by design uint64 and, depending on the language, the implementation would have to type-cast values to an incompatible type.

naahhh... I have to revise this a bit, the scala client and therefore java type will accept negative values to increment and will happily accept negative values, akkas ddata will remind us that this is not allowed though. A nice property of other languages not doing this kind of type conversions (Go).

java.lang.IllegalArgumentException: requirement failed: Can't decrement a GCounter
    at scala.Predef$.require(Predef.scala:281)
    at akka.cluster.ddata.GCounter.increment(GCounter.scala:98)
marcellanz commented 4 years ago

It certainly wouldn't pass as a draft at the IETF or anything like that. What I mean is that .proto files don't explain behavior, they just indicate what are the inputs and outputs that implementers need to infer behaviors from.

@sleipnir very much true!

I think priorities are not on this right now. Standards are a lot of work. Standard also presume interest and work to spread and having them proved by their usefulness. I think this is what we all work on right now, yes? :-) There is a good amount of input on https://github.com/cloudstateio/cloudstate/pull/119 and I'm happy to discuss all that and help. In the meantime our reference implementation gets built. Having this will lead us to a spec and a standard. Having no spec actually liberates us from too much orchestration right now.

sleipnir commented 4 years ago

Yes, of course, it was just an observation and a warning that freedom also has a price. We have speed now that costs a little formal precision. But nothing that we cannot survive.

marcellanz commented 4 years ago

I'm working on this the way proposed earlier. (don't judge me on my Scala code, I don't yet know what I'doing)

WIP branches:

marcellanz commented 3 years ago

I think this issue can be closed now as the TCK has been evolved into a model based TCK capturing mutiple state models now and is continuously expanded for new requirements and asditions to the Cloudstate protocol. wdyt all @pvlugter, @viktorklang, @sleipnir?

pvlugter commented 3 years ago

Yes, was going to close after some more reorganisation in the TCK, and removing the shopping cart in particular, and using the model entities for proxy tests. But can be closed now. Tests for the different state models are added now.

viktorklang commented 3 years ago

I'll defer to @pvlugter :)

pvlugter commented 3 years ago

For tracking progress:

marcellanz commented 3 years ago

cool 👍