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

Large packets (2GB) don't get sent #352

Open
Jasper-Bekkers opened this issue May 16, 2020 · 6 comments
Open

Large packets (2GB) don't get sent #352

Jasper-Bekkers opened this issue May 16, 2020 · 6 comments
Labels
A-tonic C-bug Category: Something isn't working

Comments

@Jasper-Bekkers
Copy link

Bug Report

It takes Tonic approximately ~25 seconds to prepare a packet when it's 2GB in size, but ultimately it doesn't get received by the localhost server and the client just sits idle. Gave up after 5 minutes of waiting.

image

Minimal repro attached

tonic-repro.zip

@LucioFranco
Copy link
Member

@seanmonstar you may be interested

@Matthias247
Copy link

This will just be a collection of various worst-case scenarios again:

  • Reading big data chunks with some read_to_end() API will require those to be resized/reallocated each time until they had been read completely. On each resize you need to copy all already received data into the new segment. Assuming you ask for 1MB extra on each resize you need 2000 allocations, with 2000 * 1 GB byte copies on average.
  • Besides that some problems with tokios AsyncRead trait might lead to calling excessive extra initialization of target arrays with 0s on every read. Which could mean at some time you will write up to 2GB of 0s for every read() call.

Fixes:

  • Generally: Don't use gRPC for transferring big data chunks. Use streaming.
  • Implementation wise: Things which read bigger chunks of data should resize them upfront to the target size and don't do dynamic resizing.

@Jasper-Bekkers
Copy link
Author

Surely though; they should still arrive properly?

@seanmonstar
Copy link
Member

Yep, I just checked with logs turned on, the chunk is rejected for a wrong reason (woops). I'll file an issue on the h2 repo.

@repi
Copy link
Contributor

repi commented Jun 27, 2020

@seanmonstar this issue hyperium/h2#471?

@LucioFranco
Copy link
Member

Sounds about right. Tonic doesn't do any sort of checking for that and delegates to h2.

@LucioFranco LucioFranco added C-bug Category: Something isn't working A-tonic labels Nov 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-tonic C-bug Category: Something isn't working
Projects
None yet
Development

No branches or pull requests

5 participants