Uploaded image for project: 'Jenkins'
  1. Jenkins
  2. JENKINS-8990

Ping interval for JNLP slaves should be configurable

    Details

    • Type: Improvement
    • Status: Closed
    • Priority: Minor
    • Resolution: Fixed
    • Component/s: core
    • Labels:
      None

      Description

      JNLP slaves have a fixed 10 minute ping interval to keep the connection alive. However, some people have proxies with a shorter timeout. The ping interval should be configurable.

      This idea has been mentioned before, for example at http://jenkins.361315.n4.nabble.com/JNLP-Slave-client-disconnects-connection-automatically-tp3309341p3313395.html

        Activity

        Hide
        nparry Nathan Parry added a comment -

        I've submitted a pull request to implement this (https://github.com/jenkinsci/jenkins/pull/70)

        Show
        nparry Nathan Parry added a comment - I've submitted a pull request to implement this ( https://github.com/jenkinsci/jenkins/pull/70 )
        Hide
        kohsuke Kohsuke Kawaguchi added a comment - - edited

        Thanks for the change.

        I think it'd be easier and more uniform to set up ping after the channel is established, without making it dependent on a particular launch scheme. You can do so by implementing ComputerListener and checking preOnline, and use Channel.call and setup a ping thread on both sides.

        This also allows us to package this functionality in a plugin, which addresses my other concern of having too many configuration options in the UI out of the box. I feel that this option is too advanced for most users.

        I also wonder if we should have a shorter default ping interval if 10 minutes in the core is too long for too many environments. I think it's hard for people to understand the root cause from the failure mode (of dropped connections), and I'd rather be inefficient (by issuing lots of pings) than broken by default.

        Show
        kohsuke Kohsuke Kawaguchi added a comment - - edited Thanks for the change. I think it'd be easier and more uniform to set up ping after the channel is established, without making it dependent on a particular launch scheme. You can do so by implementing ComputerListener and checking preOnline, and use Channel.call and setup a ping thread on both sides. This also allows us to package this functionality in a plugin, which addresses my other concern of having too many configuration options in the UI out of the box. I feel that this option is too advanced for most users. I also wonder if we should have a shorter default ping interval if 10 minutes in the core is too long for too many environments. I think it's hard for people to understand the root cause from the failure mode (of dropped connections), and I'd rather be inefficient (by issuing lots of pings) than broken by default.
        Hide
        nparry Nathan Parry added a comment -

        Good points, I'll take a look at those things.

        I wonder if like you said trying to customize this is just over-thinking it and we should simply make the ping much more frequent.

        Show
        nparry Nathan Parry added a comment - Good points, I'll take a look at those things. I wonder if like you said trying to customize this is just over-thinking it and we should simply make the ping much more frequent.
        Hide
        nparry Nathan Parry added a comment -

        I reworked the change using the suggested approach, and indeed it is much cleaner! (Commit)

        Show
        nparry Nathan Parry added a comment - I reworked the change using the suggested approach, and indeed it is much cleaner! ( Commit )
        Hide
        nparry Nathan Parry added a comment -

        Oops, forgot to resubmit a pull requests - new pull req is https://github.com/jenkinsci/jenkins/pull/74

        Show
        nparry Nathan Parry added a comment - Oops, forgot to resubmit a pull requests - new pull req is https://github.com/jenkinsci/jenkins/pull/74
        Hide
        scm_issue_link SCM/JIRA link daemon added a comment -

        Code changed in jenkins
        User: Nathan Parry
        Path:
        core/src/main/java/hudson/slaves/ChannelPinger.java
        remoting/src/main/java/hudson/remoting/Engine.java
        http://jenkins-ci.org/commit/jenkins/18327e9de69b2937ce29730071ba818899c7ac51
        Log:
        [FIXED JENKINS-8990] Configurable ping interval

        This lets you configure the ping interval for slaves via a
        system property. (hudson.slaves.ChannelPinger.pingInterval)

        The default ping interval has been lowered from 10 to 5 minutes.

        It also moves the ping setup logic into a ComputerListener
        (out of the JNLP slave Engine class). As a side effect, all
        slaves will now have a ping instead of just JNLP slaves.

        Show
        scm_issue_link SCM/JIRA link daemon added a comment - Code changed in jenkins User: Nathan Parry Path: core/src/main/java/hudson/slaves/ChannelPinger.java remoting/src/main/java/hudson/remoting/Engine.java http://jenkins-ci.org/commit/jenkins/18327e9de69b2937ce29730071ba818899c7ac51 Log: [FIXED JENKINS-8990] Configurable ping interval This lets you configure the ping interval for slaves via a system property. (hudson.slaves.ChannelPinger.pingInterval) The default ping interval has been lowered from 10 to 5 minutes. It also moves the ping setup logic into a ComputerListener (out of the JNLP slave Engine class). As a side effect, all slaves will now have a ping instead of just JNLP slaves.
        Hide
        scm_issue_link SCM/JIRA link daemon added a comment -
        Show
        scm_issue_link SCM/JIRA link daemon added a comment - Code changed in jenkins User: Kohsuke Kawaguchi Path: changelog.html http://jenkins-ci.org/commit/jenkins/9c8fcf2e75f13312b1c7067716aaef0b5b95d61e Log: recording JENKINS-8990 fix Compare: https://github.com/jenkinsci/jenkins/compare/67cd5b4...9c8fcf2
        Hide
        dogfood dogfood added a comment -

        Integrated in jenkins_main_trunk #620
        [FIXED JENKINS-8990] Configurable ping interval
        recording JENKINS-8990 fix

        Kohsuke Kawaguchi : 18327e9de69b2937ce29730071ba818899c7ac51
        Files :

        • core/src/main/java/hudson/slaves/ChannelPinger.java
        • remoting/src/main/java/hudson/remoting/Engine.java

        Kohsuke Kawaguchi : 9c8fcf2e75f13312b1c7067716aaef0b5b95d61e
        Files :

        • changelog.html
        Show
        dogfood dogfood added a comment - Integrated in jenkins_main_trunk #620 [FIXED JENKINS-8990] Configurable ping interval recording JENKINS-8990 fix Kohsuke Kawaguchi : 18327e9de69b2937ce29730071ba818899c7ac51 Files : core/src/main/java/hudson/slaves/ChannelPinger.java remoting/src/main/java/hudson/remoting/Engine.java Kohsuke Kawaguchi : 9c8fcf2e75f13312b1c7067716aaef0b5b95d61e Files : changelog.html
        Hide
        scm_issue_link SCM/JIRA link daemon added a comment -

        Code changed in jenkins
        User: Nathan Parry
        Path:
        core/src/main/java/hudson/slaves/ChannelPinger.java
        remoting/src/main/java/hudson/remoting/Engine.java
        http://jenkins-ci.org/commit/jenkins/18327e9de69b2937ce29730071ba818899c7ac51
        Log:
        [FIXED JENKINS-8990] Configurable ping interval

        This lets you configure the ping interval for slaves via a
        system property. (hudson.slaves.ChannelPinger.pingInterval)

        The default ping interval has been lowered from 10 to 5 minutes.

        It also moves the ping setup logic into a ComputerListener
        (out of the JNLP slave Engine class). As a side effect, all
        slaves will now have a ping instead of just JNLP slaves.

        Show
        scm_issue_link SCM/JIRA link daemon added a comment - Code changed in jenkins User: Nathan Parry Path: core/src/main/java/hudson/slaves/ChannelPinger.java remoting/src/main/java/hudson/remoting/Engine.java http://jenkins-ci.org/commit/jenkins/18327e9de69b2937ce29730071ba818899c7ac51 Log: [FIXED JENKINS-8990] Configurable ping interval This lets you configure the ping interval for slaves via a system property. (hudson.slaves.ChannelPinger.pingInterval) The default ping interval has been lowered from 10 to 5 minutes. It also moves the ping setup logic into a ComputerListener (out of the JNLP slave Engine class). As a side effect, all slaves will now have a ping instead of just JNLP slaves.
        Hide
        scm_issue_link SCM/JIRA link daemon added a comment -

        Code changed in jenkins
        User: Kohsuke Kawaguchi
        Path:
        changelog.html
        http://jenkins-ci.org/commit/jenkins/9c8fcf2e75f13312b1c7067716aaef0b5b95d61e
        Log:
        recording JENKINS-8990 fix

        Show
        scm_issue_link SCM/JIRA link daemon added a comment - Code changed in jenkins User: Kohsuke Kawaguchi Path: changelog.html http://jenkins-ci.org/commit/jenkins/9c8fcf2e75f13312b1c7067716aaef0b5b95d61e Log: recording JENKINS-8990 fix

          People

          • Assignee:
            Unassigned
            Reporter:
            nparry Nathan Parry
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: