Add ability to reconnect based on number of failed ping requests #18
Conversation
…s can be configured in the sip-comminicator.properties by setting the property configPropertiesBase.RECONNECT_ON_PING_FAILURES to true. To maintain the original behavior this is set to false by default. Setting this value will cause the connection to be reset after the number of pings set in the ping threshold fail.
|
bump... does this repo get love anymore? |
…emoved from the ComponentManager the connection is disconnected and properly reset
| @@ -271,6 +287,25 @@ public boolean isConnectionAlive() | |||
| } | |||
| } | |||
|
|
|||
| /** | |||
| * Restart the component connection if we have exceeded the threshold for ping failures | |||
There was a problem hiding this comment.
Could you please break this comment to fit in 80 chars.
|
Thank you very much for the detailed description. Reading it has helped me understand why we need this. Could you please add this text (or something similar) in the commit description or inside the code? Other than that, the code looks good to me. |
…ers through an ELB. When deploys of the XMPP server happen, the nodes are terminated but the connection to the ELB still stays up, even though the nodes behind it have been removed. This causes the videobridge to think that it still has a connection to the XMPP server. Although the connection still appears valid, nothing is there to respond to pings or pubsub messages so the videobridge gets into a bad state. Forcing the underlying connection to reset based on failed ping requests helped to solve this for us. The jitsi videobridge pom.xml will need to be updated to use the new version of Jicoco once this is merged into master. I'm not sure of the proper way to make that happen together so please advise here. Also did not see any documentation for jicoco and would be happy to start some but it felt like since this is used in the videobridge that the documentation should go there. Please advise as to where I should document this new configuration property. The new configuration property can be set in the sip-comminicator.properties by adding the property configPropertiesBase.RECONNECT_ON_PING_FAILURES and setting it to true. To maintain the original behavior is treated as false by default. Setting this value will cause the connection to be reset after the number of pings set in the ping threshold fail.
|
Updated the commit with your comments, thank you for reviewing. |
…ers through an ELB. When deploys of the XMPP server happen, the nodes are terminated but the connection to the ELB still stays up, even though the nodes behind it have been removed. This causes the videobridge to think that it still has a connection to the XMPP server. Although the connection still appears valid, nothing is there to respond to pings or pubsub messages so the videobridge gets into a bad state. Forcing the underlying connection to reset based on failed ping requests helped to solve this for us. The jitsi videobridge pom.xml will need to be updated to use the new version of Jicoco once this is merged into master. I'm not sure of the proper way to make that happen together so please advise here. Also did not see any documentation for jicoco and would be happy to start some but it felt like since this is used in the videobridge that the documentation should go there. Please advise as to where I should document this new configuration property. The new configuration property can be set in the sip-comminicator.properties by adding the property configPropertiesBase.RECONNECT_ON_PING_FAILURES and setting it to true. To maintain the original behavior is treated as false by default. Setting this value will cause the connection to be reset after the number of pings set in the ping threshold fail.
|
@gpolitis anything else you would like me to update on this PR? Thank you again for reviewing it |
|
bump? |
|
Hi @davidertel, I apologize for the late response. I'm sorry to bother you with this, but could you please make sure that your code does not go beyond 80 chars? You can configure IntelliJ IDEA (or any other tool) to show you a ruler. Thanks very much and again, I'm sorry for bothering with this and for taking so long to reply. |
|
Hi, thanks for your contribution! |
|
Signed that several months ago. You should have it on record
|
|
We do, sorry for the noise. Jenkins: add to whitelist. |
|
merged in the latest from master to update this PR, reduced the width of the code to 80 characters... who has a screen that only supports 80 characters? email me and I'll send you a nice widescreen :-) |
|
Yep, there were a lot of discussions on the 80 chars online and offline, but one big + for that one is mobile. Also you should count it 160, normally you look diffs of two files side by side. |
|
Anything further needed for this PR before merging? |
|
Hi, I encounter the same problem when using NLB to connect to xmpp server for jvb. |
When running jitsi videobridge in amazon, we connect to our XMPP servers through an ELB. When deploys of the XMPP server happen, the nodes are terminated but the connection to the ELB still stays up, even though the nodes behind it have been removed. This causes the videobridge to think that it still has a connection to the XMPP server. Although the connection still appears valid, nothing is there to respond to pings or pubsub messages so the videobridge gets into a bad state. Forcing the underlying connection to reset based on failed ping requests helped to solve this for us.
The jitsi videobridge pom.xml will need to be updated to use the new version of Jicoco once this is merged into master. I'm not sure of the proper way to make that happen together so please advise here. Also did not see any documentation for jicoco and would be happy to start some but it felt like since this is used in the videobridge that the documentation should go there. Please advise as to where I should document this new configuration property.
The new configuration property can be set in the sip-comminicator.properties by adding the property configPropertiesBase.RECONNECT_ON_PING_FAILURES and setting it to true. To maintain the original behavior is treated as false by default. Setting this value will cause the connection to be reset after the number of pings set in the ping threshold fail.