Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid sending whole file when transfer fails #3

Closed
wants to merge 2 commits into from
Closed

Avoid sending whole file when transfer fails #3

wants to merge 2 commits into from

Conversation

benrubson
Copy link
Contributor

Hi,

Here is a PR which stops file upload when it fails.
This can be useful for example when the remote FS is full, and client is uploading big files, preventing it from sending its whole file file for nothing...

Many thanks 馃憤

Initial report there, pushed here by convenience.

@WayneD
Copy link
Member

WayneD commented May 30, 2020

The whole point of the noop_io_until_death() call is to make sure that the user gets the failure message instead of a cryptic closed socket, so I won't be merging a change to remove it.

@WayneD WayneD closed this May 30, 2020
@benrubson
Copy link
Contributor Author

benrubson commented May 30, 2020

Thank you for your feedback @WayneD.
Issue then is that if the sender has started a 500GB file, and its destination repository gets full, it will spend hours sending this file to get an error message... Which does not really makes sense, and confuses user who then wrongly thinks things are properly running 馃槙 ..

@WayneD
Copy link
Member

WayneD commented May 30, 2020

It occurred to me that we could actually skip that noop call when --msgs2stderr was specified, since there is no need to hang around slurping data when the messages aren't intermixed with all the other data. I checked-in a change to that effect.

I'm also imagining a future feature that tries to always leave some room in the generator -> sender buffered data for messages to arrive, but that will require a change in how the receiving side manages the socket, so it will be a long-term change.

@benrubson benrubson deleted the nowhole branch May 30, 2020 13:16
@benrubson
Copy link
Contributor Author

benrubson commented May 30, 2020

So using --msgs2stderr, well -M--msgs2stderr to be precise, works nicely.

It works without using the deamon, i.e. :
rsync -e 'ssh ...' -M--msgs2stderr -a /source/ target.com:/destdir/

It also works in daemon mode, i.e. :
rsync -e 'ssh ...' -M--msgs2stderr -a /source/ target.com::module/destdir/

But for this later one, I had to comment-out the following condition to get the error messages :

https://github.com/WayneD/rsync/blob/da448cdc99e64d2016ec84bb1e6eebd23c3151fd/log.c#L267

Any reason why this condition exists ?

After these successful tests (but the condition above, easily fixable though), I'm not sure why you introduced --msgs2stderr as a debug option in the man page, as it seems to be a proper solution to have a dedicated channel for error messages.

Thank you again 馃憤

@WayneD
Copy link
Member

WayneD commented May 31, 2020

The point of FLOG messages is that they are only for the daemon's log file, just as FCLIENT messages are only for the client (skipping the daemon's log file). You don't mention what the message is, however, so I can't check why it's an FLOG message.

@benrubson
Copy link
Contributor Author

benrubson commented May 31, 2020

You're right, so, instead of just getting, from the sender itself :

rsync: [sender] write error: Broken pipe (32)

I also get, from the deamon (removing the condition then) :

rsync: write failed on "source/bf1" (in module): Disc quota exceeded (69)
rsync error: error in file IO (code 11) at receiver.c(378) [receiver=3.2.0dev]
rsync error: error in file IO (code 11) at io.c(1633) [generator=3.2.0dev]

Note that without the deamon, I don't get the 3rd message (from io.c), but this is not important.

@benrubson
Copy link
Contributor Author

(just to be sure I explained correctly)
Note that just removing the if (!am_daemon) condition does the trick.
So I'm not sure this is a message type (FLOG, FCLIENT...) problem.

@WayneD
Copy link
Member

WayneD commented May 31, 2020

Indeed, it wasn't related to a FLOG message. The issue is that a normal daemon doesn't have a stderr channel to use to get to the user over its socket, and the code had no way to know if the daemon was rsh-run or not so it was assuming that the option only got set via the bad-socket error handling (for which we want to transform a message into a log-only destination). I also noticed that the code was bypassing the quiet check, so I've revamped it a good bit, including adding a way to differentiate socket & rsh-run daemons once things are in full swing.

@benrubson
Copy link
Contributor Author

And it works perfectly, client now properly gets the error messages through stderr from the daemon when -M--msgs2stderr is used.
Thank you very much Wayne 馃憤

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants