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

accurev plugin triggers rebuild if slave that holds workspace is offline

    XMLWordPrintable

Details

    • Improvement
    • Status: Closed (View Workflow)
    • Minor
    • Resolution: Fixed
    • accurev-plugin
    • None
    • Distributed master/slave arrangement. Project configured to get its source from accurev and to poll accurev for changes.

    Description

      If the slave that build the most-recent build on an accurev project goes offline, the project rebuilds, even if no changes happened and hence no rebuild is required.

      JENKINS-1348 describes (and fixed) an issue whereby builds would be triggered if the slave that did the last build went offline (even temporarily). The fix extended the SCM base class and then relied on SCM implementations to override that behaviour where appropriate.
      However, no corresponding change was made to the accurev plugin.

      The accurev plugin code needs to implement requiresWorkspaceForPolling() and return false (as its polling method doesn't rely on a workspace).

      Attachments

        Issue Links

          Activity

            robsimon robsimon added a comment -

            We have the same issue and we started to looked into it. We tweak a bit around with overriding

            public boolean requiresWorkspaceForPolling() {return false;}

            . However, we still run into some issues where polling fails due to NPE's...

            robsimon robsimon added a comment - We have the same issue and we started to looked into it. We tweak a bit around with overriding public boolean requiresWorkspaceForPolling() { return false ;} . However, we still run into some issues where polling fails due to NPE's...
            robsimon robsimon added a comment -

            has at least 2 issues to be solved

            robsimon robsimon added a comment - has at least 2 issues to be solved
            robsimon robsimon added a comment -

            issue_1:

            D:\AccuRevPlugin_0616b\src\main\java\hudson\plugins\accurev\AccurevSCM.java:768: warning: [deprecation] getWorkspace() in hudson.model.AbstractProject has been deprecated
                              workspace = project.getWorkspace();// new FilePath(new java.io.File("."));
                                                 ^
            

            issue_2:

            Started on 08.08.2011 19:12:57
            ERROR: Failed to record SCM polling
            java.lang.NullPointerException
                    at hudson.plugins.accurev.AccurevSCM.ensureLoggedInToAccurev(AccurevSCM.java:880)
                    at hudson.plugins.accurev.AccurevSCM.pollChanges(AccurevSCM.java:781)
                    at hudson.scm.SCM.poll(SCM.java:374)
                    at hudson.model.AbstractProject.poll(AbstractProject.java:1324)
                    at hudson.triggers.SCMTrigger$Runner.runPolling(SCMTrigger.java:420)
                    at hudson.triggers.SCMTrigger$Runner.run(SCMTrigger.java:449)
                    at hudson.util.SequentialExecutionQueue$QueueEntry.run(SequentialExecutionQueue.java:118)
                    at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:441)
                    at java.util.concurrent.FutureTask$Sync.innerRun(FutureTask.java:303)
                    at java.util.concurrent.FutureTask.run(FutureTask.java:138)
                    at java.util.concurrent.ThreadPoolExecutor$Worker.runTask(ThreadPoolExecutor.java:886)
                    at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:908)
                    at java.lang.Thread.run(Thread.java:662)
            
            robsimon robsimon added a comment - issue_1: D:\AccuRevPlugin_0616b\src\main\java\hudson\plugins\accurev\AccurevSCM.java:768: warning: [deprecation] getWorkspace() in hudson.model.AbstractProject has been deprecated workspace = project.getWorkspace(); // new FilePath( new java.io.File( "." )); ^ issue_2: Started on 08.08.2011 19:12:57 ERROR: Failed to record SCM polling java.lang.NullPointerException at hudson.plugins.accurev.AccurevSCM.ensureLoggedInToAccurev(AccurevSCM.java:880) at hudson.plugins.accurev.AccurevSCM.pollChanges(AccurevSCM.java:781) at hudson.scm.SCM.poll(SCM.java:374) at hudson.model.AbstractProject.poll(AbstractProject.java:1324) at hudson.triggers.SCMTrigger$Runner.runPolling(SCMTrigger.java:420) at hudson.triggers.SCMTrigger$Runner.run(SCMTrigger.java:449) at hudson.util.SequentialExecutionQueue$QueueEntry.run(SequentialExecutionQueue.java:118) at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:441) at java.util.concurrent.FutureTask$Sync.innerRun(FutureTask.java:303) at java.util.concurrent.FutureTask.run(FutureTask.java:138) at java.util.concurrent.ThreadPoolExecutor$Worker.runTask(ThreadPoolExecutor.java:886) at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:908) at java.lang. Thread .run( Thread .java:662)
            pjdarton pjdarton added a comment -

            I guess I should have anticipated that this would not be as simple as I first thought

            The code attached contains the comment:
            // MIB: pollChanges will be called with null workspace since we overrode requiresWorkspaceForPolling to return false
            // now trying to use projects workspace directory (local?)

            I'd guess that this is the reason that line 880:
            accurevEnv.put("ACCUREV_HOME", workspace.getParent().getRemote());
            is throwing a NPE.

            What that line needs to do is to set ACCUREV_HOME to a fixed location that'll be consistent on any given master/slave machine, but which doesn't need to be the same on different machines (ACCUREV_HOME controls where accurev will put its .accurev folder, which contains the login authentication tokens).
            The current code sets it to the "workspaces" folder, which works well from accurev's point of view. What we need to do is to change the method by which this folder is being obtained so that it no longer requires workspace to be valid.

            I think that there are probably Jenkins methods/properties that can be used to obtain this (but I don't know offhand what they are) - what's needed is for the polling to get the directory where the workspaces live on the local machine, in a manner that doesn't require "the" workspace.

            The polling can then run the accurev commands from this workspaces folder, whereas the "checkout code into folder" method should continue to run accurev from the active workspace folder where the build is taking place.

            pjdarton pjdarton added a comment - I guess I should have anticipated that this would not be as simple as I first thought The code attached contains the comment: // MIB: pollChanges will be called with null workspace since we overrode requiresWorkspaceForPolling to return false // now trying to use projects workspace directory (local?) I'd guess that this is the reason that line 880: accurevEnv.put("ACCUREV_HOME", workspace.getParent().getRemote()); is throwing a NPE. What that line needs to do is to set ACCUREV_HOME to a fixed location that'll be consistent on any given master/slave machine, but which doesn't need to be the same on different machines (ACCUREV_HOME controls where accurev will put its .accurev folder, which contains the login authentication tokens). The current code sets it to the "workspaces" folder, which works well from accurev's point of view. What we need to do is to change the method by which this folder is being obtained so that it no longer requires workspace to be valid. I think that there are probably Jenkins methods/properties that can be used to obtain this (but I don't know offhand what they are) - what's needed is for the polling to get the directory where the workspaces live on the local machine, in a manner that doesn't require "the" workspace. The polling can then run the accurev commands from this workspaces folder, whereas the "checkout code into folder" method should continue to run accurev from the active workspace folder where the build is taking place.
            pjdarton pjdarton added a comment -

            I've implemented a fix for this and it's attached to JENKINS-10638.

            There's now an additional checkbox on the "Manage Jenkins" -> "Configure System" page, in the "Accurev" section entitled "Poll on master".
            The help says it all (I hope):

            If checked then:

            • The plugin will (only) run accurev polling operations on the master node.
            • Builds will no longer be triggered just because the slave that did the last build goes offline.

            If not checked then:

            • Polling operations will be run on the slave that did the last build.
            • If that slave does offline, a new build will be triggered, whether or not there are changes.
              Do NOT enable this unless accurev is installed on the master node.
            pjdarton pjdarton added a comment - I've implemented a fix for this and it's attached to JENKINS-10638 . There's now an additional checkbox on the "Manage Jenkins" -> "Configure System" page, in the "Accurev" section entitled "Poll on master". The help says it all (I hope): If checked then: The plugin will (only) run accurev polling operations on the master node. Builds will no longer be triggered just because the slave that did the last build goes offline. If not checked then: Polling operations will be run on the slave that did the last build. If that slave does offline, a new build will be triggered, whether or not there are changes. Do NOT enable this unless accurev is installed on the master node.

            released in v0.6.17

            helterscelter helter scelter added a comment - released in v0.6.17

            People

              pjdarton pjdarton
              pjdarton pjdarton
              Votes:
              1 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved: