cli --min-packsize and --file-read-concurrency#2671
cli --min-packsize and --file-read-concurrency#2671madsi1m wants to merge 9 commits intorestic:masterfrom
Conversation
|
While I understand the need to tune such options I think they require proper documentation. People tend to tune such options to the extreme which might cause bottlenecks or performance issues and result in a lot of questions and complains on the forum. Hence the documentation should cover these questions:
Edit for reference: https://forum.restic.net/t/control-the-minimal-pack-files-size/617 |
|
Does |
|
--file-read-concurrency doesn't need to be a global option, you are quite right it is only used by the backup command. |
| @@ -0,0 +1,5 @@ | |||
| Feature: Addition of --min-packsize and --file-read-concurrency | |||
There was a problem hiding this comment.
Judging from the changelog template https://github.com/restic/restic/blob/master/changelog/TEMPLATE this should read Add [...] flags
| @@ -0,0 +1,5 @@ | |||
| Feature: Addition of --min-packsize and --file-read-concurrency | |||
|
|
|||
| In order to tune restic for large installs we need to be able to configure --min-packsize and --file-read-concurrency. In version 0.9.6 these numbers are hard coded, this pull request allows you to adjust these. | |||
There was a problem hiding this comment.
The changelog usually doesn't refer explicitly to previous versions. The description should also be written in past tense. There's also no need to mention that this was change via a pull request. (Sorry for the nitpicking)
| f.BoolVar(&globalOptions.CleanupCache, "cleanup-cache", false, "auto remove old cache directories") | ||
| f.IntVar(&globalOptions.LimitUploadKb, "limit-upload", 0, "limits uploads to a maximum rate in KiB/s. (default: unlimited)") | ||
| f.IntVar(&globalOptions.LimitDownloadKb, "limit-download", 0, "limits downloads to a maximum rate in KiB/s. (default: unlimited)") | ||
| f.UintVar(&globalOptions.MinPackSize, "min-packsize", 0, "set min pack size in MiB. (default: $RESTIC_MIN_PACKSIZE or 4)") |
There was a problem hiding this comment.
If you put backticks '`' around the word size then cobra will use that to name the flag parameter.
|
@madsi1m I'd love to see this included. Do you have the bandwidth to finish this up, or would you mind if I took over? |
|
@metalsp0rk sadly I don't have the bandwidth at the moment. Go for it. Would be great to have this in master 👍 |
|
No worries, I just didn't want to step on any toes. 👍 |
|
I've submitted a new PR for this, #2750 |
|
cool |
|
What about closing this now that #2750 is almost finished? |
|
Closing this in favor of #2750. |
What is the purpose of this change? What does it change?
Would like to add the following in order to tune restic rather than hardcoding it.
--min-packsize
--file-read-concurrency
Was the change discussed in an issue or in the forum before?
Nope
Checklist
changelog/unreleased/that describes the changes for our users (template here)gofmton the code in all commits