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

Support reset reapply exclude types for batch reset #187

Open
dhiaayachi opened this issue Sep 5, 2024 · 6 comments
Open

Support reset reapply exclude types for batch reset #187

dhiaayachi opened this issue Sep 5, 2024 · 6 comments

Comments

@dhiaayachi
Copy link
Owner

temporalio/api#348 introduced ResetReapplyExcludeType and deprecated ResetReapplyType, and temporalio#5360 implemented support for this in the server.

However, we need to make the corresponding proto api and server changes for batch reset. Specifically, this will probably mean adding a new field reset_reapply_exclude_types, and deprecating reset_reapply_type in ResetOptions.

@dhiaayachi
Copy link
Owner Author

Issue: Proto API and Server Changes for Batch Reset

The pull requests temporalio/api#348 and temporalio#5360 introduced ResetReapplyExcludeType and deprecated ResetReapplyType, adding support for this functionality in the server.

However, we still need to update the corresponding proto API and server changes for batch reset. Specifically, this likely involves adding a new field reset_reapply_exclude_types, and deprecating reset_reapply_type in the ResetOptions message defined in the proto file:

https://github.com/temporalio/api/blob/64ce34ae33a5b56412a6f68edd8f7e1e3a11a80c/temporal/api/common/v1/message.proto#L175

This change is necessary to maintain consistency and allow for batch resets that correctly exclude certain event types.

Here are the proposed changes:

  1. Proto API:

    • Add a new field reset_reapply_exclude_types to the ResetOptions message, which will be a repeated field of type ResetReapplyExcludeType.
    • Deprecate the reset_reapply_type field in ResetOptions.
  2. Server:

    • Implement support for the new reset_reapply_exclude_types field in the batch reset logic.
    • Update the server code to use the new field and deprecate the reset_reapply_type field.

By making these changes, we ensure that the batch reset functionality is updated to use the new ResetReapplyExcludeType enum, allowing for more granular control and preventing any potential issues with the deprecated ResetReapplyType.

@dhiaayachi
Copy link
Owner Author

This looks like a feature request. Thanks for the suggestion!

You can work around this by modifying the batch command to use ResetOptions with the ResetReapplyExcludeType option set to the desired type.

For example, to exclude WorkflowType you could run the following command:

temporal workflow reset-batch --query 'ExecutionStatus = "Running" AND WorkflowType="YourWorkflow"' --reset-options '{"reset_reapply_exclude_types": ["WorkflowType"]}'

@dhiaayachi
Copy link
Owner Author

This is a feature request.

Thank you for the request! There are no explicit limitations to the size of the input to the ResetOptions protobuf message, but large messages can impact performance and memory use. You could add a new field, reset_reapply_exclude_types, to the ResetOptions protobuf message to handle this. While it is not yet implemented, you can use the CLI and the batch interface to reset Workflows, by sending a signal with the --input flag to the temporal workflow signal command:

temporal workflow signal \
  --workflow-id MyWorkflowId \
  --name MySignal \
  --input '{"Input": "As-JSON"}' \
  --query 'ExecutionStatus = "Running" AND WorkflowType="YourWorkflow"' \
  --reason "Testing" 

This will send the signal to all Workflows that match the --query filter.

@dhiaayachi
Copy link
Owner Author

Thanks for reporting this!

This is a feature request to add support for batch reset with the new ResetReapplyExcludeType field. We are currently missing the corresponding proto API and server changes for this feature.

We are aware of this and are working to implement it. You can follow the progress in the linked PRs, and we will update the documentation once this feature is available.

@dhiaayachi
Copy link
Owner Author

This is a feature request. Thank you for your request!

To work around this issue, you could use the ResetReapplyType in the ResetOptions for now. This field is deprecated but still supported.

For more information on ResetOptions please refer to the ResetOptions proto file.

@dhiaayachi
Copy link
Owner Author

Thanks for reporting this!
This is a feature request, you can work around it by using the deprecated reset_reapply_type field in ResetOptions for now.
We are working on implementing support for ResetReapplyExcludeType in the batch reset API. You can follow the progress of this feature request in the issue here.

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

No branches or pull requests

1 participant