Skip to content
This repository has been archived by the owner on Feb 15, 2024. It is now read-only.

Check plot description size in advance #89

Merged
merged 5 commits into from
Dec 7, 2022
Merged

Check plot description size in advance #89

merged 5 commits into from
Dec 7, 2022

Conversation

i1i1
Copy link
Contributor

@i1i1 i1i1 commented Dec 6, 2022

Following suggestion of @nazar-pc of how to check minimal plot size

@i1i1 i1i1 added the farmer label Dec 6, 2022
@i1i1 i1i1 requested a review from ozgunozerk December 6, 2022 11:05
@i1i1 i1i1 self-assigned this Dec 6, 2022
ozgunozerk
ozgunozerk previously approved these changes Dec 6, 2022
src/farmer.rs Show resolved Hide resolved
src/farmer.rs Outdated
impl PlotDescription {
const SECTOR_OVERHEAD: ByteSize = ByteSize::mb(1);
Copy link
Member

Choose a reason for hiding this comment

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

Looks like a random value. We have fixed prefix +metadata size in faming components, but nothing for databases yet.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

How can I calculate the prefix size though?

Copy link
Member

@nazar-pc nazar-pc Dec 6, 2022

Choose a reason for hiding this comment

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

It is a fixed size reserved space to allow for metadata upgrades in the future:
https://github.com/subspace/subspace/blob/d06ad0bad8a9cac203d0e0a9b27611b9f0f48e2a/crates/subspace-farmer/src/single_disk_plot.rs#L56-L57
Was supposed to be an implementation detail, I'll think about how to expose it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay. I'll add a todo to account for that. Thanks!

Copy link
Member

@nazar-pc nazar-pc left a comment

Choose a reason for hiding this comment

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

Not sure why cache size has minimum amount of space though, looks a bit strange to me

@@ -86,37 +87,45 @@ impl CacheDescription {
}

/// Description of the plot
Copy link
Member

Choose a reason for hiding this comment

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

BTW, there is a data structure with details about the plot in farmer already and it has more details in it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@i1i1 i1i1 merged commit d396016 into master Dec 7, 2022
@i1i1 i1i1 deleted the check-plot branch December 7, 2022 07:12
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants