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-Sharing example silently fails and or panics when trying to send a file too big. #5018

Closed
Jorropo opened this issue Dec 18, 2023 · 6 comments

Comments

@Jorropo
Copy link

Jorropo commented Dec 18, 2023

Summary

The file sharing example fails in weird ways with big enough files (I tried with 1GiB).
Similar to #5016

Expected behavior

The file should be streamed without having to hold everything in memory.

Actual behavior

The file is slurped and AFAIT some guards are failing because reading a >1GiB single message is too big.

Relevant log output

> /home/hugo/Documents/Scripts/rust-libp2p/target/release/file-sharing-example --peer /ip4/127.0.0.1/tcp/37745/p2p/12D3KooWPfjY1bigGcpUKw6bu5oyFaXHRGUkD1spn8JYTzGvdgPy get --name a
Local node is listening on "/ip4/127.0.0.1/tcp/43357/p2p/12D3KooWDKx57BhENcicUwdiMD6LUr9zCWwwsMA1eNCwvdNrm2yC"
Local node is listening on "/ip4/192.168.1.25/tcp/43357/p2p/12D3KooWDKx57BhENcicUwdiMD6LUr9zCWwwsMA1eNCwvdNrm2yC"
Local node is listening on "/ip4/172.17.0.1/tcp/43357/p2p/12D3KooWDKx57BhENcicUwdiMD6LUr9zCWwwsMA1eNCwvdNrm2yC"
Error: "None of the providers returned file."

Possible Solution

Stream the file.

Version

c115cdd

Would you like to work on fixing this bug ?

No

@dariusc93
Copy link
Member

Hey! The issue might be due to the response limit that is set for cbor in request-response

/// Max response size in bytes
const RESPONSE_SIZE_MAXIMUM: u64 = 10 * 1024 * 1024;

Ideally you would want to split the file down into chunks and send it that way to be more efficient (or maybe use something like libp2p-bitswap), but to play around with request-response, you could copy the cbor code above and increase the the response limit to your desire size

@Jorropo
Copy link
Author

Jorropo commented Dec 18, 2023

libp2p-bitswap is overkill for such a simple example and will require a lot of other machinery to get working.

I don't think the splitting is what we should be thinking about.
Other layers (yamux & tcp) already takes care of doing this.

I think the better alternative is to make this example streaming.
For example in go you would write:

_, err := io.Copy(stream, file)

@thomaseizinger
Copy link
Contributor

I think the better alternative is to make this example streaming.

Correct. We currently don't expose an abstraction that "just" gives you a stream. The current suggestion approach forward to resolve this is: #4457 (which is being worked on actually!).

@thomaseizinger
Copy link
Contributor

I am going to close this as a wont-fix because it is by design that request-response limits the size. Until #4457 is ready, you can always implement your own NetworkBehaviour and ConnectionHandler that use the AsyncWrite interface of Stream to copy the file.

@thomaseizinger thomaseizinger closed this as not planned Won't fix, can't repro, duplicate, stale Dec 19, 2023
@Jorropo
Copy link
Author

Jorropo commented Dec 19, 2023

#4457 looks good, I wasn't sure why file-sharing had to use request response, imo it don't need it. But if you write an other example it's fine.

@thomaseizinger
Copy link
Contributor

I wasn't sure why file-sharing had to use request response

It doesn't! The main thing this example shows is how to separate the network layer into its own task. See https://github.com/libp2p/rust-libp2p/tree/master/examples/file-sharing#architectural-properties.

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

3 participants