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

Replace opts.NumFiles with opts.MaxFileSize #94

Merged
merged 5 commits into from
Dec 14, 2022
Merged

Replace opts.NumFiles with opts.MaxFileSize #94

merged 5 commits into from
Dec 14, 2022

Conversation

moshababo
Copy link
Contributor

Part of spacemeshos/go-spacemesh#3529.

Remarks:

  • Default opts.MaxFileSize value is set to 4 GB.
  • Files layout is calculated according to the config and options to accommodate the max size. The last file size might be lower than other files.
  • Changing the param value after data was initialized is currently not supported.
  • Changing opts.NumUnits after data was initialized is supported, and might involve adding new files or removing existing ones.

@moshababo moshababo requested a review from fasmat December 12, 2022 07:49
poszu
poszu reviewed Dec 12, 2022
View reviewed changes
config/layout.go Outdated Show resolved Hide resolved
initialization/initialization.go Show resolved Hide resolved
initialization/initialization.go Outdated Show resolved Hide resolved
initialization/initialization_test.go Outdated Show resolved Hide resolved
fasmat
fasmat requested changes Dec 12, 2022
View reviewed changes
config/layout.go Outdated Show resolved Hide resolved
persistence/info.go Outdated Show resolved Hide resolved
shared/util.go Outdated Show resolved Hide resolved
initialization/initialization_test.go Outdated Show resolved Hide resolved
initialization/initialization_test.go Outdated Show resolved Hide resolved
initialization/initialization_test.go Outdated Show resolved Hide resolved
initialization/initialization.go Outdated Show resolved Hide resolved
initialization/initialization.go Outdated Show resolved Hide resolved
initialization/initialization.go Show resolved Hide resolved
initialization/diskstate.go Outdated Show resolved Hide resolved
@fasmat fasmat self-requested a review December 12, 2022 17:08
fasmat
fasmat requested changes Dec 12, 2022
View reviewed changes
Copy link
Member

@fasmat fasmat left a comment

Choose a reason for hiding this comment

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

I think we should re-add the check that NumUnits doesn't increase. At least until we have implemented the necessary changes to gpu-post to allow for proper amortized work.

@moshababo
Copy link
Contributor Author

It was previously supported, but only limited due to opts.NumFiles complications and allowed if opts.NumFiles=1. Those complications don't exist with opts.MaxFileSize, but I now disallowed it following #94 (comment).

@moshababo moshababo requested a review from fasmat December 13, 2022 10:08
@fasmat
Copy link
Member

fasmat commented Dec 13, 2022

LGTM

@moshababo moshababo merged commit 7e13dca into develop Dec 14, 2022
@moshababo moshababo deleted the max_file_size branch December 14, 2022 07:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@poszu poszu poszu left review comments

@fasmat fasmat fasmat approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@moshababo @fasmat @poszu