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

fix(ci): Expand cached state disks before running tests #4962

Merged
merged 6 commits into from
Aug 28, 2022

Conversation

teor2345
Copy link
Contributor

@teor2345 teor2345 commented Aug 27, 2022

Motivation

In PR #4945, we increased the cached state disk size to 200GB. But that only applies to newly created cached states. (Which take 24 hours to run, and still need fixes like PR #4961.)

So we want to expand older cached state disks to the latest size before using them.

Also fixes #4295.

Solution

  • Grow the disk partition to the full disk size
  • Resize the filesystem to the full partition size
  • Actually run the test

Related fixes:

Review

This is urgent because it's stopping all the other PRs from passing.

Reviewer Checklist

  • CI passes

@teor2345 teor2345 added C-bug Category: This is a bug A-devops Area: Pipelines, CI/CD and Dockerfiles P-Critical 🚑 I-heavy Problems with excessive memory, disk, or CPU usage I-integration-fail Continuous integration fails, including build and test failures labels Aug 27, 2022
@teor2345 teor2345 self-assigned this Aug 27, 2022
@teor2345 teor2345 requested a review from a team as a code owner August 27, 2022 03:11
@teor2345 teor2345 requested review from gustavovalverde and removed request for a team August 27, 2022 03:11
Copy link
Member

@gustavovalverde gustavovalverde left a comment

Choose a reason for hiding this comment

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

This was a great finding! Thank you!

@gustavovalverde
Copy link
Member

This PR is failing as the disk name is too long. I'll be fixing this so we're able to continue with this merge https://github.com/ZcashFoundation/zebra/runs/8050845710?check_suite_focus=true#step:9:75

@teor2345
Copy link
Contributor Author

This PR is failing as the disk name is too long. I'll be fixing this so we're able to continue with this merge https://github.com/ZcashFoundation/zebra/runs/8050845710?check_suite_focus=true#step:9:75

@gustavovalverde I'm happy to fix it, I have a format that will come in under the 63 character limit, but still be unique.

@gustavovalverde
Copy link
Member

Oh, I think we were working on the disk name solution in parallel. I have a PR with a similar solution here: #4966

I don't mind if your prefer your solution, but please take into consideration some of the examples I made there as, following the actual name reference in this PR (${{ inputs.disk_prefix }}-${SHORT_GITHUB_REF}-${{ env.GITHUB_SHA_SHORT }}-v${{ env.STATE_VERSION }}-${{ env.NETWORK }}-${{ inputs.disk_suffix }}${UPDATE_SUFFIX}-${TIME_SUFFIX}) a disk exceeding 63 characters would be possible, for example:

  • zebrad-cache-4962-merge-e098efa-v25-mainnet-checkpoint-update-2022-08-27-14-04-40 (81 characters)

Also, keeping the commit does not seem necessary as explained in #4966 as the GITHUB_REF should be enough to find the latest available disk from the PR/branch

@teor2345
Copy link
Contributor Author

teor2345 commented Aug 28, 2022

Oh, I think we were working on the disk name solution in parallel. I have a PR with a similar solution here: #4966

Sorry about that, but I'm happy to be responsible for fixing this one.

I don't mind if your prefer your solution, but please take into consideration some of the examples I made there as, following the actual name reference in this PR (${{ inputs.disk_prefix }}-${SHORT_GITHUB_REF}-${{ env.GITHUB_SHA_SHORT }}-v${{ env.STATE_VERSION }}-${{ env.NETWORK }}-${{ inputs.disk_suffix }}${UPDATE_SUFFIX}-${TIME_SUFFIX}) a disk exceeding 63 characters would be possible, for example:

  • zebrad-cache-4962-merge-e098efa-v25-mainnet-checkpoint-update-2022-08-27-14-04-40 (81 characters)

This doesn't actually happen with the latest code in this PR, because:

  • the checkpoint is at a fixed height, so it is never updated
  • the update suffix is "-u"
  • the date doesn't have a year or hyphens

So the actual longest branch name is 63 characters:

  • zebrad-cache-4962-merge123-e098efa-v25-mainnet-tip-u-0827140440

Also, keeping the commit does not seem necessary as explained in #4966 as the GITHUB_REF should be enough to find the latest available disk from the PR/branch

Since the branch name is truncated, it's no longer unique, but bug #4842 and accurate search require it to be unique.

mergify bot added a commit that referenced this pull request Aug 28, 2022
@mergify mergify bot merged commit 6fd3cdb into main Aug 28, 2022
@mergify mergify bot deleted the expand-docker-disks branch August 28, 2022 09:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-devops Area: Pipelines, CI/CD and Dockerfiles C-bug Category: This is a bug I-heavy Problems with excessive memory, disk, or CPU usage I-integration-fail Continuous integration fails, including build and test failures
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Truncate branch name when used in Google Cloud names
2 participants