HTTP-FLV: Notify connection to expire when unpublishing. v6.0.152 v7.0.11 #4164

winlinvip · 2024-08-31T09:12:56Z

When stopping the stream, it will wait for the HTTP Streaming to exit. If the HTTP Streaming goroutine hangs, it will not exit automatically.

void SrsHttpStreamServer::http_unmount(SrsRequest* r)
{
    SrsUniquePtr<SrsLiveStream> stream(entry->stream);
    if (stream->entry) stream->entry->enabled = false;
    srs_usleep(...); // Wait for about 120s.
    mux.unhandle(entry->mount, stream.get()); // Free stream.
}

srs_error_t SrsLiveStream::serve_http(ISrsHttpResponseWriter* w, ISrsHttpMessage* r)
{
    err = do_serve_http(w, r); // If stuck in here for 120s+
    alive_viewers_--; // Crash at here, because stream has been deleted.

We should notify http stream connection to interrupt(expire):

void SrsHttpStreamServer::http_unmount(SrsRequest* r)
{
    SrsUniquePtr<SrsLiveStream> stream(entry->stream);
    if (stream->entry) stream->entry->enabled = false;
    stream->expire(); // Notify http stream to interrupt.

Note that we should notify all viewers pulling stream from this http stream.

Note that we have tried to fix this issue, but only try to wait for all viewers to quit, without interrupting the viewers, see #4144

Co-authored-by: Jacob Su suzp1984@gmail.com

suzp1984

add 2 comments.

trunk/src/app/srs_app_http_stream.cpp

suzp1984 · 2024-08-31T13:47:31Z

trunk/src/app/srs_app_http_stream.cpp

+    // Remove viewer from the viewers list.
+    vector<ISrsExpire*>::iterator it = std::find(viewers_.begin(), viewers_.end(), hc);
+    srs_assert (it != viewers_.end());
+    viewers_.erase(it);


viewers_, as vector container , has bad efficiency in erase, and also find for unsorted data.
For one SRS instance, it is reasonable to support 1K flv http connection.
So it's 1K unsorted vector search and delete frequently. (Not so bad, also not so good).
If the vector size is 10K, it will be serious problem.

In those reason, consider map or unordered_map as the container. the SrsHttpConn->get_id as the key, make sure the key is unique for each SrsHttpConn.

No performance issues even for 10,000 viewers, because this only executes once when a viewer closes the connection.

If the vector size is 10,000, it will be a serious problem.

I have written a test program to verify it; it is definitely not that slow. We should never fix problems that do not exist.

Try bellow benchmark, to randomly find a elem in 100k vector:

#include <iostream> #include <vector> #include <cstdlib> #include <ctime> #include <chrono> using namespace std; using namespace chrono; int main() { int max_elements = 100 * 1000; vector<uint64_t> vec; for (uint64_t i = 0; i < max_elements; ++i) { vec.push_back(i); } srand(static_cast<unsigned>(time(0))); for (int i = 0; i < 10; i++) { size_t random_index = rand() % max_elements; uint64_t value = vec[random_index]; auto start = high_resolution_clock::now(); vector<uint64_t>::iterator it = std::find(vec.begin(), vec.end(), value); auto end = high_resolution_clock::now(); auto duration = duration_cast<nanoseconds>(end - start).count() / 1000.0; cout << "Element at index " << random_index << " is " << value << " find " << *it << endl; cout << "Time taken to find the element: " << duration << " microseconds" << endl << endl; } return 0; }

Result:

Element at index 0 is 0 find 0 Time taken to find the element: 0.166 microseconds Element at index 30000 is 30000 find 30000 Time taken to find the element: 77.25 microseconds Element at index 20000 is 20000 find 20000 Time taken to find the element: 50.5 microseconds Element at index 30000 is 30000 find 30000 Time taken to find the element: 75.125 microseconds Element at index 70000 is 70000 find 70000 Time taken to find the element: 172.541 microseconds Element at index 30000 is 30000 find 30000 Time taken to find the element: 74.042 microseconds Element at index 90000 is 90000 find 90000 Time taken to find the element: 223.125 microseconds Element at index 80000 is 80000 find 80000 Time taken to find the element: 199.542 microseconds Element at index 20000 is 20000 find 20000 Time taken to find the element: 51.875 microseconds Element at index 50000 is 50000 find 50000 Time taken to find the element: 123.084 microseconds

It only increase 0.1ms when viewer closing the connection, note that in 100k vector, not 10k.

one viewer close the http connection, it exec one time, but if 10K viewers close http flv connections, this code will run 10K times. The rarely case is the 10K viewers close the connection at same time. OK, I accept this.

For the benchmark, The real disaster of the vector is the erase. I would avoid doing this as far as possible.

Even with 10,000 viewers, each increase is only 0.01ms. When will 10,000 viewers close the connection simultaneously? We should never fix problems that do not exist. You will be completely occupied if you try to solve every imaginary problem that does not exist.

OK, accept it.

For the benchmark, The real disaster of the vector is the erase. I would avoid doing this as far as possible.

Try this:

#include <iostream> #include <vector> #include <cstdlib> #include <ctime> #include <chrono> using namespace std; using namespace chrono; int main() { int max_elements = 10 * 1000; vector<uint64_t> vec; for (uint64_t i = 0; i < max_elements; ++i) { vec.push_back(i); } srand(static_cast<unsigned>(time(0))); for (int i = 0; i < 10; i++) { size_t random_index = rand() % vec.size(); uint64_t value = vec[random_index]; auto start = high_resolution_clock::now(); vector<uint64_t>::iterator it = std::find(vec.begin(), vec.end(), value); vec.erase(it); auto end = high_resolution_clock::now(); auto duration = duration_cast<nanoseconds>(end - start).count() / 1000.0; cout << "Element at index " << random_index << " is " << value << " find " << *it << endl; cout << "Time taken to find the element: " << duration << " microseconds" << endl << endl; } return 0; }

The result:

Element at index 8555 is 8555 find 8556 Time taken to find the element: 21 microseconds Element at index 8770 is 8771 find 8772 Time taken to find the element: 25.25 microseconds Element at index 7152 is 7152 find 7153 Time taken to find the element: 18.25 microseconds Element at index 2981 is 2981 find 2982 Time taken to find the element: 8.5 microseconds Element at index 2102 is 2102 find 2103 Time taken to find the element: 6.292 microseconds Element at index 5273 is 5275 find 5276 Time taken to find the element: 13.709 microseconds Element at index 8957 is 8963 find 8964 Time taken to find the element: 22.417 microseconds Element at index 1154 is 1154 find 1155 Time taken to find the element: 4 microseconds Element at index 9198 is 9206 find 9207 Time taken to find the element: 23.292 microseconds Element at index 7305 is 7310 find 7311 Time taken to find the element: 21.625 microseconds

For 10,000 viewers, each increase is 0.02ms. I think the key point is to run some benchmarks and gather data when you are concerned about performance issues.

suzp1984

seems good now.

suzp1984 · 2024-08-31T14:51:07Z

trunk/src/app/srs_app_http_stream.cpp

@@ -1075,6 +1094,7 @@ void SrsHttpStreamServer::http_unmount(SrsRequest* r)

    // Notify cache and stream to stop.
    if (stream->entry) stream->entry->enabled = false;
+    stream->expire();
    cache->stop();

    // Wait for cache and stream to stop.


stream->expire() will disconnect the SrsHTTPConn soon, so no long to wait 120 seconds below. Maybe 10 seconds is OK.

No essential difference. I want keep it.

) When stopping the stream, it will wait for the HTTP Streaming to exit. If the HTTP Streaming goroutine hangs, it will not exit automatically. ```cpp void SrsHttpStreamServer::http_unmount(SrsRequest* r) { SrsUniquePtr<SrsLiveStream> stream(entry->stream); if (stream->entry) stream->entry->enabled = false; srs_usleep(...); // Wait for about 120s. mux.unhandle(entry->mount, stream.get()); // Free stream. } srs_error_t SrsLiveStream::serve_http(ISrsHttpResponseWriter* w, ISrsHttpMessage* r) { err = do_serve_http(w, r); // If stuck in here for 120s+ alive_viewers_--; // Crash at here, because stream has been deleted. ``` We should notify http stream connection to interrupt(expire): ```cpp void SrsHttpStreamServer::http_unmount(SrsRequest* r) { SrsUniquePtr<SrsLiveStream> stream(entry->stream); if (stream->entry) stream->entry->enabled = false; stream->expire(); // Notify http stream to interrupt. ``` Note that we should notify all viewers pulling stream from this http stream. Note that we have tried to fix this issue, but only try to wait for all viewers to quit, without interrupting the viewers, see #4144 --------- Co-authored-by: Jacob Su <suzp1984@gmail.com>

HTTP-FLV: Notify connection to expire.

f6736af

winlinvip added the EnglishNative This issue is conveyed exclusively in English. label Aug 31, 2024

winlinvip changed the title ~~HTTP-FLV: Notify connection to expire.~~ HTTP-FLV: Notify connection to expire when unpublishing Aug 31, 2024

Fix warning.

8dc90e0

suzp1984 reviewed Aug 31, 2024

View reviewed changes

Refine code.

5196dcf

suzp1984 approved these changes Aug 31, 2024

View reviewed changes

suzp1984 reviewed Aug 31, 2024

View reviewed changes

Update release to v6.0.152 v7.0.11

db8e747

winlinvip changed the title ~~HTTP-FLV: Notify connection to expire when unpublishing~~ HTTP-FLV: Notify connection to expire when unpublishing. v6.0.152 v7.0.11 Aug 31, 2024

winlinvip added the RefinedByAI Refined by AI/GPT. label Aug 31, 2024

winlinvip merged commit 05c3a42 into ossrs:develop Aug 31, 2024
5 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

HTTP-FLV: Notify connection to expire when unpublishing. v6.0.152 v7.0.11 #4164

HTTP-FLV: Notify connection to expire when unpublishing. v6.0.152 v7.0.11 #4164

winlinvip commented Aug 31, 2024 •

edited

Loading

suzp1984 left a comment

suzp1984 Aug 31, 2024

winlinvip Aug 31, 2024 •

edited

Loading

winlinvip Aug 31, 2024 •

edited

Loading

suzp1984 Aug 31, 2024

suzp1984 Aug 31, 2024

winlinvip Aug 31, 2024

suzp1984 Aug 31, 2024

winlinvip Aug 31, 2024

suzp1984 left a comment

suzp1984 Aug 31, 2024

winlinvip Aug 31, 2024

HTTP-FLV: Notify connection to expire when unpublishing. v6.0.152 v7.0.11 #4164

HTTP-FLV: Notify connection to expire when unpublishing. v6.0.152 v7.0.11 #4164

Conversation

winlinvip commented Aug 31, 2024 • edited Loading

suzp1984 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

winlinvip Aug 31, 2024 • edited Loading

Choose a reason for hiding this comment

winlinvip Aug 31, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

suzp1984 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

winlinvip commented Aug 31, 2024 •

edited

Loading

winlinvip Aug 31, 2024 •

edited

Loading

winlinvip Aug 31, 2024 •

edited

Loading