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 durable archival #3828

Merged
merged 2 commits into from
Jan 25, 2023
Merged

Conversation

MichaelSnowden
Copy link
Contributor

What changed?
This PR enables the archival queue by default.

Why?
To finish the migration to the new archival system.

How did you test it?
I ran manual tests to:

  • Verify that the archival records are created in local file storage
  • They are also created properly in local stack S3
  • I made a branch which has this PR and removes everything for the old inline/workflow-based archival, and I observed that the tests pass, meaning we don't depend on any of that old code: https://buildkite.com/temporal/temporal-public/builds/7417

Potential risks
There are very few users currently using archival, but durable archival is designed to be forwards and backwards-compatible, so migrating existing users should not create any issues. However, the archival queue hasn't been tested on a large scale yet.

Is hotfix candidate?
Yes

@MichaelSnowden MichaelSnowden requested a review from a team as a code owner January 23, 2023 22:41
@@ -410,7 +410,7 @@ func NewConfig(dc *dynamicconfig.Collection, numberOfShards int32, isAdvancedVis
NumArchiveSystemWorkflows: dc.GetIntProperty(dynamicconfig.NumArchiveSystemWorkflows, 1000),
ArchiveRequestRPS: dc.GetIntProperty(dynamicconfig.ArchiveRequestRPS, 300), // should be much smaller than frontend RPS
ArchiveSignalTimeout: dc.GetDurationProperty(dynamicconfig.ArchiveSignalTimeout, 300*time.Millisecond),
DurableArchivalEnabled: dc.GetBoolProperty(dynamicconfig.DurableArchivalEnabled, false),
DurableArchivalEnabled: dc.GetBoolProperty(dynamicconfig.DurableArchivalEnabled, true),
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 want to enable it by default?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes; this won't do anything for people not using archival, but it will migrate users who are to the new system. We'll need to include a notice in the patch notes

@MichaelSnowden MichaelSnowden changed the base branch from snowden/enable-archival-base to master January 24, 2023 23:17
@MichaelSnowden MichaelSnowden enabled auto-merge (squash) January 25, 2023 18:58
@MichaelSnowden MichaelSnowden merged commit 1c638ac into master Jan 25, 2023
@MichaelSnowden MichaelSnowden deleted the snowden/enable-durable-archival branch January 25, 2023 19:18
MichaelSnowden added a commit that referenced this pull request Jan 31, 2023
MichaelSnowden added a commit that referenced this pull request Feb 3, 2023
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