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

Make timer B configurable #75

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

juggernaut
Copy link

For fast failover of an initial INVITE, we want to be able to configure
timer B to a value lower than 32 seconds. This is similar to how
opensips allows you to configure $T_fr_timeout (see
http://opensips.org/pipermail/users/2014-October/030150.html)

For fast failover of an initial INVITE, we want to be able to configure
timer B to a value lower than 32 seconds. This is similar to how
opensips allows you to configure $T_fr_timeout (see
http://opensips.org/pipermail/users/2014-October/030150.html)
@deruelle deruelle added this to the 2.0.0 milestone Mar 17, 2016
@deruelle
Copy link
Member

@juggernaut Thanks for the contribution. What you're proposing is to be able to specify Timer B independently of T1 ? I see from the RFC https://tools.ietf.org/html/rfc3261#appendix-A that it's coupled to T1 although I'm not entirely sure if it's only for the default value or not but I always assumed it was dependent on T1 by reading the relevant section of the RFC:

For any transport, the client transaction MUST start timer B with a value of 64*T1 seconds (Timer B controls transaction timeouts)

This is the reason why Timer B (and some others Timers) is not configurable in the stack. Can you describe why you want to set this timer independently of T1 ?

@juggernaut
Copy link
Author

Hi @deruelle , I have opened case 1866 on the telestax portal with details. Happy to continue the conversation here as well. Let me know what you prefer.

@deruelle
Copy link
Member

OK sure. We will look at it there then.

@juggernaut
Copy link
Author

@deruelle looks like I filed the ticket on desk.com instead of zendesk.com. The new request # is 33062

@jaimecasero
Copy link
Contributor

Reviewed the pull request. The code seems fair, although I dont like computations inside setter method, but its following current convention followed for the rest of configurable Timers. We may leave that to future refactoring.

Just one thought, how do you forsee the relation of Timer B and Timer H as specified at RFC 3261 [http://www.tech-invite.com/y30/tinv-ietf-rfc-3261-7.html#e-17-2-1]?:

When the "Completed" state is entered, timer H MUST be set to fire in
64*T1 seconds for all transports. Timer H determines when the server
transaction abandons retransmitting the response. Its value is
chosen to equal Timer B....

How do you interpret RFC statement? was TimerH decided to be initiated with TimerB value? Should we reconfigure Timer H on Timer B setting ?

For example, in setTimerT4 we can see TimerI and TimerK are reconfigured...

@jaimecasero
Copy link
Contributor

@juggernaut any thoughts on latest comments?

@juggernaut
Copy link
Author

@jaimecasero Sorry for the late response, I have been busy with other things. Re: Timer H, I interpret the statement in the RFC to mean that it is set to be equal to the value of timer B, but it is still defined in terms of T1 (See www.tech-invite.com/y30/tinv-ietf-rfc-3261-13.html#e-A). Timer I and Timer K are directly defined as functions of T4, so it makes sense to reconfigure them if T4 is changed. So, I feel that it should be okay for Timer H and Timer B values to be different. What do you think?

@jaimecasero
Copy link
Contributor

@juggernaut

I think, apart form RFC interpretation, changing default timers is an advance feature. So we may let the users decide on their own here.

Before accepting contribution, would you mind checking our agreement at CLA agreement . If you are fine with it, feel free to submit. If not, please let me know your issue so we can try to tackle it somehow...

@atsakiridis
Copy link

Hey guys, just to pass my positive vote for this feature, as it's something that would come in handy in restcomm-android-sdk as well

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

Successfully merging this pull request may close these issues.

5 participants