Enable --progress-total option using pv. #547
Conversation
There was a problem hiding this comment.
Thanks for the MR, looks promising. I've had a brief look at it, and did not have time to test it yet (I'm quite busy at the moment, this might take a while sorry).
Do you have any numbers on how good are the estimates are and how long it takes to calculate the diffs? I'm especially curious as the btrfs send-stream adds quite some metadata.
btrbk
Outdated
| my @cmd = ( "pv" ); | ||
| push @cmd, ("-s", $total_len); | ||
| push @cmd, "-e -r -p -i 2"; |
There was a problem hiding this comment.
Did you check if mbuffer supports similar functionality? I'd hate to re-introduce pv as a new dependency, even if optional.
see 9dc717c
There was a problem hiding this comment.
mbuffer does not support this unfortunately.
btrbk
Outdated
| sub sub_diff($$) { | ||
| # | ||
| # calc snapshot diff (btrfs find-new) | ||
| # | ||
| my $src_vol = $_[0]; | ||
| my $target_vol = $_[1]; | ||
|
|
||
| # NOTE: in some cases "cgen" differs from "gen", even for read-only snapshots (observed: gen=cgen+1) | ||
| my $lastgen = $src_vol->{node}{gen} + 1; | ||
|
|
||
| # dump files, sorted and unique | ||
| my $ret = btrfs_subvolume_find_new($target_vol, $lastgen); | ||
|
|
||
| INFO "Listing changed files for subvolume: $target_vol->{PRINT} (gen=$target_vol->{node}{gen})"; | ||
| INFO "Starting at generation after subvolume: $src_vol->{PRINT} (gen=$src_vol->{node}{gen})"; | ||
| INFO "Listing files modified within generation range: [$lastgen..$target_vol->{node}{gen}]"; | ||
| DEBUG "Newest file generation (transid marker) was: $ret->{transid_marker}"; | ||
| my $files = $ret->{files}; | ||
| my $total_len = 0; | ||
| foreach my $name (sort keys %$files) { | ||
| my $finfo = $files->{$name}; | ||
| $total_len += $finfo->{len}; | ||
| } | ||
|
|
||
| INFO "Total size: " . print_size($total_len) ; | ||
|
|
||
| return $total_len; | ||
| } |
There was a problem hiding this comment.
We could get similar (same? better?) results by using the results from btrbk extents, which might have some advantages:
- faster (?) (note: installing module "IO::AIO" gives quite some speedup!)
- has caching capabilities (!)
There was a problem hiding this comment.
It don't know, I haven't used btrbk extents so far. This is just a copy of the part run by btrbk diff
There was a problem hiding this comment.
Is this a must or still debatable?
Speed-wise it was pretty fast / negligible compared to the transfer time for all my cases. |
|
Another general comment: |
digint
force-pushed
the
master
branch
6 times, most recently
from
594fc6d
to
dac6350
Compare
|
Any update / further requests for the PR? |
|
I'd suggest updating the --progress entry: A man entry should be added too. Note that there wasn't any commit here since August 2023, so kpoeppel is probably busy. It would be nice to show total progress even for the initial snapshot, at least if quotas are enabled (otherwise it can be pretty slow to compute for HDDs with lots of data). Still, it's pretty useful already. |
Add a
--progress-totaloption that internally runsbtrbk difffirst to get a total size to be transferred (per target). This option is an alternative to--progressand shows aETAtime estimate as well as progress bar viapv.Aims to resolve #543 .