[jitsi-dev] [jitsi-videobridge] Uses the thread pool to handle messages coming from the usrsctp stack, (#177)


#1

in order to ensure that the calling thread does not block. We have
observed that blocking this thread (e.g. by having it eventually try
to write to a Socket) results in calls to usrsctp_socket to block (i.e.
to our inability to initialize any new SCTP sockets).
You can view, comment on, or merge this pull request online at:

  https://github.com/jitsi/jitsi-videobridge/pull/177

-- Commit Summary --

  * Uses the thread pool to handle messages coming from the usrsctp stack,

-- File Changes --

    M src/main/java/org/jitsi/videobridge/SctpConnection.java (14)

-- Patch Links --

https://github.com/jitsi/jitsi-videobridge/pull/177.patch
https://github.com/jitsi/jitsi-videobridge/pull/177.diff

···

---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jitsi/jitsi-videobridge/pull/177


#2

@@ -943,8 +943,16 @@ public void onConnOut(SctpSocket s, byte[] packet)
                             packet);
                 }

- // Send through DTLS transport
- transformer.sendApplicationData(packet, 0, packet.length);
+ // Send through DTLS transport.
+ threadPool.execute(new Runnable()

I'm not absolutely comfortable with sending each "packet" in a separate Thread of "threadPool". You said the TCP socket would block for very roughly 30 seconds. DominantSpeakerIdentification runs at an interval of 300 milliseconds which in the worst case would result in 100 "threadPool" Threads. I'd rather we use a packet queue and send it in a single Thread (of "threadPool"). Anyway, I'll think about it a tiny bit more.

···

---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jitsi/jitsi-videobridge/pull/177/files/da01034413018f00e6d763acc17e162b6741b8c1#r56774315


#3

@@ -943,8 +943,16 @@ public void onConnOut(SctpSocket s, byte[] packet)
                             packet);
                 }

- // Send through DTLS transport
- transformer.sendApplicationData(packet, 0, packet.length);
+ // Send through DTLS transport.
+ threadPool.execute(new Runnable()

This would be a queue and a thread for each SctpConnection instance, correct?

···

---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jitsi/jitsi-videobridge/pull/177/files/da01034413018f00e6d763acc17e162b6741b8c1#r56777002


#4

@@ -943,8 +943,16 @@ public void onConnOut(SctpSocket s, byte[] packet)
                             packet);
                 }

- // Send through DTLS transport
- transformer.sendApplicationData(packet, 0, packet.length);
+ // Send through DTLS transport.
+ threadPool.execute(new Runnable()

Yup. If you think we're fine though, I'll merge this the way it is.

···

---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jitsi/jitsi-videobridge/pull/177/files/da01034413018f00e6d763acc17e162b6741b8c1#r56778561


#5

After a discussion with @lyubomir we agreed to go with a queue.

···

---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jitsi/jitsi-videobridge/pull/177#issuecomment-199401854


#6

Closed #177.

···

---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jitsi/jitsi-videobridge/pull/177#event-597526074