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
    • Similar Issues:
      Show 5 results

      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

        Attachments

          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: