Revisiting two old issues

[jw schultz]

On Thu, May 08, 2003 at 12:56:31PM -0700, Wayne Davison wrote:
> I'd like some opinions on a couple of long-standing rsync issues.  My
> two oldest, uncommitted patches are:
> 
>   - A "no hang" patch that makes sure that the pipe from the receiver
>     to the generator can't block with resend requests.
> 
>   - The "move files" patch that got changed into a --delete-sent-files
>     option.
> 
> For each item I have two questions -- do we need to deal with this?
> And is the proposed change a good way to implement the change?  Some
> comments on each item follow:
> 
> Redo-Channel Anti-Hang Patch
> ============================
> 
> I've had a couple different incarnations of this patch because the IO
> section is quite complex and there were concerns about memory usage (if
> rsync keeps the redo channel clear, it has to note the redo items
> somewhere).  My first one kept a buffer of redo items that would expand
> only as redo items arrived (Red Hat actually incorporated this one into
> their released version of rsync 2.4.6 at some point, so it even got a
> lot of testing, unbeknownst to me at the time).  My later patch changed
> the no-hang algorithm to keep a flag char for every item in the file
> list (to avoid a criticism about having a growing buffer).  I actually
> prefer the first patch these days, because it has a lower memory impact
> on large file lists.  Anyone have an opinion on this?

If it is used just for the redo the redo buffer makes more
sense.  However, i think this has more use than just redo so
i lean toward a transfer-status flags array.

> More technical comments:  I don't believe that we should use the
> existing per-item flags in the flist data since it is shared between
> two forked processes, and twiddling data in copy-on-write memory may
> well cause a larger memory bloat than just keeping a separate flag
> array.  Of course if we eventually switch over to threads, this
> copy-on-write pitfall would go away.

The current flags field in the flist is a char is sent as a
char and has no free bits.  Adding more, non-transmitted,
flags would be best done by adding another field rather than
expanding the current one.  As for the copy-on-write issue;
I've not looked into it but are you sure the fork is done
after the flist is fully populated?

> Another alternative implementation would be to change the rsync
> algorithm to recycle the redo items immediately instead of in a
> separate pass.  This would eliminate the need for caching the redo
> data.  (Aside: have the recent checksum-length changes taken into
> account the redo pass that tries to use an alternate checksum size
> for the resends?)

When starting the redo a phase variable (seemingly in
conflict with other vars of the same name
so i haven't yet identified its scope) is incremented and
the global csum_length is set to SUM_LENGTH.  Currently this
is detected with

        if (remote_version < 27) {
                s2length = csum_length;
        } else if (csum_length == SUM_LENGTH) {
                s2length = SUM_LENGTH;
        } else {

If i could be sure that the same phase as indicates redo
were in scope i'd use that and eliminate csum_length.  Were
there a per-file status to test that is what i would use.

As is clear, we depend on global vars to determine that we
are in the redo phase.  For performance reasons i'd favour
doing the redo as near to detection as possible but it
couldn't depend on global vars to identify it as a redo so
we would need to have per-file flags for that.

SPECULATIVE
	While dynamic checksum and block sizes should reduce
	redo frequency i wonder if we shouldn't fall back on
	whole-file if a redo fails.


-- 
________________________________________________________________
	J.W. Schultz            Pegasystems Technologies
	email address:		jw at pegasys.ws

		Remember Cernan and Schmitt