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

Enable redis server as coordination backend for platform development #5052

Closed
wants to merge 2 commits into from

Conversation

m4dcoder
Copy link
Contributor

@m4dcoder m4dcoder commented Oct 5, 2020

A lot of workflow use cases that contain parallel branches, join task, and with items task require coordination service to be configured to be executed properly. For platform development, setup coordination service using redis server as backend by default to avoid confusion in troubleshooting and debugging workflows.

@pull-request-size pull-request-size bot added the size/XS PR that changes 0-9 lines. Quick fix/merge. label Oct 5, 2020
@m4dcoder m4dcoder added this to the 3.4.0 milestone Oct 5, 2020
@punkrokk
Copy link
Member

punkrokk commented Oct 5, 2020 via email

@m4dcoder
Copy link
Contributor Author

m4dcoder commented Oct 5, 2020

@punkrokk I'm not sure I know what you mean. Coordination service is really a requirement given the number of workflow use cases that depend on it to work correctly. The only workflow use case that does not require coordination service are sequential workflows. For sequential workflows, the workflow engine does not invoke the coordination service whether it is configured or not. So configuring the coordination service has no bearing on testing these workflows.

@amanda11
Copy link
Contributor

amanda11 commented Oct 6, 2020

I think in the documentation at the moment then it states that the coordination service is only required for the concurrency policy or HA.
I don't think it states it is needed for the other examples that have been listed, so do we also need a doc update?

And then does it also follow that if it's used in dev, should we have a default coordination service installed by one liner installer and others?

@arm4b
Copy link
Member

arm4b commented Oct 6, 2020

Indeed coordination was really documented to be a requirement for Policies and HA only.

If the reality now that [coordination] is required for any normal Orquesta Workflow execution, - apart of just documentation update it should be added to the default StackStorm architecture and all the installers/deployment methods we have.

@m4dcoder
Copy link
Contributor Author

m4dcoder commented Oct 7, 2020

It really is required for any non-sequential workflows to function properly. The documentation should be updated. We will save a lot of user headaches just including a coordination service backend as part of the StackStorm install. For this PR here though, which is specifically to ensure development environment is setup properly at least, I'm proposing redis server as the backend.

@guzzijones
Copy link
Contributor

YES omg YES. I have been wanting this as well. Orquesta workflows don't work correctly if they are non-sequential without a coordination backend.

@pull-request-size pull-request-size bot added size/S PR that changes 10-29 lines. Very easy to review. and removed size/XS PR that changes 0-9 lines. Quick fix/merge. labels Oct 22, 2020
@pull-request-size pull-request-size bot added size/XS PR that changes 0-9 lines. Quick fix/merge. and removed size/S PR that changes 10-29 lines. Very easy to review. labels Oct 22, 2020
@pull-request-size pull-request-size bot added size/S PR that changes 10-29 lines. Very easy to review. and removed size/XS PR that changes 0-9 lines. Quick fix/merge. labels Oct 22, 2020
A lot of workflow use cases that contain parallel branches, join task, and with items task require coordination service to be configured to be executed properly. For platform development, setup coordination service using redis server as backend by default to avoid confusion in troubleshooting and debugging workflows.
@punkrokk
Copy link
Member

Ok, I see your point. I am also seeing situations where concurrency is enabled absent coordination and the scheduler service is very unhappy. (@m4dcoder )

Copy link
Member

@arm4b arm4b left a comment

Choose a reason for hiding this comment

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

Per team feedback and as a follow-up,

The change here is part of the whole story of enabling [coordination] backend across the StackStorm fleet. From what I understand the following might be involved:

  • documentation update signalizing that [coordination] is now required/enabled
  • scripted installer changes to include Redis in default installation

That sounds like a major item for a release.
I believe we all care about the user experience making sure Orquesta workflows are executing fine in production, not only in dev env.

@m4dcoder Do you think you can target next v3.5.0 release with these major set of changes?

Copy link
Member

@cognifloyd cognifloyd left a comment

Choose a reason for hiding this comment

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

Once merge conflicts are resolved, LGTM.

Copy link
Member

@arm4b arm4b left a comment

Choose a reason for hiding this comment

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

I believe we'll need to include redis in main pip requirements https://github.com/StackStorm/st2/blob/master/requirements.txt

Besides of that, it's all good as the target is to ship the Redis as a default st2 coordination backend in v3.5.0 👍

@punkrokk
Copy link
Member

punkrokk commented Apr 1, 2021 via email

@m4dcoder
Copy link
Contributor Author

m4dcoder commented Apr 8, 2021

Replaced this PR with #5226.

@m4dcoder m4dcoder closed this Apr 8, 2021
@m4dcoder m4dcoder deleted the redis-dev-default branch April 8, 2021 19:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size/S PR that changes 10-29 lines. Very easy to review.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants