Skip to content

fix: use memory friendly file split logic#2264

Merged
Racer159 merged 11 commits intozarf-dev:mainfrom
daniel-s-palmer:fix-memory-issue
Feb 7, 2024
Merged

fix: use memory friendly file split logic#2264
Racer159 merged 11 commits intozarf-dev:mainfrom
daniel-s-palmer:fix-memory-issue

Conversation

@daniel-s-palmer
Copy link
Copy Markdown
Contributor

@daniel-s-palmer daniel-s-palmer commented Jan 28, 2024

Description

When splitting a tarball using --max-package-size, zarf will load the entire tarball into memory and copy it to new slices before writing the new files to disk resulting in memory usage up to 2x of the tarball. This can result in large memory usage when the package tarball is large itself.

This PR updates the logic to read the original tarball in chunks of 16 MiB at a time thereby fixing the memory issue.

Testing

Will be breaking this down into a few sections:

  1. Test setup
  2. Test off of main
  3. Test off of PR

Test setup

We will be reusing the TestMultiPartPackage test and updating src\test\packages\05-multi-part\zarf.yaml to the following to test a larger package size and setting --max-package-size=500:

kind: ZarfPackageConfig
metadata:
  name: multi-part
components:
  - name: big-ol-file
    required: true
    description: test larger package around 2GB
    images:
      - atlassian/confluence-server:8.6.0-jdk17
      - atlassian/confluence-server:8.5.1-jdk17

We will expect the test to fail as the number of expected split files will no longer match the original test, however we will be able to see the memory usage while the test runs.

We will capture memory usage using top | grep zarf.

Test off of main

Before we look at the results from the code changes, lets first look at the results from main for comparison.

zarf-2024-01-27-23-31-37-3566031216.log

image

When the package tarball is being split, memory tops out at 2.1g

Test off of PR

zarf-2024-01-27-23-42-37-480658840.log

image

When the package tarball is being split, memory remains steady at under 250m

Related Issue

Fixes #2231

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Other (security config, docs update, etc)

Checklist before merging

@netlify
Copy link
Copy Markdown

netlify Bot commented Jan 28, 2024

Deploy Preview for zarf-docs canceled.

Name Link
🔨 Latest commit 51d0d64
🔍 Latest deploy log https://app.netlify.com/sites/zarf-docs/deploys/65c3ee660f222b00081ad741

Comment thread src/pkg/utils/io.go
Comment thread src/pkg/utils/io.go
daniel-s-palmer and others added 3 commits January 29, 2024 21:37
Comment thread src/pkg/utils/io.go Outdated
Comment thread src/pkg/packager/common.go Outdated
Co-authored-by: Wayne Starr <Racer159@users.noreply.github.com>
Copy link
Copy Markdown
Contributor

@Racer159 Racer159 left a comment

Choose a reason for hiding this comment

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

Sorry for the delay, one tiny change on the messaging but should be GTG after that

Comment thread src/pkg/utils/io.go Outdated
Copy link
Copy Markdown
Contributor

@Racer159 Racer159 left a comment

Choose a reason for hiding this comment

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

lgtm

@Racer159 Racer159 merged commit 68889b0 into zarf-dev:main Feb 7, 2024
TristanHoladay referenced this pull request in defenseunicorns/uds-core Feb 15, 2024
[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Update | Change |
|---|---|---|
| [defenseunicorns/zarf](https://github.com/defenseunicorns/zarf) |
patch | `v0.32.2` -> `v0.32.3` |

---

### Release Notes

<details>
<summary>defenseunicorns/zarf (defenseunicorns/zarf)</summary>

###
[`v0.32.3`](https://github.com/defenseunicorns/zarf/releases/tag/v0.32.3)

[Compare
Source](https://github.com/defenseunicorns/zarf/compare/v0.32.2...v0.32.3)

##### What's Changed

##### Fixes

- Properly handle panic that could occur during checksum validation by
[@&#8203;mjnagel](https://github.com/mjnagel) in
[https://github.com/defenseunicorns/zarf/pull/2262](https://github.com/defenseunicorns/zarf/pull/2262)
- Add the `--key` flag to the init cmd to properly allow for signed init
packages by [@&#8203;dgershman](https://github.com/dgershman) in
[https://github.com/defenseunicorns/zarf/pull/2259](https://github.com/defenseunicorns/zarf/pull/2259)
- Restore destroy script functionality during `zarf destroy` by
[@&#8203;Racer159](https://github.com/Racer159) in
[https://github.com/defenseunicorns/zarf/pull/2274](https://github.com/defenseunicorns/zarf/pull/2274)
- Fix symlink inclusion within component resources by
[@&#8203;dgershman](https://github.com/dgershman) in
[https://github.com/defenseunicorns/zarf/pull/2256](https://github.com/defenseunicorns/zarf/pull/2256)
- Use memory friendly file split logic for partial packages by
[@&#8203;daniel-palmer-gu](https://github.com/daniel-palmer-gu) in
[https://github.com/defenseunicorns/zarf/pull/2264](https://github.com/defenseunicorns/zarf/pull/2264)
- Fix reproducible tarball creation on Windows systems by
[@&#8203;Noxsios](https://github.com/Noxsios) in
[https://github.com/defenseunicorns/zarf/pull/2293](https://github.com/defenseunicorns/zarf/pull/2293)

##### Docs

- Make branding more consistent and add community meetup references to
docs by [@&#8203;Racer159](https://github.com/Racer159) in
[https://github.com/defenseunicorns/zarf/pull/2258](https://github.com/defenseunicorns/zarf/pull/2258)

##### Dependencies

- Update github.com/anchore/clio digest by
[@&#8203;renovate](https://github.com/renovate) in
[https://github.com/defenseunicorns/zarf/pull/2277](https://github.com/defenseunicorns/zarf/pull/2277)
and
[https://github.com/defenseunicorns/zarf/pull/2283](https://github.com/defenseunicorns/zarf/pull/2283)
- Update all non-major dependencies (including Gitea v1.21.5, Syft
v0.100.0, K9s v0.31.7 and Crane v0.19.0) by
[@&#8203;renovate](https://github.com/renovate) in
[https://github.com/defenseunicorns/zarf/pull/2187](https://github.com/defenseunicorns/zarf/pull/2187)

##### Development

- Add a more robust chart search regexManager by
[@&#8203;Racer159](https://github.com/Racer159) in
[https://github.com/defenseunicorns/zarf/pull/2278](https://github.com/defenseunicorns/zarf/pull/2278)
and
[https://github.com/defenseunicorns/zarf/pull/2284](https://github.com/defenseunicorns/zarf/pull/2284)
- Partial refactor of injector logic in `k8s`, and `cluster` packages by
[@&#8203;chrishorton](https://github.com/chrishorton) in
[https://github.com/defenseunicorns/zarf/pull/2271](https://github.com/defenseunicorns/zarf/pull/2271)

##### New Contributors

- [@&#8203;daniel-palmer-gu](https://github.com/daniel-palmer-gu) made
their first contribution in
[https://github.com/defenseunicorns/zarf/pull/2264](https://github.com/defenseunicorns/zarf/pull/2264)

**Full Changelog**:
zarf-dev/zarf@v0.32.2...v0.32.3

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined),
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you
are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View
repository job log
[here](https://developer.mend.io/github/defenseunicorns/uds-core).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy4xNzMuMCIsInVwZGF0ZWRJblZlciI6IjM3LjE3My4wIiwidGFyZ2V0QnJhbmNoIjoibWFpbiJ9-->

---------

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: TristanHoladay <40547442+TristanHoladay@users.noreply.github.com>
Co-authored-by: Micah Nagel <micah.nagel@defenseunicorns.com>
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.

Large memory usage when using --max-package-size

3 participants