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

Streaming large files #13

Closed
xyproto opened this issue Sep 1, 2017 · 12 comments
Closed

Streaming large files #13

xyproto opened this issue Sep 1, 2017 · 12 comments
Assignees
Labels
bug This is a bug

Comments

@xyproto
Copy link
Owner

xyproto commented Sep 1, 2017

Algernon uses too much memory when serving large files (like 500MB audio files). This might be related to caching. Please fix.

@xyproto
Copy link
Owner Author

xyproto commented Oct 21, 2017

Not compressing the file before checking if it would fit in the cache in a compressed state fixed this.

@xyproto xyproto closed this as completed Oct 21, 2017
@epitron
Copy link

epitron commented Jan 21, 2019

This is a great project! I really love all the built-in features.

Unfortunately, this issue has re-emerged.

It looks like the server is completely loading the file into some kind of cache or a buffer before sending it to the client: https://github.com/xyproto/algernon/blob/master/engine/handlers.go#L369

Shouldn't all static files be streamed directly from the disk to the client? (That's the default behaviour for all of the other webservers I've used.)

Additionally, files that are already compressed (eg: zip, jpg, mp4, ogg, etc.) can be streamed directly to the client without gzipping.

Could we have a function that streams static files?

@xyproto
Copy link
Owner Author

xyproto commented Jan 21, 2019

Thanks for reporting. I suspect that the streaming issues are in debug mode only. Could you please try the -x flag and see if it streams correctly?

@epitron
Copy link

epitron commented Jan 22, 2019

Same issue.

Strangely, the process also balloons up to many multiples of the size of the file it's serving. When I first start algernon, the process uses 293MB of memory. Then I load a 75MB video, and the process's memory usage grows to 771MB.

Is algernon is copying the memory block as it's being passed around between functions?

Would it be a large architectural change to pass a file handle as a response, and have that stream to the client?

@xyproto
Copy link
Owner Author

xyproto commented Jan 22, 2019

I will create a reproducible test case and then fix this.

xyproto added a commit that referenced this issue Jan 23, 2019
@xyproto xyproto reopened this Jan 23, 2019
@xyproto
Copy link
Owner Author

xyproto commented Jan 23, 2019

I am able to reproduce the issue.

@xyproto
Copy link
Owner Author

xyproto commented Jan 23, 2019

The issue is that this line is trying to read in the entire file:

https://github.com/xyproto/datablock/blob/8c3914e5c4fe78accb948fad712c0080dfb5ded9/filecache.go#L437

I need to change the code to only read the file if:

  • caching is enabled and it is under a certain size
  • it's one of the supported data types that will need to be interpreted (like Lua scripts) and under a certain size

xyproto added a commit that referenced this issue Jan 23, 2019
@xyproto
Copy link
Owner Author

xyproto commented Jan 23, 2019

Added a fix to master. Please test.

@epitron
Copy link

epitron commented Jan 24, 2019

Just tested it:
$ ./algernon --httponly --nocache --server ~/bigfiles :8889

When I try to get a big file, this happens:

< HTTP/1.1 500 Internal Server Error
< Content-Type: text/plain; charset=utf-8
< Last-Modified: Thu, 07 May 2015 08:36:32 GMT
< Server: Algernon 1.12.1
< X-Content-Type-Options: nosniff
< X-Frame-Options: SAMEORIGIN
< X-Rate-Limit-Duration: 1
< X-Rate-Limit-Limit: 10.00
< X-Rate-Limit-Request-Forwarded-For: 
< X-Rate-Limit-Request-Remote-Addr: 127.0.0.1:34934
< X-Xss-Protection: 1; mode=block
< Date: Thu, 24 Jan 2019 00:55:57 GMT
< Content-Length: 18
< 
seeker can't seek

@xyproto
Copy link
Owner Author

xyproto commented Jan 24, 2019

Thanks for testing. I will add a better test and fix this.

@xyproto xyproto self-assigned this Jan 24, 2019
@xyproto xyproto added the bug This is a bug label Jan 24, 2019
@xyproto
Copy link
Owner Author

xyproto commented Jan 24, 2019

I updated Algernon to be able to stream large files. I tried both with your test case and the regression test.

Two new flags were also added:

--timeout=N for setting a timeout in seconds, when serving large files (but there is range support, so if a download times out, the client can continue where it left).
--largesize=N for setting a threshold for when a file is too large to be read into memory (the default is 42 MiB).

Please test. :)

@epitron
Copy link

epitron commented Jan 24, 2019

Success!!!

@xyproto xyproto closed this as completed Jan 25, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This is a bug
Projects
None yet
Development

No branches or pull requests

2 participants