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

Adding mutable metadata and cluster id #803

Merged
merged 13 commits into from
Oct 8, 2020

Conversation

vitarb
Copy link
Contributor

@vitarb vitarb commented Oct 6, 2020

This change allows us to have have unique stable cluster id stored in the database and also provides ability to store any other cluster specific data alongside without having to fear that we will override immutable data.
It also allows changing mutability of certain fields in code when needed.

Previously all cluster metadata was immutable containing only cluster name (from the config) and shard count. This limited its usability as it was impossible to add new fields (e.g. cluster id) to it as well as store mutable components like latest available version.

In the first attempt I've tried to add another blob alongside immutable metadata but quickly realized that keeping it would result in messy code due to lack of clear boundary between mutable and immutable.
As a result I've decided to refactor existing implementation and move immutability check on application layer at the same time adding versioning to the storage.

Change was tested locally using persistence test and manually on cassandra/mysql.

Copy link
Contributor

@wxing1292 wxing1292 left a comment

Choose a reason for hiding this comment

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

Copy link
Contributor

@wxing1292 wxing1292 left a comment

Choose a reason for hiding this comment

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

plz add some tests before landing

schema/mysql/v57/temporal/schema.sql Outdated Show resolved Hide resolved
@vitarb vitarb removed the request for review from mastermanu October 8, 2020 00:08
@vitarb vitarb merged commit b63d577 into temporalio:master Oct 8, 2020
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

3 participants