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

backupccl: make checkpoint interval configurable #83151

Merged

Conversation

stevendanna
Copy link
Collaborator

This PR makes the interval between checkpoints configurable and also
excludes the processing time of the checkpoint itself from that
interval.

The goal of this change is to potentially address issues we've seen in
large clusters that we currently believe can be attributed to the
backup process slowing down substantially once it takes a minute or
longer to marshall, compress, and write the progress checkpoint.

Release note (ops change): A new setting
bulkio.backup.checkpoint_interval controls the minimum interval
between writes of progress checkpoints to external storage.

@stevendanna stevendanna requested a review from a team June 21, 2022 20:16
@stevendanna stevendanna requested a review from a team as a code owner June 21, 2022 20:16
@stevendanna stevendanna requested review from livlobo and removed request for a team June 21, 2022 20:16
@cockroach-teamcity
Copy link
Member

This change is Reviewable

@stevendanna stevendanna requested review from adityamaru and benbardin and removed request for livlobo June 21, 2022 20:17
Copy link
Collaborator

@benbardin benbardin left a comment

Choose a reason for hiding this comment

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

This is splendid! Thanks!

What are the backport plans for this? Maybe we want to backport the change to where lastCheckpoint is determined, but not necessarily the cluster setting?

@stevendanna
Copy link
Collaborator Author

@benbardin I think we need to backport the setting as well since we plan on shipping the setting to a customer and we want them to be able to upgrade to a real release once it is available.

@stevendanna stevendanna force-pushed the configurable-checkpoint-interval branch from 99d286a to d235fe1 Compare June 21, 2022 22:36
defer leaktest.AfterTest(t)()
defer log.Scope(t).Close(t)

skip.WithIssue(t, 33357)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This test has been skipped since 2019 and this issue has been closed.

This PR makes the interval between checkpoints configurable and also
excludes the processing time of the checkpoint itself from that
interval.

The goal of this change is to potentially address issues we've seen in
large clusters that we currently believe can be attributed to the
backup process slowing down substantially once it takes a minute or
longer to marshall, compress, and write the progress checkpoint.

Release note (ops change): A new setting
`bulkio.backup.checkpoint_interval` controls the minimum interval
between writes of progress checkpoints to external storage.
@stevendanna stevendanna force-pushed the configurable-checkpoint-interval branch from d235fe1 to 76f5a09 Compare June 21, 2022 23:35
@stevendanna
Copy link
Collaborator Author

TFTR!

bors r=adityamaru,benbardin

@craig
Copy link
Contributor

craig bot commented Jun 22, 2022

Build succeeded:

@craig craig bot merged commit 76cbd88 into cockroachdb:master Jun 22, 2022
@blathers-crl
Copy link

blathers-crl bot commented Jun 22, 2022

Encountered an error creating backports. Some common things that can go wrong:

  1. The backport branch might have already existed.
  2. There was a merge conflict.
  3. The backport branch contained merge commits.

You might need to create your backport manually using the backport tool.


error creating merge commit from 76f5a09 to blathers/backport-release-22.1-83151: POST https://api.github.com/repos/cockroachdb/cockroach/merges: 409 Merge conflict []

you may need to manually resolve merge conflicts with the backport tool.

Backport to branch 22.1.x failed. See errors above.


🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is otan.

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.

4 participants