Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Streaming queue size protection #171

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
2 participants
Contributor

mikrohard commented Oct 23, 2012

Now that all memory leaks in http streaming are cleared there is only one major memory issue left. HTSP streaming has a queue size protection but HTTP streaming doesn't.

When there are clients on a slow connection or if the server has connection problems the streaming queue size can increase indefinitely and sooner or later cause out of memory problems on the server.

This can be very easily simulated with VLC. Just start a HTTP stream and press play/pause in 5-10 seconds intervals. You will see the memory usage of tvheadend increase at the bit rate of the stream.

Here is a massif heap profile of tvheadend (marked is the highest memory usage --> cca. 140Mb):
Tvheadend without queue size protection

Here is a massif heap profile of tvheadend with the queue size protection from these PR (marked is the highest memory usage --> cca. 9Mb):
Tvheadend with queue size protection

Both heap profiles were recorded with the same stream, same player and similar play/pause intervals.

I'm not sure if this is the best solution for this problem. I just wanted to point out that there is a problem.

Owner

adamsutton commented Oct 23, 2012

@mikrohard great I'll take a look, I believe at the time I made the previous "fix" I did state that the exact scenario you describe was still an issue that ultimately needed to be solved.

Looks like you might have done the job for me :)

@ghost ghost assigned adamsutton Oct 23, 2012

Owner

adamsutton commented Oct 23, 2012

@mikrohard OK, I've had a look and I'm going to make some changes. The queue size checking will be optional and configurable by the caller. This will remove the requirement to check size where its not necessary, i.e.:

DVR - since it write to disk should be able to keep up
Timeshift - same as DVR (not in master of course)
HTSP - has its own q limiting
Intermediate queues - anything else that's in the internal streaming chain.

So this currently just leaves the HTTP streaming for which a queue size is specified but can also be overridden by the user (using HTTP GET arg).

Ultimately we should consider (for PKT based streaming) whether we should try and employ a similar methodology to HTSP (frame type dependant q lengths). But that's for another day.

I'll push this all to master shortly.

Adam

@adamsutton adamsutton closed this Oct 23, 2012

Contributor

mikrohard commented Oct 23, 2012

Sure... but keep in mind that DVR (and timeshift) could be also written on a USB drive or sd card on a low power (and memory) device. There should always be a protection (even if it's set to a higher value than streaming).

Owner

adamsutton commented Oct 23, 2012

@mikrohard it's a fair point, but I'd argue if you're silly enough to try and record/shift to such slow media then you're asking for trouble anyway ;) but we can easily integrate limits later.

However since the only issues reported thus far relate to HTTP streaming that seems the most important place to cover things. And we definitely don't want to add extra queue size checking where its not needed (such as intermediate queues, that will ultimately be bounded by the final Q) and for streaming chains that already have their own limiting (HTSP).

At least with DVR and timeshift they will (mostly) be bounded by the fact there will be intrinsic time limits (though admittedly still more than enough data to kill a system).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment