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

Development environment for multi cluster setup #2848

Merged
merged 3 commits into from
May 16, 2022

Conversation

alexshtin
Copy link
Member

What changed?
Add development environment for multi cluster setup:

  1. make start-dependencies-cdc/make stop-dependencies-cdc to run dedicated UIs for standby (port 8081) and other (port 8082) clusters.
  2. make install-schema-cdc to install all schemas, including Elasticsearch for advnaced visibility.
  3. make start-cdc-active, make start-cdc-standby, and make start-cdc-other to run corresponding cluster with advanced visibility enabled.

Also:

  1. Sync all configs.
  2. Renamed development.yaml to development_cass.yaml to prevent config inheritance. Previously development.yaml was always loaded.

Why?
To improve local cross DC devlopment expirience.

How did you test it?
Run locally.

Potential risks
No risks.

Is hotfix candidate?
No.

@alexshtin alexshtin requested a review from a team as a code owner May 14, 2022 00:26
@@ -85,6 +89,8 @@ archival:
filestore:
fileMode: "0666"
dirMode: "0766"
gstorage:
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need this?

Copy link
Member Author

Choose a reason for hiding this comment

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

You know better. I found it in one the configs and just copied it everywhere. Should it be in development_cass-archival.yaml only?

Copy link
Contributor

Choose a reason for hiding this comment

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

My understanding is the dev config for file store and it will ignore the gstorage config. @yycptt Do you have context here?

Copy link
Member

Choose a reason for hiding this comment

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

I think we should keep it. Although the default URI is local file store, the gstorage section may still be used if someone register a namespace with gcp archival URI in local environment for development purpose.

@@ -85,6 +89,8 @@ archival:
filestore:
fileMode: "0666"
dirMode: "0766"
gstorage:
Copy link
Member

Choose a reason for hiding this comment

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

I think we should keep it. Although the default URI is local file store, the gstorage section may still be used if someone register a namespace with gcp archival URI in local environment for development purpose.

URI: "file:///tmp/temporal_vis_archival/development"

publicClient:
hostPort: "localhost:7233"

dynamicConfigClient:
filepath: "config/dynamicconfig/development_es.yaml"
pollInterval: "10s"

Copy link
Member

Choose a reason for hiding this comment

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

Previously when I do --zone es start, this file will be merged with development.yaml and result in using dual visibility manager, which is super confusing...

I like the new approach, just not sure if this will be surprising for others who is currently using dual manager setup.


dynamicConfigClient:
filepath: "config/dynamicconfig/development_es.yaml"
pollInterval: "10s"
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: new line

Copy link
Member Author

Choose a reason for hiding this comment

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

It is not there in any of configs. Why do you want it?

Copy link
Member Author

Choose a reason for hiding this comment

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

It actually does present in few configs. I am removing it from everywhere.

@alexshtin alexshtin enabled auto-merge (squash) May 16, 2022 20:28
@alexshtin alexshtin merged commit b93f878 into temporalio:master May 16, 2022
@alexshtin alexshtin deleted the feature/dev-muti-cluster-setup branch May 16, 2022 21:19
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

Successfully merging this pull request may close these issues.

None yet

4 participants