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

Rclone: don't rely on cached bandwidth limit in configguard #4120

Merged
merged 2 commits into from
Dec 10, 2024

Conversation

Michal-Leszczynski
Copy link
Collaborator

This PR fixes the way in which configguard checked if bandwidth limit should be updated.
Previously it was updating bandwidth limit with each Rclone call, now it updates it only when it's changed.

This PR also removes unnecessary usages of RcloneSetBandwidthLimit, as handling bandwidth limit should be done by specific Rclone calls.

Fixes #4119

@Michal-Leszczynski
Copy link
Collaborator Author

Example agent logs from TestRestoreFullIntegration before the change:

{"L":"INFO","T":"2024-11-22T10:52:47.258Z","N":"rclone","M":"Bandwidth limit reset to unlimited"}
{"L":"INFO","T":"2024-11-22T10:52:57.568Z","N":"rclone","M":"Bandwidth limit reset to unlimited"}
{"L":"INFO","T":"2024-11-22T10:53:07.829Z","N":"rclone","M":"Bandwidth limit reset to unlimited"}
{"L":"INFO","T":"2024-11-22T10:53:17.901Z","N":"rclone","M":"Bandwidth limit reset to unlimited"}

And after the change:

{"L":"INFO","T":"2024-11-22T10:46:50.445Z","N":"rclone","M":"Bandwidth limit set to {999M 999M}"}

(changed bandwidth limit to 999M because otherwise we wouldn't see any log with bandwidth limit change, but that's expected)

@Michal-Leszczynski Michal-Leszczynski changed the title Ml/fix 4119 Rclone: don't rely on cached bandwidth limit in configguard Nov 22, 2024
@Michal-Leszczynski
Copy link
Collaborator Author

TestPurgeIntegration failed, but it looks flaky and not related to this PR, but I will try to investigate it.

@Michal-Leszczynski
Copy link
Collaborator Author

TestPurgeIntegration failed, but it looks flaky and not related to this PR, but I will try to investigate it.

No luck with that - there is some chance that minio lagged when listing manifests just after their creation.

@Michal-Leszczynski Michal-Leszczynski marked this pull request as ready for review November 27, 2024 15:05
Copy link
Collaborator

@VAveryanov8 VAveryanov8 left a comment

Choose a reason for hiding this comment

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

LGTM

@Michal-Leszczynski
Copy link
Collaborator Author

@karol-kokoszka The PR is ready for review!

@Michal-Leszczynski
Copy link
Collaborator Author

@karol-kokoszka This PR is ready for review! Let's merge it and start the 3.4.1 release process.

@Michal-Leszczynski Michal-Leszczynski merged commit d9dde35 into master Dec 10, 2024
51 checks passed
@Michal-Leszczynski Michal-Leszczynski deleted the ml/fix-4119 branch December 10, 2024 11:17
Michal-Leszczynski added a commit that referenced this pull request Dec 10, 2024
…4120)

* fix(rcserver): don't rely on cached bandwidth limit in configguard

Fixes #4119

* refactor(backup): remove unnecessary calls to RcloneSetBandwidthLimit

Setting bandwidth limit should be handled by specific Rclone calls.

(cherry picked from commit d9dde35)
Michal-Leszczynski added a commit that referenced this pull request Dec 10, 2024
…4120)

* fix(rcserver): don't rely on cached bandwidth limit in configguard

Fixes #4119

* refactor(backup): remove unnecessary calls to RcloneSetBandwidthLimit

Setting bandwidth limit should be handled by specific Rclone calls.

(cherry picked from commit d9dde35)
Michal-Leszczynski added a commit that referenced this pull request Dec 11, 2024
…4120)

* fix(rcserver): don't rely on cached bandwidth limit in configguard

Fixes #4119

* refactor(backup): remove unnecessary calls to RcloneSetBandwidthLimit

Setting bandwidth limit should be handled by specific Rclone calls.

(cherry picked from commit d9dde35)
Michal-Leszczynski added a commit that referenced this pull request Dec 11, 2024
…4120)

* fix(rcserver): don't rely on cached bandwidth limit in configguard

Fixes #4119

* refactor(backup): remove unnecessary calls to RcloneSetBandwidthLimit

Setting bandwidth limit should be handled by specific Rclone calls.

(cherry picked from commit d9dde35)
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.

Don't reset rclone rate limit when not needed
3 participants