Skip to content

Serialize shard data #6869

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

Merged
merged 2 commits into from
May 5, 2025
Merged

Serialize shard data #6869

merged 2 commits into from
May 5, 2025

Conversation

Shaddoll
Copy link
Member

@Shaddoll Shaddoll commented Apr 29, 2025

What changed?

  • Serialized shard data into a blob and store it into data column in Cassandra
  • Introduce a feature flag to control where to read data from
  • Fix a bug in GetShard method for SQL implementation

Why?
To migrate shard data from Cassandra typed column to serialized blob column so that we don't need to update our schema when introducing new field to shard data. This is a preparation work for history queue revamp.

How did you test it?
unit tests and integration tests

Potential risks

Release notes

Documentation Changes

@@ -48,6 +48,7 @@ var (
dynamicproperties.EnableConsistentQueryByDomain: true,
dynamicproperties.MinRetentionDays: 0,
dynamicproperties.WorkflowDeletionJitterRange: 1,
dynamicproperties.ReadNoSQLShardFromDataBlob: true,
Copy link
Member

Choose a reason for hiding this comment

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

should we make this false for some tests to have some coverage for current implementation?

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not sure the best way to handle this. I have to run the integration tests with this flag on to test the change before turning it on in staging environments.

Copy link
Member

Choose a reason for hiding this comment

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

We have this problem in general for dynamic configs. Since we cannot run separate integration tests for each cartesian product of config values we go with a pre-picked values. I recommend adding at least one basic test that uses the previous value. We need to have coverage for both

Copy link
Member Author

Choose a reason for hiding this comment

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

Added a new test in host/persistence package for Cassandra.

@Shaddoll Shaddoll force-pushed the p1 branch 4 times, most recently from 9ecc5f6 to 1680590 Compare May 1, 2025 21:11
@Shaddoll Shaddoll requested a review from taylanisikdemir May 1, 2025 21:19
@Shaddoll Shaddoll merged commit 3905d66 into cadence-workflow:master May 5, 2025
23 checks passed
@Shaddoll Shaddoll deleted the p1 branch May 5, 2025 19:25
fimanishi pushed a commit to fimanishi/cadence that referenced this pull request May 9, 2025
* Serialize shard data

* Introduce metrics client to nosql stores
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.

2 participants