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

Web: improve the experience for large file uploads #83565

Closed
connor4312 opened this issue Oct 29, 2019 · 15 comments
Closed

Web: improve the experience for large file uploads #83565

connor4312 opened this issue Oct 29, 2019 · 15 comments
Assignees
Labels
feature-request Request for new features or functionality on-release-notes Issue/pull request mentioned in release notes verification-needed Verification of issue is requested verified Verification succeeded web Issues related to running VSCode in the web
Milestone

Comments

@connor4312
Copy link
Member

connor4312 commented Oct 29, 2019

Check: #83565 (comment)

Outdated:

  • VSCode Version: 1.40.0-insider @ b7b2184
  • OS Version: OS X Mojave 10.14

Steps to Reproduce:

  1. Create a large file (dd if=/dev/zero of=file.txt count=1024 bs=8388608 creates an 8GB file)
  2. Try to upload it
  3. Nothing happens

I'm guessing we need to support some kind of streaming, I notice most files get uploaded in a single websocket frame (and supposedly loaded into memory). Chrome might not like us loading or sending very large files this way.

Does this issue occur when all extensions are disabled?: Yes

Ref: #83225

@connor4312 connor4312 added bug Issue identified by VS Code Team member as probable bug web Issues related to running VSCode in the web labels Oct 29, 2019
@bpasero bpasero assigned isidorn and unassigned bpasero Oct 30, 2019
@bpasero
Copy link
Member

bpasero commented Oct 30, 2019

My understanding is that we are using streams.

@isidorn isidorn added this to the On Deck milestone Nov 12, 2019
@bpasero
Copy link
Member

bpasero commented May 20, 2020

@steven166 in #97347 you first showed a solution using streams and unfortunately I cannot see that commit anymore. Can you share it again or point me to the docs? I cannot really see how to read from a File as stream, or maybe this is specific to Chrome API and not a standard.

Bottom line, I wonder if the stream based file handling would help in this case.

@steven166
Copy link
Contributor

Had it ready, but forgot to create a pr... Will resolve those merge conflicts.

@bpasero bpasero assigned bpasero and unassigned isidorn May 28, 2020
@bpasero bpasero modified the milestones: On Deck, May 2020 May 28, 2020
@bpasero
Copy link
Member

bpasero commented May 29, 2020

via 203508d

I tested with a 2GB file and it worked.

@bpasero bpasero closed this as completed May 29, 2020
@joaomoreno
Copy link
Member

joaomoreno commented Jun 4, 2020

@bpasero I tried uploading a 2GB file on MS Edge, Windows. It was hanging/processing for a couple of minutes and then just blew up. The node process output had:

Error: write ECONNABORTED
    at afterWriteDispatched (internal/stream_base_commons.js:146:25)
    at writeGeneric (internal/stream_base_commons.js:137:3)
    at Socket._writeGeneric (net.js:696:11)
    at Socket._write (net.js:708:8)
    at doWrite (_stream_writable.js:417:12)
    at writeOrBuffer (_stream_writable.js:401:5)
    at Socket.Writable.write (_stream_writable.js:301:11)
    at h.write (C:\Users\jomo\Downloads\vscode-server-win32-x64-web\out\vs\server\remoteExtensionHostAgent.js:179:705)
    at p.write (C:\Users\jomo\Downloads\vscode-server-win32-x64-web\out\vs\server\remoteExtensionHostAgent.js:180:925)
    at p._writeNow (C:\Users\jomo\Downloads\vscode-server-win32-x64-web\out\vs\server\remoteExtensionHostAgent.js:171:911)
    at Immediate.<anonymous> (C:\Users\jomo\Downloads\vscode-server-win32-x64-web\out\vs\server\remoteExtensionHostAgent.js:171:849)
    at processImmediate (internal/timers.js:439:21) {
  errno: 'ECONNABORTED',
  code: 'ECONNABORTED',
  syscall: 'write'
}
[14:14:21] [::1][baec1b55][ManagementConnection] The client has disconnected, will wait for reconnection 3h before disposing...
Error: read ECONNRESET
    at TCP.onStreamRead (internal/stream_base_commons.js:183:27) {
  errno: 'ECONNRESET',
  code: 'ECONNRESET',
  syscall: 'read'
}

I was uploading a 2.6GB file. After those two minutes, all I had was 4KB transfered.

@bpasero
Copy link
Member

bpasero commented Jun 4, 2020

I only tried on macOS so far and only running locally. Just verified that I could upload a 5MB file to Codespaces at least. I think we can probably leave it as that, I doubt this is a use case. If someone wanted to upload something very large to a Codespace, I would think it would be via git. Bottom line is that the upload will be much slower compared to using the terminal because of having to send each bit through the websocket connection.

Nevertheless, you can also run wget to download from within a terminal.

@steven166
Copy link
Contributor

If you can only upload small files, then it is the same as the old situation and there might be an issue with the streams implementation.

I think there are real world use-cases where people just want to upload large files from their local machine into a remote workspace. I hit this limitation myself when I wanted to upload a database dump for example.

@bpasero
Copy link
Member

bpasero commented Jun 4, 2020

One issue I immediately hit is that a reconnection dialog appears when uploading a large file. It is possible that the pressure the upload puts on the web socket connection makes the client believe the server is gone because afaik it uses a simple ping-pong check to see if the server responds. Possibly the upload needs to throttle itself to avoid that from happening.

I think it would be great to validate these cases against an online hosted solution and not locally because that is the scenario we are after. As I said, I could upload a 5 MB file just now.

I do not see the 4GB case any pleasant with the current experience because we are not rendering any progress detailed enough to know what is going on. E.g. I would think the throughput and status should be visible for very large file uploads.

@bpasero bpasero added feature-request Request for new features or functionality and removed bug Issue identified by VS Code Team member as probable bug labels Jun 4, 2020
@bpasero bpasero modified the milestones: May 2020, Backlog Jun 4, 2020
@bpasero bpasero reopened this Jun 4, 2020
@bpasero bpasero changed the title Web: uploading large files is a no-op Web: improve the experience for large file uploads Jun 4, 2020
@bpasero
Copy link
Member

bpasero commented Jun 4, 2020

Maybe better to reopen this issue and really think about the missing points for a good experience:

@steven166
Copy link
Contributor

I experienced the reconnection dialog myself, but after my implementation of streams in #98248 this was resolved. Therefor I suspect that the stream is somehow still loaded as a whole in memory.

@bpasero
Copy link
Member

bpasero commented Jun 4, 2020

I have a todo item to look into our use of streams, I too realised that the solution puts a lot of pressure on the browser because the stream is being written to without waiting for it to write to the target. I plan to fix that next week and still think that the stream based solution is more elegant. But we can revalidate this once I pushed the changes.

PS: maybe a combination of your solution together with my stream based approach would be possible too. I can take a look. Your solution had the advantage that the slice of Buffer was defined by us and not the browser.

@bpasero
Copy link
Member

bpasero commented Jun 11, 2020

There is now accurate progress for large files and streams have a new highWaterMark option that will reduce pressure:

recording

@bpasero bpasero closed this as completed Jun 11, 2020
@bpasero bpasero modified the milestones: Backlog, June 2020 Jun 11, 2020
@bpasero bpasero added the verification-needed Verification of issue is requested label Jun 26, 2020
@bpasero
Copy link
Member

bpasero commented Jun 26, 2020

Verification:

  • connect to a Codespace that has our latest insider
  • upload a file of N megabytes
  • verify you see progress during the upload
  • verify the file is uploaded properly

@bpasero bpasero added the on-release-notes Issue/pull request mentioned in release notes label Jun 26, 2020
@lramos15 lramos15 added the verified Verification succeeded label Jul 1, 2020
@DanielHabenicht
Copy link

DanielHabenicht commented Jul 15, 2020

Question: is there an option to upload files via the right-click-menu or does it only support drag and drop?

@bpasero
Copy link
Member

bpasero commented Jul 16, 2020

There is only drag and drop currently.

@github-actions github-actions bot locked and limited conversation to collaborators Jul 26, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
feature-request Request for new features or functionality on-release-notes Issue/pull request mentioned in release notes verification-needed Verification of issue is requested verified Verification succeeded web Issues related to running VSCode in the web
Projects
None yet
7 participants