RestComm / Restcomm-Connect

The Open Source Cloud Communications Platform
http://www.restcomm.com/
GNU Affero General Public License v3.0
240 stars 214 forks source link

Issue zendesk#35767, maxP2PCallLength configuration to extend P2P call, Ask partten for statuscallback #2950

Closed xhoaluu closed 6 years ago

xhoaluu commented 6 years ago

What this PR does / why we need it:

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged): Fixes #

Special notes for your reviewer:

What this PR does Point 1: Add configuration for Max P2P call length in restcomm.xml, that allow customer adjust when RC will release P2P within RC. Point 2: Using ask partten for Completed statusCallBack of Dial verb. Some time, Call actor stop task too soon that make Downloader actor stop as well.

why we need it https://telestax.atlassian.net/browse/RESTCOMM-2139 RC clear P2P call after 60 mins, that make Caller/Callee cannot send BYE to close the call with the other site.

https://telestax.zendesk.com/agent/tickets/35865 [Restcomm]Sometimes, StatusCallBack of "completed" is not created.

Customer confirm it fixed their issue.

xhoaluu commented 6 years ago

Rebase Branch with master. Rerun testsuite again, 4 faliling test are passed in local

org.restcomm.connect.testsuite.sms.push.SmsPushNotificationServerTest.testSmsEndpointMessage org.restcomm.connect.testsuite.telephony.DialRecordingTest.testRecordWithErrorOnRecordAction org.restcomm.connect.testsuite.UssdPullTest.testUssdPullClosedAccount org.restcomm.connect.testsuite.telephony.DialRecordingTest.testDialClientAlice_AliceDisconnects

jaimecasero commented 6 years ago

test results look good,this is ready to merge

xhoaluu commented 6 years ago

@jaimecasero I have added automation task for configure max-p2p-call-length. Please help me to review.

jaimecasero commented 6 years ago

@xhoaluu is this PR covering the fact container is not sending BYE to call participants when sessionTimer is expired? assuming not..., should we cover this behavior on this PR, or this solution is enough for the customer?

jaimecasero commented 6 years ago

@xhoaluu as discussed i added commit to handle session expiration properly

gvagenas commented 6 years ago

@jaimecasero as we discussed on the call, it might be necessary to check what is the application session timeout for akka actor calls because this patch might introducing an unexpected side-effect there. From what I see, the akka call actors are using the akka context receiveTimeout and not the sip timers. I am also checking

gvagenas commented 6 years ago

@jaimecasero Actually for some call flows we store the Akka call actor as an attribute in the application session, for example https://github.com/RestComm/Restcomm-Connect/blob/5292520c2667e8b6190d4c925222c5e5e0da8c5d/restcomm/restcomm.telephony/src/main/java/org/restcomm/connect/telephony/CallManager.java#L1403 but we don't set the sip timer as far as I can see, so there might be a side effect there

jaimecasero commented 6 years ago

@xhoaluu this has been merged, next releas will contain all fixes