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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

file transfer improvements #1026

Closed
totaam opened this issue Nov 9, 2015 · 12 comments
Closed

file transfer improvements #1026

totaam opened this issue Nov 9, 2015 · 12 comments

Comments

@totaam
Copy link
Collaborator

totaam commented Nov 9, 2015

Split from #494, remaining tasks:

  • each file needs to be chunked so we can interleave regular xpra traffic - high priority
  • ability to cancel transfers in progress
  • client option of showing a dialog to accept files before starting the transfer (and choose download location?)
@totaam
Copy link
Collaborator Author

totaam commented Nov 10, 2015

Edit: comment about a file too big error moved to #494#comment:6

We should probably also handle the file-too-big issue with an alert box?

@totaam
Copy link
Collaborator Author

totaam commented Nov 12, 2015

Also from #494#comment:8 : GtkWarning: Could not find the icon 'gtk-file'. The 'hicolor' theme was not found either, perhaps you need to install it
Looks like we may need to tweak the win32 packaging to include or point to the (more complete?) theme.

@totaam
Copy link
Collaborator Author

totaam commented Feb 29, 2016

#1124 is a nicer, more generic solution to this problem.

@totaam
Copy link
Collaborator Author

totaam commented Jun 13, 2016

2016-06-13 07:13:48: antoine uploaded file file-transfer-refactoring.diff (29.6 KiB)

refactoring needed to add chunking code only in one place

@totaam
Copy link
Collaborator Author

totaam commented Jun 13, 2016

2016-06-13 14:15:01: antoine uploaded file chunked-file-transfers.patch (16.2 KiB)

mostly complete patch, just needs cleaning up and cancelling the timers

@totaam
Copy link
Collaborator Author

totaam commented Jun 13, 2016

Done: preparatory refactoring in r12810 + r12812, actual chunking code in r12813.

The files are now sent in chunks. We wait for the file chunk ack packet before sending the next chunk, which helps to ensure that we don't use up all the bandwidth, though it does reduce the throughput (only 4MB/s on a powerfull system loopback).

New tunables which should be self-explanatory:

XPRA_FILE_CHUNKS_SIZE=65536
XPRA_MAX_CONCURRENT_FILES=10

Here's what the transfer looks like with -d file for an ~80MB file upload to the server:

  • client side:
show_file_upload(<gtk.ImageMenuItem object at 0x7f95f25496e0 (GtkImageMenuItem at 0x56554cae59c0)>,) can open=False
load_contents: filename=/home/antoine/Downloads/NVIDIA-Linux-x86_64-361.45.11.run, 86205078 bytes, entity=1464443710:327571, response=-5
send_file('/home/antoine/Downloads/NVIDIA-Linux-x86_64-361.45.11.run', '', <type 'str'>, '86205078 bytes', False, False, {})
sha1 digest(/home/antoine/Downloads/NVIDIA-Linux-x86_64-361.45.11.run)=28d1586dd300c04f2e1ce66604a939bccfa51fa6
ack-file-chunk: ['9f4c40d1ebf74aa69f02e8ebd866786c', True, '', 0]
ack-file-chunk: ['9f4c40d1ebf74aa69f02e8ebd866786c', True, '', 1]
...
ack-file-chunk: ['9f4c40d1ebf74aa69f02e8ebd866786c', True, '', 1316]
1316 chunks of 65536 bytes sent in 17338ms (4MB/s)
_check_chunk_sending(9f4c40d1ebf74aa69f02e8ebd866786c, 1316) chunk_state found: False
  • server side:
receiving file: ['bigfile', '', False, False, 86205078, '0 bytes', {'file-chunk-id': '9f4c40d1ebf74aa69f02e8ebd866786c', 'sha1': '28d1586dd300c04f2e1ce66604a939bccfa51fa6'}]
using filename 'bigfile'
_process_send_file_chunk('9f4c40d1ebf74aa69f02e8ebd866786c', 1, '65536 bytes', True)
_process_send_file_chunk('9f4c40d1ebf74aa69f02e8ebd866786c', 2, '65536 bytes', True)
...
sha1 digest matches: 28d1586dd300c04f2e1ce66604a939bccfa51fa6
86205078 bytes received in 1316 chunks, took 16769ms
downloaded 86205078 bytes to temporary file:
 '/home/guest/Downloads/bigfile'

@afarr: ready for testing, you should now be able to send or print big files without causing much of a slow down for anything else. Logging was also improved, small bugs fixed along the way. (I may backport some)

@totaam
Copy link
Collaborator Author

totaam commented Jun 13, 2016

2016-06-13 22:25:14: maxmylyn commented


Did some testing of trunk 12814 file uploads (Fedora 23 both ends):

  • All good with sending files - doesn't seem to hang anything anywhere.

Of note:

The select file dialogue will hang for a solid second or two if you try to upload a large file

@totaam
Copy link
Collaborator Author

totaam commented Jun 14, 2016

r12815 closes the dialog more quickly and loads the file in the background as much as possible.

@totaam
Copy link
Collaborator Author

totaam commented Sep 29, 2016

2016-09-29 01:58:37: afarr commented


Tested with a 1.0 r13101 win32 client against a 1.0 r13902 fedora 23 server.

Dialog closes very quickly, whether I'm uploading a 4 kb file or a 68 MB file, or even if I'm uploading a 218 MB file... though there wasn't anything to let me know that the 218 MB file was too ambitious, until I noticed the client side warning:

2016-09-28 17:31:12,180 Warning: cannot upload the file 'sound-test-weekend-log.txt'
2016-09-28 17:31:12,180  this file is too large: 218MB
2016-09-28 17:31:12,181  the local file size limit is 100MB

The session wasn't in any way impacted while uploading a 68 MB file though... so, other than considering a popup to warn a user of overzealous ambitions, I think this is ready to close.

@totaam
Copy link
Collaborator Author

totaam commented Sep 29, 2016

2016-09-29 04:59:33: antoine uploaded file file-alert-dialog.png (13.0 KiB)

new alert dialog
file-alert-dialog.png

@totaam
Copy link
Collaborator Author

totaam commented Sep 29, 2016

Good point. Alert dialog added in r13903

We also check the file size sooner and avoid trying to load really big files into memory before showing the error.

@totaam
Copy link
Collaborator Author

totaam commented Sep 30, 2016

2016-09-30 23:15:55: maxmylyn commented


13937 Trunk Windows 8.1 client against an 13937 Trunk Fedora 23 machine:

  • Error noted in the popup (edited) when attempting to upload a 1.7GB ISO.

Not much else left here, closing.

@totaam totaam closed this as completed Sep 30, 2016
totaam added a commit that referenced this issue Feb 12, 2023
even the html5 client now supports chunked file transfers:
Xpra-org/xpra-html5#120
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

No branches or pull requests

1 participant