cloudstateio / cloudstate

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

Passivation configured in language supports via the discovery protocol #486

Closed ralphlaude closed 3 years ago

ralphlaude commented 3 years ago

Resolves #298

Passivation timeouts configured in language supports via the discovery protocol. The first step is the java-support and then node-support.

In the java-support all entity annotations are extended @XXXEntity (passivationTimeout = 10), the service class io.cloudstate.javasupport.Service is also extented with passivationTimeout and the grpc entity in the entity.proto.

ralphlaude commented 3 years ago

@pvlugter what do you think about the option?

ralphlaude commented 3 years ago

The discovery process, the proxy and the API have been extended to support passivation strategy and also that new passivation strategy (never, immedaite and perhaps resource specifics strategies) can be added without breaking the API. The only supported passivation strategy right now is timeout. Passivation strategy could be as complex as possible to configure and this does not fit within annotations. So the approach to pass configuration parameter at registration time is a better fit. There is EntityOptions for configuring passivation strategy and other aspects if needed. The language support should deal with default passivation timeout and more generally fallback. All entities must support a passavation strategy and right now it is timeout. The entity.proto file has been modified like this:

message Entity {
    ...

    // The passivation strategy for the entity.
    EntityPassivationStrategy passivation_strategy = 4;
}

// A passivation strategy for the entity which is sent to the proxy.
message EntityPassivationStrategy {
    oneof strategy {
    // the timeout passivation strategy.
        TimeoutPassivationStrategy timeout = 1;
        ImmediatePassivationStrategy immediate = 2; // this strategy can be added later on
        NeverPassivationStrategy never = 3; // this strategy can also be added later on
        // more strategies can be added here
    }
}

// A passivation strategy based on a timeout. The idle timeout after which an entity is passivated.
message TimeoutPassivationStrategy {
    // The timeout in millis
    int64 timeout = 1;
}

// A passivation strategy which passivates the entity after a command is handled.
message ImmediatePassivationStrategy {}

// A passivation strategy which never passivates the entity.
message NeverPassivationStrategy {}

For the java support it will be like at the end:


// passivation stratgey timeout with default passivation timeout set which is sent with entity spec to proxy
@EventSourcedEntity(persistenceId = "something")

// no extra options specified, passivation stratgey timeout with default timeout sent with entity spec to proxy
.registerEventSourcedEntity(
  SomeEntity.class,
  SomeProtocol.getDescriptor().findServiceByName("SomeService"),
  SomeProtocol.getDescriptor())

// options specified with passivation stratgey timeout, which will be sent with entity spec to proxy
.registerEventSourcedEntity(
  SomeEntity.class,
  SomeProtocol.getDescriptor().findServiceByName("SomeService"),
  EventSourcedEntityOptions.defaults().withPassivationStrategy(PassivationStrategy.timeout(Duration.ofSeconds(10)))
  SomeProtocol.getDescriptor())

Only the java support is implemented and all others language supports (Nodejs, GO, Kotlin, Python, Spring and so on) should be extended before this can be merged. Wo can help with other language supports (Nodejs, GO, Kotlin, Python, Spring and so on)?

marcellanz commented 3 years ago

Thanks @ralphlaude :) For Go for sure I can help. If needed we could work based on a dedicated branch so that maintained language supports can be aligned. As the new field EntityPassivationStrategy protocol-wise is an addition, entity discovery could probably even work without the support on user languages?

ralphlaude commented 3 years ago

@marcellanz thanks for Go. Yes we can think how to work on a dedicated branch to be aligned. With the addition of the new field EntityPassivationStrategy in the protocol the discovery is broken without the support on user language. It can be changed if we want it.

sleipnir commented 3 years ago

Hi @ralphlaude , thanks for another one! I can help with the other languages mentioned but generally as Kotlin and Springboot depend on the java-support jar I choose to wait for the release of an official release with the functionality, I do this to not have to build the jars locally. I sincerely wish there were more releases to help me with this, but I may have to follow the branches of the main repository with equivalent branches in the languages, I don't know, I find this approach very laborious, but this should be discussed in another forum.

marcellanz commented 3 years ago

@sleipnir I think, it should be possible or better said desirable for language supports to evolve before the core project releases. For non-JVM supports it is already manageable with tck- and proxy images available from master. For JVM support perhaps snapshot releases help? But as you said, could be discussed elsewhere.

sleipnir commented 3 years ago

For JVM support perhaps snapshot releases help? ...

Yes! But they are also often not available.

pvlugter commented 3 years ago

For JVM support perhaps snapshot releases help? ...

Yes! But they are also often not available.

It's easy to publish a versioned snapshot for java support. Just ask if you need one :)

pvlugter commented 3 years ago

With the addition of the new field EntityPassivationStrategy in the protocol the discovery is broken without the support on user language. It can be changed if we want it.

Since it's an additional field, the protocol can be compatible with older versions. In the proxy, instead of throwing exceptions if the language support doesn't define a passivation strategy, we can use the proxy-side default.

I think it will be better to have this compatible and use the proxy default, rather than require all language supports to be updated straight away. There's also an older issue on configurable entity passivation (#111) which describes having the passivation strategy defined in the Kubernetes CRD or ConfigMap and injected by the operator — which would allow the proxy-side default to be configured, if the language support doesn't define it.

ralphlaude commented 3 years ago

Since it's an additional field, the protocol can be compatible with older versions. In the proxy, instead of throwing exceptions if the language support doesn't define a passivation strategy, we can use the proxy-side default.

I think it will be better to have this compatible and use the proxy default, rather than require all language supports to be updated straight away. There's also an older issue on configurable entity passivation (#111) which describes having the passivation strategy defined in the Kubernetes CRD or ConfigMap and injected by the operator — which would allow the proxy-side default to be configured, if the language support doesn't define it.

The proxy-default for passivation strategy will be introduced again to allow the compatibility so we can move forward. I think it would better in the middle term to have a central place where to configure things like passivation strategy. Now we have two places where passivation strategy is configured. I think the better place for that is the language support.

ralphlaude commented 3 years ago

Everything is done here. I am not sure if we want to extends the nodejs support here or do it in another issue. If yes, I will create issues for nodejs support and another language support (GO, Spring, Kotlin, Python and others). @sleipnir, @marcellanz what do you think?

marcellanz commented 3 years ago

@ralphlaude I think for changes on the protocol one implementation for a "main" user support language should be enough. With "main" this might be JS or Java I think.

As the protocol changed more often lately, it reveals how we might work with such changes in general. With changes worked on concurrently, having the proxy and TCK in the right state for development leads to a bit of coordination. I think we don't need overly complex rules for that like, as an example, opening issues for all support languages. Perhaps the TCK can help to guide where changes are pending for other support languages. This spans a matrix of combinations for proxy, TCK and support language implementations. The current situation already shows this, with currently a few languages aligned with master, and others partly up to date.

This said, I think support languages should aim to get protocol updates by themselves. If the protocol changes by a PR like this one, the changing PR a) should provide a TCK test or if not applicable, b) a comment in the spec about semantics changed in the protocol.

The TCK Verification here https://observablehq.com/@marcellanz/cloudstate-tck-verification displays the current situation also in an automatic way, with pending TCK tests visible.

WDYT? CC/ @sleipnir @pvlugter

sleipnir commented 3 years ago

@ralphlaude I agree with @marcellanz but I add that the comment on the semantics desired by the protocol must be mandatory in PR. This avoids that developers have to have a long journey of understanding evaluating codes and contracts where a comment would be enough to make the intention clear.

ralphlaude commented 3 years ago

@marcellanz the proposal is fine for me. It would be nice to have support for nodejs. I will comment the spec about the new semantic.

ralphlaude commented 3 years ago

@pvlugter, @jroper I need support here for nodejs. Please take a look.

pvlugter commented 3 years ago

Yes, would be good to add passivation timeouts for node support. Node support is behind the Java support in a few areas now: applying events immediately (and any failure handling for this) #375, support for value entities, new TCK implementation, and this. Doesn't need to be added to this PR, let's create an issue to track.

pvlugter commented 3 years ago

Could be useful to have passivation timeouts as part of the TCK for testing.

ralphlaude commented 3 years ago

Could be useful to have passivation timeouts as part of the TCK for testing.

@pvlugter Could you please be more specific?

pvlugter commented 3 years ago

Could be useful to have passivation timeouts as part of the TCK for testing.

@pvlugter Could you please be more specific?

Since passivation strategy is part of the protocol, it would be useful to test it in the TCK. So testing that language supports can configure this, and ideally also testing that the proxy applies the timeout and passivates the entity. For the TCK, it would be fine to have an additional entity defined which is just for testing this, since the passivation timeout is on entity discovery.

ralphlaude commented 3 years ago

@pvlugter ping! you can follow up.

pvlugter commented 3 years ago

I'll just close and reopen this PR to see if we have Travis CI working again.