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.

Sign up for GitHub

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

Jump to bottom

Prevent data duplication on unspill #5936

Merged
merged 5 commits into from
Mar 15, 2022
Merged

Prevent data duplication on unspill #5936

merged 5 commits into from
Mar 15, 2022

Conversation

crusaderky
Copy link
Collaborator

@crusaderky crusaderky commented Mar 11, 2022 •
edited
Loading

@crusaderky crusaderky mentioned this pull request Mar 11, 2022
Merged
@crusaderky crusaderky self-assigned this Mar 11, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Mar 11, 2022 •
edited
Loading

Unit Test Results

       12 files  ±  0         12 suites  ±0   5h 48m 34s ⏱️ + 1m 22s
  2 653 tests +  6    2 572 ✔️ +  8    80 💤  - 1  1 ❌  - 1 
13 021 runs  +16  12 381 ✔️ +19  638 💤  - 2  2 ❌  - 1 

For more details on these failures, see this check.

Results for commit 398783d. ± Comparison against base commit 2fffe74.

♻️ This comment has been updated with latest results.

@crusaderky crusaderky force-pushed the unspill_duplication branch 4 times, most recently from 4bec938 to 7843d4f Compare March 14, 2022 17:52
fjetter
fjetter reviewed Mar 14, 2022
View reviewed changes
Copy link
Member

@fjetter fjetter left a comment

Choose a reason for hiding this comment

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

Very nice! Looking forward to having this in! Changes LGTM once zict is merged, etc.

@crusaderky crusaderky marked this pull request as ready for review March 14, 2022 17:58
crusaderky
crusaderky commented Mar 14, 2022
View reviewed changes
.pre-commit-config.yaml Outdated Show resolved Hide resolved
crusaderky
crusaderky commented Mar 14, 2022
View reviewed changes
distributed/tests/test_spill.py
assert buf.slow.weight_by_key == {"a": psize(a)}
assert_buf(buf, {"bad": bad}, {"a": a})


Copy link
Collaborator Author

Choose a reason for hiding this comment

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

new test start here; everything else in this module is just a refactor

@crusaderky
Copy link
Collaborator Author

Removed blockers

@crusaderky crusaderky merged commit 7a69b5e into dask:main Mar 15, 2022
@crusaderky crusaderky deleted the unspill_duplication branch March 15, 2022 10:51
@crusaderky crusaderky mentioned this pull request Apr 27, 2022
Open
@fjetter fjetter mentioned this pull request Mar 13, 2023
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fjetter fjetter fjetter left review comments

Assignees

@crusaderky crusaderky

Labels
memory
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Spill to disk may cause data duplication
2 participants
@crusaderky @fjetter