perlundq / yajsync

A Java implementation of the rsync protocol
GNU General Public License v3.0
138 stars 37 forks source link

Keep alive message handling #45

Open usrflo opened 8 years ago

usrflo commented 8 years ago

The native rsync implementation sends a MSG_DATA message in the last protocol version to keep a connection alive. This message is sent in different i/o scenarios (checks are executed at fixed places in i/o processing) depending on the time when the last packages were sent. The The following comment comes from io.c:

/* Older rsync versions used to send either a MSG_NOOP (protocol 30) or a
 * raw-data-based keep-alive (protocol 29), both of which implied forwarding of
 * the message through the sender.  Since the new timeout method does not need
 * any forwarding, we just send an empty MSG_DATA message, which works with all
 * rsync versions.  This avoids any message forwarding, and leaves the raw-data
 * stream alone (since we can never be quite sure if that stream is in the
 * right state for a keep-alive message). */
void maybe_send_keepalive(time_t now, int flags)

Bug reports like https://bugzilla.samba.org/show_bug.cgi?id=7757 or hints like the following from https://download.samba.org/pub/rsync/rsync.html reveal that the implementation in native rsync is improvable:

--delete-before [...] Deleting before the transfer is helpful if the filesystem is tight for space and removing extraneous files would help to make the transfer possible. However, it does introduce a delay before the start of the transfer, and this delay might cause the transfer to timeout (if --timeout was specified).

Considering MSG_DATA is stable and compatible with different rsync protocol versions I propose to use scheduled threads (ScheduledExecutorService.scheduleAtFixedRate) to send out MSG_DATA packages independently from sender processing. This may result in more packages than required but assures timeouts are not met as long as the sender is running.

Do you see any better way to solve this?

perlundq commented 8 years ago

That might possibly work, and hopefully it is as backwards compatible as is claimed. But I am not sure. The other way to do this would be using non-blocking i/o, but that's not gonna happen now - it's too big a change and more intricate/harder to get right.