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

FEATURE PREVIEW: Add Actor State TTL support #482

Closed
JoshVanL opened this issue Apr 25, 2023 · 13 comments
Closed

FEATURE PREVIEW: Add Actor State TTL support #482

JoshVanL opened this issue Apr 25, 2023 · 13 comments
Assignees
Labels
awaiting dapr/dapr Awaiting a feature from Dapr/Dapr to become available enhancement-runtime-dependency New feature or request due to a runtime feature stale

Comments

@JoshVanL
Copy link

JoshVanL commented Apr 25, 2023

Add first class API support for Actor State TTL dapr/dapr#5899

A dedicated function to Actor State should be created. Psudo code: func (key string, value string, ttl duration). It should be made clear in the SDK documentation/comments that users should always use this function, unless they have specifically created some kind of Actor State clean up out of band of Dapr or don't have an issue with the Actor State store keep state and grow "forever".

Please see the go-sdk for implementation reference.


UPDATE: Due to the nature of the current implementation of write through caching of actor state and the unavailability of the real TTL expire time of state keys, SDKs will have an inconsistent view of the world when it has a cold cache and the state store has TTL keys. The TTL functionality has been put behind a feature gate in daprd. See dapr/dapr#6400 for more details. This feature is expected to be moved to GA in 1.12.

@shubham1172 shubham1172 added the enhancement-runtime-dependency New feature or request due to a runtime feature label May 2, 2023
@salmankhan-prs
Copy link
Contributor

salmankhan-prs commented May 2, 2023

HI @shubham1172 for the above issue We need to create the separate function in src/actors/runtime/ActorStateManager.ts
right?

can you go through these function

async setStateWithTTL(stateName: string, value: T, ttl: number): Promise<void> {
  const stateChangeTracker = this.getContextualStateTracker();

  if (stateChangeTracker.has(stateName)) {
    const stateMetadata = stateChangeTracker.get(stateName);

    if (!stateMetadata) {
      return;
    }

    stateMetadata.setValue(value);
    stateMetadata.setTTL(ttl);

    if (
      stateMetadata.getChangeKind() === StateChangeKind.NONE ||
      stateMetadata.getChangeKind() === StateChangeKind.REMOVE
    ) {
      stateMetadata.setChangeKind(StateChangeKind.UPDATE);
    }

    stateChangeTracker.set(stateName, stateMetadata);

    return;
  }

  const didExist = await this.actor
    .getStateProvider()
    .containsState(this.actor.getActorType(), this.actor.getActorId(), stateName);

  if (didExist) {
    stateChangeTracker.set(stateName, new StateMetadata(value, StateChangeKind.UPDATE, ttl));
  } else {
    stateChangeTracker.set(stateName, new StateMetadata(value, StateChangeKind.ADD, ttl));
  }
}

@salmankhan-prs
Copy link
Contributor

@shubham1172 any update on this ?

@shubham1172
Copy link
Member

Hi @salmankhan-prs, yes, you need to create this function in ActorStateManager. It LGTM from a high-level. Since setStateWithTTL is also a generalized version of setState, we can also explore reusing this implementation (e.g., if TTL < 0, don't set it. setState then calls setStateWithTTL(name, val, -1).

@salmankhan-prs
Copy link
Contributor

@shubham1172 you want setState to call setStateWithTTL internally

 async setState(stateName: string, value: T): Promise<void> {
  return await setStateWithTTL(name, val, -1).
 }

@salmankhan-prs
Copy link
Contributor

@shubham1172 Is the above change is want we required or anything else we are looking for?

@shubham1172
Copy link
Member

@salmankhan-prs yes, please feel free to create a PR, let me assign this issue to you.

@salmankhan-prs
Copy link
Contributor

@shubham1172 I made PR.please verify it

@salmankhan-prs
Copy link
Contributor

@shubham1172 can you help me to find out where I can write the test case for setStateWithTTL if It should be in test/actors/DemoActorActivateImpl.ts .can you also give a brief idea of how to write the test case It's somewhat difficult to me understand the testing approach of actors.

@shubham1172
Copy link
Member

Taking this conversation to the pull request - @salmankhan-prs.

@shubham1172 shubham1172 added the awaiting dapr/dapr Awaiting a feature from Dapr/Dapr to become available label May 10, 2023
@JoshVanL JoshVanL changed the title Add Actor State TTL support FEATURE PREVIEW: Add Actor State TTL support May 25, 2023
@JoshVanL
Copy link
Author

@salmankhan-prs @shubham1172 I have update the issue description. For v1.11, This feature now requires the ActorStateTTL feature gate to be enabled in daprd.

@artursouza
Copy link
Member

Can this be added in the next release?

@dapr-bot
Copy link
Collaborator

This issue has been automatically marked as stale because it has not had activity in the last 60 days. It will be closed in the next 7 days unless it is tagged (pinned, good first issue, help wanted or triaged/resolved) or other activity occurs. Thank you for your contributions.

@dapr-bot dapr-bot added the stale label Aug 28, 2023
@dapr-bot
Copy link
Collaborator

dapr-bot commented Sep 4, 2023

This issue has been automatically closed because it has not had activity in the last 67 days. If this issue is still valid, please ping a maintainer and ask them to label it as pinned, good first issue, help wanted or triaged/resolved. Thank you for your contributions.

@dapr-bot dapr-bot closed this as completed Sep 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting dapr/dapr Awaiting a feature from Dapr/Dapr to become available enhancement-runtime-dependency New feature or request due to a runtime feature stale
Projects
None yet
Development

No branches or pull requests

5 participants