Bug 14390 - Feature request: don't fail if using "-z" transferring to rsync complied with --with-included-zlib=no
Summary: Feature request: don't fail if using "-z" transferring to rsync complied with...
Status: ASSIGNED
Alias: None
Product: rsync
Classification: Unclassified
Component: core (show other bugs)
Version: 3.1.3
Hardware: All All
: P5 enhancement (vote)
Target Milestone: ---
Assignee: Wayne Davison
QA Contact: Rsync QA Contact
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2020-05-21 18:48 UTC by Samuel Henrique
Modified: 2020-05-25 20:49 UTC (History)
1 user (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Samuel Henrique 2020-05-21 18:48:15 UTC
Hello,

I'm one of the Debian maintainers of rsync and I'm currently
investigating the switch from "--with-included-zlib=yes" to "no" for
the next stable release (ETA 2021). So basically I'm investigating
what are the impacts of using upstream's zlib instead of rsync's
bundled one.

For the investigation I did simple transfers (1 file) using both "-z"
and "-zz" parameters against different versions of rsync. With the
focus being on 3.1.3 with dynamically linked zlib.

Please note that versions lower than 3.1.1 (2014) are not so impactful
as they don't support the new "-zz" parameter and almost all systems
with those versions are already EOL, the ones which aren't (Debian 7 [2013],RHEL6
[2010], Amazon Linux1[2010], Centos6[2011]) are already at the late
stage of their support and they're planned to go EOL by the end of the
year.

Versions I used for the tests were:
 --with-included-zlib=no:
3.1.3 (let's call this one 3.1.3dynamicLink)

 --with-included-zlib=yes:
3.0.9
3.1.1
3.1.2
3.1.3

Findings:
A) 3.0.9 and 3.1.3dynamicLink: 3.0.9 only supports receiving "-z"
transfers from 3.1.3dynamicLink, this transfer is downgraded to a
non-compressed one but it works.
B) 3.0.9 and others: "-z" works both ways but "-zz" fails.

3.1.1, 3.1.2, 3.1.3 and 3.1.3dynamicLink:
C) Parameter "-zz" works both ways for all versions.
D) Parameter "-z" only works if 3.1.3dynamicLink is the destination,
not the other way around, this transfer is downgraded to a
non-compressed one.

"both ways" means transfers "versionA -> version B" AND "versionB -> versionA"

Issue:
Finding D means that the introduction of an rsync using upstream's
zlib will change the behavior of system's interoperability in two
ways:
1) Sending to bundled zlib versions with "-z" will fallback to a
non-compressed transfer, but still work.
2) Receiving from bundled zlib versions with "-z" will fail.

Issue number 1 is not a big deal since the transfer still happens, but
issue number 2 might cause issues for users who don't migrate their
old systems to use the parameter "-zz" instead of " -z".

The error message that I get from FIndingD, Issue 2, is:
"rsync: This rsync lacks old-style --compress due to its external
zlib.  Try -zz."
Which suggests that rsync realizes "-zz" could work.

So my feature request is to implement some auto fallback to
non-compressed or perform the transfer with "-zz" automatically if
that happens. I believe it would make the transition to upstream's
zlib easier as at least the systems with up-to-date bundled rsync
won't break when transferring with "-z".

FWIW this is the error (3.1.3 transfering with -z to 3.1.3dynamicLink):
rsync: This rsync lacks old-style --compress due to its external zlib.  Try -zz.
rsync error: syntax or usage error (code 1) at main.c(1596) [server=3.1.3]
rsync: connection unexpectedly closed (0 bytes received so far) [sender]
rsync error: error in rsync protocol data stream (code 12) at
io.c(235) [sender=3.1.3]
[sender] _exit_cleanup(code=12, file=io.c, line=235): about to call exit(12)

Feel free to ask for more information about the tests, if needed, I
tried to not be too much verbose.

This feature request is based on the email I sent on the list, titled: "Feature request: don't fail if using -z from --with-included-zlib=yes to --with-included-zlib=no"
Comment 1 Wayne Davison 2020-05-24 17:20:29 UTC
Yeah, the whole -zz idiom was not the best way to have implemented the internal/external compression idiom.  I'll look into what options we have to make it better.

The best way to improve how rsync can interacts with an older rsync that is using the internal zlib is to somehow fix the external-zlib compression support to make them both compatible. That was the original intent, and while it often works OK it likes to fail in strange ways, so I ended up making it complain and die to avoid the potential for corrupted data or aborted transfers. If anyone has time to look into why the two get out of sync, that would be very helpful.
Comment 2 Wayne Davison 2020-05-25 03:04:21 UTC
I've updated the compression code to add a negotiation idiom like I did for checksums, and then I re-enabled the external zlib's ability to handle both the old-style compression (now named "zlib") and the newer, external-library style compression (now named "zlibx").  The code favors zlibx over zlib when the rsyncs are both new enough to perform the negotiation phase.

I've also checked in a preliminary patch that implements lz4 compression (someone sent me a big diff that contained some lz4 code, but I can't figure out who it was who sent it).  If you want to try it out, look for lz4.diff in the patches git.

If anyone has some time to delved into the compression code to stress test it and look for issues, that would be very helpful.
Comment 3 Sebastian A. Siewior 2020-05-25 12:05:19 UTC
(In reply to Wayne Davison from comment #2)

I've sent a zstd patch, what do you want me to do with it (#14338)?
Comment 4 Wayne Davison 2020-05-25 20:49:07 UTC
Thanks for the gentle prod to remind me about 14338.  As things currently stand, the master branch now has support for both zstd & lz4 compression.