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

Migration Critical Downloads #717

Merged
merged 18 commits into from
Nov 16, 2023
Merged

Migration Critical Downloads #717

merged 18 commits into from
Nov 16, 2023

Conversation

peterjan
Copy link
Member

@peterjan peterjan commented Nov 3, 2023

This PR attempts to improve the behaviour of renterd when it comes to the migration of critically low-health slabs in an attempt to avoid file loss at all costs. We found that from time to time we fail to migrate (low-health) slabs due to some hosts that are price gouging.

This PR introduces an extra gouging setting (MigrationSurchargeMultiplier) that essentially says "when it's really necessary, I allow overpaying by a factor of x for some critical sector downloads that would otherwise be the cause of migration failure and potential file loss".

I added two alerts that bubble up when necessary to notify the user his gouging settings are either working, or need to be adjusted. I had to refactor the download code a bit, I tried to keep the refactoring to a minimum and would like to keep it more or less like this to avoid needlessly falling into a huge download refactor rabbit-hole right before 1.0

@peterjan peterjan changed the title Critical Migrations Migration Critical Downloads Nov 3, 2023
@peterjan peterjan self-assigned this Nov 8, 2023
@peterjan peterjan marked this pull request as ready for review November 8, 2023 11:21
@peterjan
Copy link
Member Author

peterjan commented Nov 8, 2023

pushing it into review, thinking of adding a test for this but it's obviously a little tricky to write a solid integration test for this

internal/testing/gouging_test.go Outdated Show resolved Hide resolved
api/worker.go Outdated Show resolved Hide resolved
stores/metadata.go Outdated Show resolved Hide resolved
worker/gouging.go Show resolved Hide resolved
worker/download.go Outdated Show resolved Hide resolved
worker/download.go Outdated Show resolved Hide resolved
@@ -140,6 +140,10 @@ func (sb HostScoreBreakdown) String() string {
return fmt.Sprintf("Age: %v, Col: %v, Int: %v, SR: %v, UT: %v, V: %v, Pr: %v", sb.Age, sb.Collateral, sb.Interactions, sb.StorageRemaining, sb.Uptime, sb.Version, sb.Prices)
}

func (hgb HostGougingBreakdown) DownloadGouging() bool {
Copy link
Member Author

Choose a reason for hiding this comment

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

leaving comment to prevent merge - should extend UI to make sure we can set the surcharge multiplier

@peterjan
Copy link
Member Author

@ChrisSchinnerl ready for another 👀

@peterjan peterjan merged commit 780a5eb into master Nov 16, 2023
6 checks passed
@peterjan peterjan deleted the pj/low-health branch November 16, 2023 09:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

2 participants