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

Matrix parent build shouldn't consume an executor.

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Component/s: matrix-project-plugin
    • Labels:
      None
    • Environment:
      Platform: All, OS: All

      Description

      Hi,

      On a multi-configuration project, I encounter a systematic deadlock because the
      parent job and one of its child are queued on the same node.

      The configuration matrix is as follows:
      [x] Build on multiple nodes
      [-] Individual nodes
      [x] APOLLON (apollon)
      [x] CHRONOS (chronos)
      [x] DEMETER (demeter)
      [ ] EOS (eos)
      [x] EROS (eros)
      [x] PAN (pan)
      [x] PONTOS (pontos)
      [ ] master (the master Hudson node)
      [+] Labels
      [ ] Axes

      Here is the console output:

      started
      Building remotely on PONTOS
      Triggering label=PAN
      Triggering label=APOLLON
      Triggering label=DEMETER
      Triggering label=PONTOS
      Triggering label=EROS
      Triggering label=CHRONOS

      All jobs except the ones on "PONTOS" successfully terminate.
      The parent job keeps executing, waiting for all its children termination, but it
      never stops as the child job supposed to run on "PONTOS" remains in the build queue.

      Enabling the master Hudson node has no effect. Its corresponding job also
      remains in the build queue.

      Thank you for reading.

      Regards,
      Regis.

        Issue Links

          Activity

          Hide
          kohsuke Kohsuke Kawaguchi added a comment -

          Right. The parent build needs to be run outside of the normal executors so that
          it can let child builds use that executor.

          In the mean time, a work around is to tie the parent build to somewhere where
          more executors are available.

          Show
          kohsuke Kohsuke Kawaguchi added a comment - Right. The parent build needs to be run outside of the normal executors so that it can let child builds use that executor. In the mean time, a work around is to tie the parent build to somewhere where more executors are available.
          Hide
          rdesgroppes Régis Desgroppes added a comment -

          I don't see how to put in place the workaround you propose, in that the
          multi-project configuration page doesn't allow to tie the parent build to a
          dedicated node.

          Show
          rdesgroppes Régis Desgroppes added a comment - I don't see how to put in place the workaround you propose, in that the multi-project configuration page doesn't allow to tie the parent build to a dedicated node.
          Hide
          kohsuke Kohsuke Kawaguchi added a comment -
              • Issue 961 has been marked as a duplicate of this issue. ***
          Show
          kohsuke Kohsuke Kawaguchi added a comment - Issue 961 has been marked as a duplicate of this issue. ***
          Hide
          kohsuke Kohsuke Kawaguchi added a comment -
              • Issue 1432 has been marked as a duplicate of this issue. ***
          Show
          kohsuke Kohsuke Kawaguchi added a comment - Issue 1432 has been marked as a duplicate of this issue. ***
          Hide
          musilt2 musilt2 added a comment -
              • Issue 1561 has been marked as a duplicate of this issue. ***
          Show
          musilt2 musilt2 added a comment - Issue 1561 has been marked as a duplicate of this issue. ***
          Hide
          musilt2 musilt2 added a comment -

          what's the current status of this issue? any time-frame when the fix will be
          available?

          Show
          musilt2 musilt2 added a comment - what's the current status of this issue? any time-frame when the fix will be available?
          Hide
          mirilovic mirilovic added a comment -

          It would be great to have a possibility to tie the parent job to particular
          machine - and I personally think the best one would be Master.

          Show
          mirilovic mirilovic added a comment - It would be great to have a possibility to tie the parent job to particular machine - and I personally think the best one would be Master.
          Hide
          lloydchang Lloyd Chang added a comment -

          Kohsuke, unless I'm mistaken, a code change in the Hudson Core is needed to even
          try the work-around.

          Like others, I'm not sure how to configure Hudson to tie the parent build to a
          specific node. In node selection, I selected 1 label for multiple axes to use,
          but I don't see any options for the parent build.

          Show
          lloydchang Lloyd Chang added a comment - Kohsuke, unless I'm mistaken, a code change in the Hudson Core is needed to even try the work-around. Like others, I'm not sure how to configure Hudson to tie the parent build to a specific node. In node selection, I selected 1 label for multiple axes to use, but I don't see any options for the parent build.
          Hide
          scm_issue_link SCM/JIRA link daemon added a comment -

          Code changed in hudson
          User: : kohsuke
          Path:
          http://fisheye4.cenqua.com/changelog/hudson/?cs=19911
          Log:
          JENKINS-936 Created a branch to experiment with the solution.

          Show
          scm_issue_link SCM/JIRA link daemon added a comment - Code changed in hudson User: : kohsuke Path: http://fisheye4.cenqua.com/changelog/hudson/?cs=19911 Log: JENKINS-936 Created a branch to experiment with the solution.
          Hide
          kohsuke Kohsuke Kawaguchi added a comment -

          Note to myself. There's two ways to do this.

          One is to just add one more executor temporarily to compensate the effect. This
          is easy, but the downside is that the added executor may end up doing something
          else, and it might take a bit of time before they get released. Plus this will
          cause a UI discrepancy between the user setting vs what they see.

          Another is to add subtype of Executor and let it run just so that it can execute
          the parent build. The trick is to find a situation where an executor shouldn't
          run (like Hudson is shutting down) so that we won't lose the item.

          Show
          kohsuke Kohsuke Kawaguchi added a comment - Note to myself. There's two ways to do this. One is to just add one more executor temporarily to compensate the effect. This is easy, but the downside is that the added executor may end up doing something else, and it might take a bit of time before they get released. Plus this will cause a UI discrepancy between the user setting vs what they see. Another is to add subtype of Executor and let it run just so that it can execute the parent build. The trick is to find a situation where an executor shouldn't run (like Hudson is shutting down) so that we won't lose the item.
          Hide
          ridesmet Ringo De Smet added a comment -

          Kohsuke, from your last comment, I think your second suggestion is a better solution. Just add a "virtual"
          executor to the master that can run the composite builds, and also making the composite build of type
          "virtual".

          Show
          ridesmet Ringo De Smet added a comment - Kohsuke, from your last comment, I think your second suggestion is a better solution. Just add a "virtual" executor to the master that can run the composite builds, and also making the composite build of type "virtual".
          Hide
          scm_issue_link SCM/JIRA link daemon added a comment -

          Code changed in hudson
          User: : kohsuke
          Path:
          branches/matrix-parent/core/src/main/java/hudson/matrix/MatrixProject.java
          branches/matrix-parent/core/src/main/java/hudson/model/Computer.java
          branches/matrix-parent/core/src/main/java/hudson/model/Executor.java
          branches/matrix-parent/core/src/main/java/hudson/model/OneOffExecutor.java
          branches/matrix-parent/core/src/main/java/hudson/model/Queue.java
          branches/matrix-parent/core/src/main/resources/lib/hudson/executors.jelly
          http://fisheye4.cenqua.com/changelog/hudson/?cs=19998
          Log:
          JENKINS-936 I believe this should do.

          Show
          scm_issue_link SCM/JIRA link daemon added a comment - Code changed in hudson User: : kohsuke Path: branches/matrix-parent/core/src/main/java/hudson/matrix/MatrixProject.java branches/matrix-parent/core/src/main/java/hudson/model/Computer.java branches/matrix-parent/core/src/main/java/hudson/model/Executor.java branches/matrix-parent/core/src/main/java/hudson/model/OneOffExecutor.java branches/matrix-parent/core/src/main/java/hudson/model/Queue.java branches/matrix-parent/core/src/main/resources/lib/hudson/executors.jelly http://fisheye4.cenqua.com/changelog/hudson/?cs=19998 Log: JENKINS-936 I believe this should do.
          Hide
          kohsuke Kohsuke Kawaguchi added a comment -

          Hudson 1.317 will include the fix for this, but because of a potential impact to
          users, the fix is disabled by default for now. I'd like interested parties to
          enable this (by setting the system property
          hudson.model.Hudson.flyweightSupport=true on Hudson JVM), and report back if
          this is working OK for you.

          If the fix appears to work without any side effect, I'll enable the fix by default.

          Show
          kohsuke Kohsuke Kawaguchi added a comment - Hudson 1.317 will include the fix for this, but because of a potential impact to users, the fix is disabled by default for now. I'd like interested parties to enable this (by setting the system property hudson.model.Hudson.flyweightSupport=true on Hudson JVM), and report back if this is working OK for you. If the fix appears to work without any side effect, I'll enable the fix by default.
          Hide
          hydraswitch hydraswitch added a comment -

          I set the property as described and hit Build Now for my multi configuration job.
          The one Node that I did not select the job for is now showing a Dead thread?
          or something. Not sure what happened. It goes away if I stop and restart hudson.
          It comes back again each time the job runs. It seems to run correctly on the nodes
          that I checked it on for.

          Show
          hydraswitch hydraswitch added a comment - I set the property as described and hit Build Now for my multi configuration job. The one Node that I did not select the job for is now showing a Dead thread? or something. Not sure what happened. It goes away if I stop and restart hudson. It comes back again each time the job runs. It seems to run correctly on the nodes that I checked it on for.
          Hide
          emmulator emmulator added a comment -

          I was hoping this might help with the problems I've encountered in Issue 1022.
          It does seem to address the problem of the deadlock, but without the ability to
          have the parent job run on a different slave node from any of the children, the
          interaction with perforce still causes one of the children to not sync its
          workspace.

          I found this issue when looking for how I would expose the
          assignedNode/hasSlaveAffinity property of a MatrixProject, in the hopes that if
          I could tie the parent to a particular node, it would no longer interfere with
          the children. I've noticed that a few other people in this ticket were thinking
          along the same lines, but you have chosen this virtual executor approach
          instead. Is there a reason it would not be desirable to be able to tie the
          parent to a particular node? And even if you don't like that approach for the
          general release, could you please point me to where in the source I would go to
          expose that property? I realize that the 'real' solution to Issue 1022 probably
          involves modifying the perforce plugin, but that seems more complicated, and I
          was hoping this would work as a workaround in the meantime.

          Thanks!

          Show
          emmulator emmulator added a comment - I was hoping this might help with the problems I've encountered in Issue 1022. It does seem to address the problem of the deadlock, but without the ability to have the parent job run on a different slave node from any of the children, the interaction with perforce still causes one of the children to not sync its workspace. I found this issue when looking for how I would expose the assignedNode/hasSlaveAffinity property of a MatrixProject, in the hopes that if I could tie the parent to a particular node, it would no longer interfere with the children. I've noticed that a few other people in this ticket were thinking along the same lines, but you have chosen this virtual executor approach instead. Is there a reason it would not be desirable to be able to tie the parent to a particular node? And even if you don't like that approach for the general release, could you please point me to where in the source I would go to expose that property? I realize that the 'real' solution to Issue 1022 probably involves modifying the perforce plugin, but that seems more complicated, and I was hoping this would work as a workaround in the meantime. Thanks!
          Hide
          huybrechts huybrechts added a comment -

          When using the experimental flyweight support, I noticed that one of my (drools)
          builds was scheduled on an offline jnlp slave. It was very hard to detect, since
          the Drools plugin does not actually use the slave. Because the slave was
          offline, Computer.defaultCharset was null, which results in an NPE in Run when
          the logfile is created.

          Show
          huybrechts huybrechts added a comment - When using the experimental flyweight support, I noticed that one of my (drools) builds was scheduled on an offline jnlp slave. It was very hard to detect, since the Drools plugin does not actually use the slave. Because the slave was offline, Computer.defaultCharset was null, which results in an NPE in Run when the logfile is created.
          Hide
          mdonohue mdonohue added a comment -
              • Issue 4552 has been marked as a duplicate of this issue. ***
          Show
          mdonohue mdonohue added a comment - Issue 4552 has been marked as a duplicate of this issue. ***
          Hide
          nairb774 nairb774 added a comment -

          Digging through the code today I found this feature and turned it on. Sadly, I
          ran into the same problem hydraswitch did. Looking into why the executor/thread
          died shows nothing on the dead page, but in the main hudson log I saw the following:

          Nov 6, 2009 10:40:36 PM hudson.ExpressionFactory2$JexlExpression evaluate
          WARNING: Caught exception evaluating: h.printThrowable(it.causeOfDeath). Reason:
          java.lang.NullPointerException
          java.lang.NullPointerException
          at hudson.Functions.printThrowable(Functions.java:916)
          at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
          <<SNIP - Full stack can be provided on request, but it is mostly non-descriptive
          jelly frames>>

          I am running 1.330 currently and would love for this to work. I took a look
          over the code and the code changes and I can't seem to see where this might be
          falling flat. Anything I can do to help diagnose what hydraswitch and I are seeing?

          Brian

          Show
          nairb774 nairb774 added a comment - Digging through the code today I found this feature and turned it on. Sadly, I ran into the same problem hydraswitch did. Looking into why the executor/thread died shows nothing on the dead page, but in the main hudson log I saw the following: Nov 6, 2009 10:40:36 PM hudson.ExpressionFactory2$JexlExpression evaluate WARNING: Caught exception evaluating: h.printThrowable(it.causeOfDeath). Reason: java.lang.NullPointerException java.lang.NullPointerException at hudson.Functions.printThrowable(Functions.java:916) at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) <<SNIP - Full stack can be provided on request, but it is mostly non-descriptive jelly frames>> I am running 1.330 currently and would love for this to work. I took a look over the code and the code changes and I can't seem to see where this might be falling flat. Anything I can do to help diagnose what hydraswitch and I are seeing? Brian
          Hide
          nairb774 nairb774 added a comment -

          I think I nailed down where the dead executor is coming from. In OneOffExecutor
          you have in the constructor:

          super(owner, -1);
          this.item = item;

          In the super constructor (Executor), the last line is a call to Thread.start.
          If the thread is able to start and complete shouldRun before the "this.item =
          item" line is run, the executor finishes out and shows up dead. On a
          multi-processor computer this race condition is quite common because of the lack
          of locking/fencing.

          Two solutions to this, one move the start call out of the executor constructor,
          and the other is to put in necessary locking around the setting of that field.
          I would argue that the best solution be the moving of the start method, but that
          might be better saved for an enhancement at a later time.

          I will attach a patch in a few minutes that should fix this dead executor problem.

          Show
          nairb774 nairb774 added a comment - I think I nailed down where the dead executor is coming from. In OneOffExecutor you have in the constructor: super(owner, -1); this.item = item; In the super constructor (Executor), the last line is a call to Thread.start. If the thread is able to start and complete shouldRun before the "this.item = item" line is run, the executor finishes out and shows up dead. On a multi-processor computer this race condition is quite common because of the lack of locking/fencing. Two solutions to this, one move the start call out of the executor constructor, and the other is to put in necessary locking around the setting of that field. I would argue that the best solution be the moving of the start method, but that might be better saved for an enhancement at a later time. I will attach a patch in a few minutes that should fix this dead executor problem.
          Hide
          nairb774 nairb774 added a comment -

          Created an attachment (id=995)
          Patch to provide proper locking for OneOffExecutor

          Show
          nairb774 nairb774 added a comment - Created an attachment (id=995) Patch to provide proper locking for OneOffExecutor
          Hide
          hydraswitch hydraswitch added a comment -

          Here's to hoping this will appear in a build soon!
          I'd really like to be able to try it out, but considering all I have
          going on at the moment, I'll not be trying to build Hudson from svn.

          Thanks for the work on this Brian!

          Show
          hydraswitch hydraswitch added a comment - Here's to hoping this will appear in a build soon! I'd really like to be able to try it out, but considering all I have going on at the moment, I'll not be trying to build Hudson from svn. Thanks for the work on this Brian!
          Hide
          mdillon mdillon added a comment -

          Perhaps it would be better to just move the start() call out of the Executor
          constructor. The only place that "new Executor(...)" is called is in
          Computer.setNumExecutors(int) and the only class that extends Executor is
          OneOffExecutor. The text "new Executor" or "extends Executor" doesn't show up in
          any of the plugins either.

          It seems like moving the call to Executor.start() from the Executor constructor
          to the while loop in setNumExecutors would be workable and simpler solution.

          Show
          mdillon mdillon added a comment - Perhaps it would be better to just move the start() call out of the Executor constructor. The only place that "new Executor(...)" is called is in Computer.setNumExecutors(int) and the only class that extends Executor is OneOffExecutor. The text "new Executor" or "extends Executor" doesn't show up in any of the plugins either. It seems like moving the call to Executor.start() from the Executor constructor to the while loop in setNumExecutors would be workable and simpler solution.
          Hide
          mdillon mdillon added a comment -

          In addition, Computer.startFlyWeightTask would also have to know to call start().

          Another possibility would be to add an alternate Executor constructor that takes
          something like this:

          class Initializer<E extends Executor> {
          void initialize(E e);
          }

          This thing could be called from the Executor constructor before calling start().
          Subclasses would be expected to put anything they wanted to initialize before
          start() is called in a new Initializer.

          Show
          mdillon mdillon added a comment - In addition, Computer.startFlyWeightTask would also have to know to call start(). Another possibility would be to add an alternate Executor constructor that takes something like this: class Initializer<E extends Executor> { void initialize(E e); } This thing could be called from the Executor constructor before calling start(). Subclasses would be expected to put anything they wanted to initialize before start() is called in a new Initializer.
          Hide
          nairb774 nairb774 added a comment -

          I would suggest the start method is moved. The one downside to the initalizer
          approach is that from within the initalizer you will not be able to set final
          fields.

          Also, a good rule of thumb is to not expose objects to other threads until the
          constructor returns. Looking in the JLS 17.5 there are the following lines:

          "The detailed semantics of final fields are somewhat different from those of
          normal fields. In particular, compilers have a great deal of freedom to move
          reads of final fields across synchronization barriers and calls to arbitrary or
          unknown methods. Correspondingly, compilers are allowed to keep the value of a
          final field cached in a register and not reload it from memory in situations
          where a non-final field would have to be reloaded."

          and the comment:

          "An object is considered to be completely initialized when its constructor
          finishes. A thread that can only see a reference to an object after that object
          has been completely initialized is guaranteed to see the correctly initialized
          values for that object's final fields."

          The implications (as I read it - and I am by no means an expert) is that even
          with locking and other thread safe actions the only way to be sure final fields
          will be properly visible is after the completion of the constructor. Any time
          earlier then that you are left exposed to the whims compiler and caching
          performed by the cpu.

          We could provide a smarter start method and have it return the executor itself.
          I.e: public Executor start()

          { super.start(); return this; }

          By doing this you
          can chain the constructor and the start in a nice compact manner:
          executors.add(new Executor(...).start()); which is what this feels like it was
          trying to do by placing start at the end of the constructor.

          Show
          nairb774 nairb774 added a comment - I would suggest the start method is moved. The one downside to the initalizer approach is that from within the initalizer you will not be able to set final fields. Also, a good rule of thumb is to not expose objects to other threads until the constructor returns. Looking in the JLS 17.5 there are the following lines: "The detailed semantics of final fields are somewhat different from those of normal fields. In particular, compilers have a great deal of freedom to move reads of final fields across synchronization barriers and calls to arbitrary or unknown methods. Correspondingly, compilers are allowed to keep the value of a final field cached in a register and not reload it from memory in situations where a non-final field would have to be reloaded." and the comment: "An object is considered to be completely initialized when its constructor finishes. A thread that can only see a reference to an object after that object has been completely initialized is guaranteed to see the correctly initialized values for that object's final fields." The implications (as I read it - and I am by no means an expert) is that even with locking and other thread safe actions the only way to be sure final fields will be properly visible is after the completion of the constructor. Any time earlier then that you are left exposed to the whims compiler and caching performed by the cpu. We could provide a smarter start method and have it return the executor itself. I.e: public Executor start() { super.start(); return this; } By doing this you can chain the constructor and the start in a nice compact manner: executors.add(new Executor(...).start()); which is what this feels like it was trying to do by placing start at the end of the constructor.
          Hide
          nairb774 nairb774 added a comment -

          Another note from testing, the isIdle() method and related methods will need to
          be updated. I have a plugin which interfaces with our internal cloud system and
          uses the idle state of the computers to manage undeployment/retention. Fun
          surprise when the computer hosting the parent project of a matrix project goes
          away because it is "idle". I'll post a patch this evening of my fix if I get a
          chance, but I am pretty sure it is missing a few changes to make it a proper
          solution.

          Show
          nairb774 nairb774 added a comment - Another note from testing, the isIdle() method and related methods will need to be updated. I have a plugin which interfaces with our internal cloud system and uses the idle state of the computers to manage undeployment/retention. Fun surprise when the computer hosting the parent project of a matrix project goes away because it is "idle". I'll post a patch this evening of my fix if I get a chance, but I am pretty sure it is missing a few changes to make it a proper solution.
          Hide
          mdillon mdillon added a comment -

          Created an attachment (id=1006)
          Patch to add an ExecutorList class

          Show
          mdillon mdillon added a comment - Created an attachment (id=1006) Patch to add an ExecutorList class
          Hide
          mdillon mdillon added a comment -

          I've attached a patch with an alternate approach for the start() issue.

          The patch adds an ExecutorList subclass of CopyOnWriteArrayList that will ensure
          that the Executor has been started before it is added to the list and made
          available to the rest of Hudson. This way, the start() is still contained to a
          single class, but Executor can be safely subclassed.

          Show
          mdillon mdillon added a comment - I've attached a patch with an alternate approach for the start() issue. The patch adds an ExecutorList subclass of CopyOnWriteArrayList that will ensure that the Executor has been started before it is added to the list and made available to the rest of Hudson. This way, the start() is still contained to a single class, but Executor can be safely subclassed.
          Hide
          mdillon mdillon added a comment -

          P.S. I only implemented add(E). If the add(int,E) and set(int,E) methods need to
          be overridden too, I can update the patch. Looking at the docs for AbstractList,
          it looks like overriding add(int,E) would have been preferable to overriding
          add(E)... I guess I'll update the patch.

          Show
          mdillon mdillon added a comment - P.S. I only implemented add(E). If the add(int,E) and set(int,E) methods need to be overridden too, I can update the patch. Looking at the docs for AbstractList, it looks like overriding add(int,E) would have been preferable to overriding add(E)... I guess I'll update the patch.
          Hide
          mdillon mdillon added a comment -

          Man, I hate when I comment before I finish looking at something thoroughly...

          CopyOnWriteArrayList doesn't extend AbstractList, so it isn't clear what needs
          to be overridden for a comprehensive fix without looking at the source. I guess
          I'll leave this patch as-is for now.

          Show
          mdillon mdillon added a comment - Man, I hate when I comment before I finish looking at something thoroughly... CopyOnWriteArrayList doesn't extend AbstractList, so it isn't clear what needs to be overridden for a comprehensive fix without looking at the source. I guess I'll leave this patch as-is for now.
          Hide
          emmulator emmulator added a comment -

          I have been using the system property
          hudson.model.Hudson.flyweightSupport=true successfully for a couple of weeks now
          to prevent the deadlock. I haven't put the Hudson cluster into 'production'
          yet, but I have been running several builds a day as I determine how best to
          utilize and configure Hudson.

          I still think it might help with issue 1022 to be able to assign the parent Job
          to a particular Node. But I have definitely seen child Jobs of the same Matrix
          build running on the same Node as the parent without any deadlock.

          Show
          emmulator emmulator added a comment - I have been using the system property hudson.model.Hudson.flyweightSupport=true successfully for a couple of weeks now to prevent the deadlock. I haven't put the Hudson cluster into 'production' yet, but I have been running several builds a day as I determine how best to utilize and configure Hudson. I still think it might help with issue 1022 to be able to assign the parent Job to a particular Node. But I have definitely seen child Jobs of the same Matrix build running on the same Node as the parent without any deadlock.
          Hide
          hydraswitch hydraswitch added a comment -

          Regarding the comment from emmulator -

          At least for my situation, the problem is that I do not want the
          jobs to run on the master. I want to be able to setup a multi-configuration
          job that I specify the slave(s) it should be run on. Currently, for
          multi-configuration jobs - Hudson insists on running the job on the master
          as well. The deadlock occurs as a result of using 1 executor.
          I was seeing dead threads when I turned on flyweightSupport.

          Ideally, what I want is:

          • 1 executor per slave (dependency issues in my build tree)
          • 1 or more executors per master (only 1 master)
          • no actual build jobs to run on the master, only on the slaves

          And I'll also add that I'm hopeful that with all the activity on this bug
          we'll eventually get to something that works for me. In a week or two,
          things will calm down enough here that I'll be able to build hudson from
          source and apply that patches to try them out. But I'm still hoping that the
          fix will appear in a "released" build before then.

          Show
          hydraswitch hydraswitch added a comment - Regarding the comment from emmulator - At least for my situation, the problem is that I do not want the jobs to run on the master. I want to be able to setup a multi-configuration job that I specify the slave(s) it should be run on. Currently, for multi-configuration jobs - Hudson insists on running the job on the master as well. The deadlock occurs as a result of using 1 executor. I was seeing dead threads when I turned on flyweightSupport. Ideally, what I want is: 1 executor per slave (dependency issues in my build tree) 1 or more executors per master (only 1 master) no actual build jobs to run on the master, only on the slaves And I'll also add that I'm hopeful that with all the activity on this bug we'll eventually get to something that works for me. In a week or two, things will calm down enough here that I'll be able to build hudson from source and apply that patches to try them out. But I'm still hoping that the fix will appear in a "released" build before then.
          Hide
          mdillon mdillon added a comment -

          I took a minute to look at the code for CopyOnWriteArrayList. Unfortunately,
          there is pretty much no code shared between any of the mutator methods, so the
          only comprehensive way to do this would be to go and put Executor.start() calls
          into an overridden method for each of them.

          Since this class is intended for limited use inside of Hudson, it seems
          reasonable to me to just override the ones other than add(E) to throw
          UnsupportedOperationException instead. They can always be implemented later if
          they are needed.

          Show
          mdillon mdillon added a comment - I took a minute to look at the code for CopyOnWriteArrayList. Unfortunately, there is pretty much no code shared between any of the mutator methods, so the only comprehensive way to do this would be to go and put Executor.start() calls into an overridden method for each of them. Since this class is intended for limited use inside of Hudson, it seems reasonable to me to just override the ones other than add(E) to throw UnsupportedOperationException instead. They can always be implemented later if they are needed.
          Hide
          pcc pcc added a comment -

          While testing the flyweight support I found that Hudson can sometimes schedule
          the parent job on an offline slave leading to an NPE. This seems related to the
          issue huybrechts reported. I developed a patch to prevent this which I am
          attaching to this issue.

          java.lang.NullPointerException
          at hudson.model.Slave.createLauncher(Slave.java:309)
          at hudson.model.AbstractBuild$AbstractRunner.createLauncher(AbstractBuild.java:417)
          at hudson.model.AbstractBuild$AbstractRunner.run(AbstractBuild.java:369)
          at hudson.model.Run.run(Run.java:1198)
          at hudson.matrix.MatrixBuild.run(MatrixBuild.java:149)
          at hudson.model.ResourceController.execute(ResourceController.java:88)
          at hudson.model.Executor.run(Executor.java:123)
          at hudson.model.OneOffExecutor.run(OneOffExecutor.java:60)

          Show
          pcc pcc added a comment - While testing the flyweight support I found that Hudson can sometimes schedule the parent job on an offline slave leading to an NPE. This seems related to the issue huybrechts reported. I developed a patch to prevent this which I am attaching to this issue. java.lang.NullPointerException at hudson.model.Slave.createLauncher(Slave.java:309) at hudson.model.AbstractBuild$AbstractRunner.createLauncher(AbstractBuild.java:417) at hudson.model.AbstractBuild$AbstractRunner.run(AbstractBuild.java:369) at hudson.model.Run.run(Run.java:1198) at hudson.matrix.MatrixBuild.run(MatrixBuild.java:149) at hudson.model.ResourceController.execute(ResourceController.java:88) at hudson.model.Executor.run(Executor.java:123) at hudson.model.OneOffExecutor.run(OneOffExecutor.java:60)
          Hide
          pcc pcc added a comment -

          Created an attachment (id=1036)
          [PATCH] Do not attempt to start flyweight tasks on offline nodes

          Show
          pcc pcc added a comment - Created an attachment (id=1036) [PATCH] Do not attempt to start flyweight tasks on offline nodes
          Hide
          scm_issue_link SCM/JIRA link daemon added a comment -

          Code changed in hudson
          User: : kohsuke
          Path:
          trunk/hudson/main/core/src/main/java/hudson/model/Queue.java
          http://fisheye4.cenqua.com/changelog/hudson/?cs=24149
          Log:
          JENKINS-936 Applied the patch from pcc.

          Show
          scm_issue_link SCM/JIRA link daemon added a comment - Code changed in hudson User: : kohsuke Path: trunk/hudson/main/core/src/main/java/hudson/model/Queue.java http://fisheye4.cenqua.com/changelog/hudson/?cs=24149 Log: JENKINS-936 Applied the patch from pcc.
          Hide
          kohsuke Kohsuke Kawaguchi added a comment -

          Incorrectly marked as resolved due to a bug in scm_issue_link.

          Show
          kohsuke Kohsuke Kawaguchi added a comment - Incorrectly marked as resolved due to a bug in scm_issue_link.
          Hide
          scm_issue_link SCM/JIRA link daemon added a comment -

          Code changed in hudson
          User: : kohsuke
          Path:
          trunk/hudson/main/core/src/main/java/hudson/model/Computer.java
          trunk/hudson/main/core/src/main/java/hudson/model/Executor.java
          trunk/hudson/main/core/src/main/java/hudson/model/Hudson.java
          trunk/www/changelog.html
          http://fisheye4.cenqua.com/changelog/hudson/?cs=24159
          Log:
          [FIXED JENKINS-936] in 1.337 by moving out the start method from Executor.
          Since not many code will own Executors, for now I didn't introduce any wrapper to hide the start method invocation.

          Show
          scm_issue_link SCM/JIRA link daemon added a comment - Code changed in hudson User: : kohsuke Path: trunk/hudson/main/core/src/main/java/hudson/model/Computer.java trunk/hudson/main/core/src/main/java/hudson/model/Executor.java trunk/hudson/main/core/src/main/java/hudson/model/Hudson.java trunk/www/changelog.html http://fisheye4.cenqua.com/changelog/hudson/?cs=24159 Log: [FIXED JENKINS-936] in 1.337 by moving out the start method from Executor. Since not many code will own Executors, for now I didn't introduce any wrapper to hide the start method invocation.
          Hide
          akostadinov akostadinov added a comment -

          hallo,

          I tested the feature with 1.335

          It seem to be working fine except for two issues:
          1. When all slaves are marked offline including master, then the matrix parent job is put in the queue waiting. This is totally correct. But once a slave is put online, the job does one of the following:
          1.1. (if slave is "utilize as much as possible") executes by an existing executor
          1.2. (if slave is "for tied jobs only") waits forever in the queue
          2. Currently parent job executes on a random slave. IMHO it should try to run first on a slave that is "utilize as much as possible" or one should be able to specify where the job should execute. Otherwise one cannot guarantee job is not executed on a machine running a performance test and render the results invalid.

          Thanks!

          Show
          akostadinov akostadinov added a comment - hallo, I tested the feature with 1.335 It seem to be working fine except for two issues: 1. When all slaves are marked offline including master, then the matrix parent job is put in the queue waiting. This is totally correct. But once a slave is put online, the job does one of the following: 1.1. (if slave is "utilize as much as possible") executes by an existing executor 1.2. (if slave is "for tied jobs only") waits forever in the queue 2. Currently parent job executes on a random slave. IMHO it should try to run first on a slave that is "utilize as much as possible" or one should be able to specify where the job should execute. Otherwise one cannot guarantee job is not executed on a machine running a performance test and render the results invalid. Thanks!
          Hide
          akostadinov akostadinov added a comment -

          Excuse me for reopening. I looked at "fixed" field and didn't se a version there so thought just installed 1.335 here is recent enough. Now I notice last comment is talking about 1.337. So marking resolved again.

          It would be nice to use the "Fixed" field in the future as that is what probably most users are used to.

          Regards

          Show
          akostadinov akostadinov added a comment - Excuse me for reopening. I looked at "fixed" field and didn't se a version there so thought just installed 1.335 here is recent enough. Now I notice last comment is talking about 1.337. So marking resolved again. It would be nice to use the "Fixed" field in the future as that is what probably most users are used to. Regards
          Hide
          mdonohue mdonohue added a comment -

          JENKINS-5076 is a request to avoid 'tied' slaves for the master checkout. Is that fixed in 1.337?

          Show
          mdonohue mdonohue added a comment - JENKINS-5076 is a request to avoid 'tied' slaves for the master checkout. Is that fixed in 1.337?
          Hide
          timotm TimoTM added a comment - - edited

          We have environment where some slaves (executing tests) do not have svn client installed. After upgrading to 1.337 (I don't know if it was just coincidence), multiconfiguration parent jobs started to get executed on such slaves. At least jobs that have been configured to do SCM polling fail in that point.

          Is there no way to tell Hudson not to build the parent on such a slave? ("leave this machine for tied jobs only" has been configured)

          Show
          timotm TimoTM added a comment - - edited We have environment where some slaves (executing tests) do not have svn client installed. After upgrading to 1.337 (I don't know if it was just coincidence), multiconfiguration parent jobs started to get executed on such slaves. At least jobs that have been configured to do SCM polling fail in that point. Is there no way to tell Hudson not to build the parent on such a slave? ("leave this machine for tied jobs only" has been configured)
          Hide
          axelheider Axel Heider added a comment -

          Problem appears again with Hudson V1.369 wit a windows master and a linux slave node. I have two executors on the slave node and two matrix jobs tied to it (via tie-matrix-partent plugin and all matrix job). When they were started via SVN checkin the matrix job 1 parent took executor 1 and matrix job 2 parent took executor 2. Then both started their matrix-config jobs, but none can run as executors are blocked. I had to kill one parent job to resolve this. However, it keeps happening again.

          Show
          axelheider Axel Heider added a comment - Problem appears again with Hudson V1.369 wit a windows master and a linux slave node. I have two executors on the slave node and two matrix jobs tied to it (via tie-matrix-partent plugin and all matrix job). When they were started via SVN checkin the matrix job 1 parent took executor 1 and matrix job 2 parent took executor 2. Then both started their matrix-config jobs, but none can run as executors are blocked. I had to kill one parent job to resolve this. However, it keeps happening again.
          Hide
          khushsk Sagar Khushalani added a comment -

          I have a project running in multi-configuration mode and it seems to work fine, except that the parent job seems to run on a random slave, which becomes problematic. Is it possible add an option to tie the parent to a particular slave?

          Show
          khushsk Sagar Khushalani added a comment - I have a project running in multi-configuration mode and it seems to work fine, except that the parent job seems to run on a random slave, which becomes problematic. Is it possible add an option to tie the parent to a particular slave?
          Hide
          axelheider Axel Heider added a comment -

          there is a tie-matrix-parent pluging thatr became available some time ago, see http://wiki.jenkins-ci.org/display/JENKINS/Matrix+Tie+Parent+Plugin It will do what you want.

          Show
          axelheider Axel Heider added a comment - there is a tie-matrix-parent pluging thatr became available some time ago, see http://wiki.jenkins-ci.org/display/JENKINS/Matrix+Tie+Parent+Plugin It will do what you want.
          Hide
          alex_barna Tzuchien added a comment -

          What is the current status of this long-lived issue, please? According to the release note, this issued has been resolved in 1.337. I didn't have this issue with 1.368, but recently after I've upgraded to 1.382, it has just appeared again.

          Show
          alex_barna Tzuchien added a comment - What is the current status of this long-lived issue, please? According to the release note, this issued has been resolved in 1.337. I didn't have this issue with 1.368, but recently after I've upgraded to 1.382, it has just appeared again.
          Hide
          kohsuke Kohsuke Kawaguchi added a comment -

          The original issue, which is that the matrix parent build consumes an executor, is fixed indeed in 1.337. In the executor table, the parent build will show up without an executor number — think of it as getting executed in a temporary executor.

          People then started talking about other related but different issues, such as the fact that the matrix parent execution cannot be tied to a specific node.

          Since this issue is getting overloaded with multiple things, I'm closing this bug once and for all.

          If you think you hit a bug, please open a new one.

          Thanks!

          Show
          kohsuke Kohsuke Kawaguchi added a comment - The original issue, which is that the matrix parent build consumes an executor, is fixed indeed in 1.337. In the executor table, the parent build will show up without an executor number — think of it as getting executed in a temporary executor. People then started talking about other related but different issues, such as the fact that the matrix parent execution cannot be tied to a specific node. Since this issue is getting overloaded with multiple things, I'm closing this bug once and for all. If you think you hit a bug, please open a new one . Thanks!
          Hide
          rdesgroppes Régis Desgroppes added a comment -

          I agree with Kohsuke: closed.

          Show
          rdesgroppes Régis Desgroppes added a comment - I agree with Kohsuke: closed.
          Hide
          imakowski Ireneusz Makowski added a comment -

          I still have this issue on latest LTS. It blocks our executors when Restrict label for matrix job is used!

          Show
          imakowski Ireneusz Makowski added a comment - I still have this issue on latest LTS. It blocks our executors when Restrict label for matrix job is used!
          Hide
          imakowski Ireneusz Makowski added a comment -

          See my latest comment

          Show
          imakowski Ireneusz Makowski added a comment - See my latest comment
          Hide
          danielbeck Daniel Beck added a comment -

          It blocks our executors when Restrict label for matrix job is used!

          Please file a new issue, and include specific information on how to reproduce this.

          https://wiki.jenkins-ci.org/display/JENKINS/How+to+report+an+issue

          Show
          danielbeck Daniel Beck added a comment - It blocks our executors when Restrict label for matrix job is used! Please file a new issue, and include specific information on how to reproduce this. https://wiki.jenkins-ci.org/display/JENKINS/How+to+report+an+issue

            People

            • Assignee:
              rdesgroppes Régis Desgroppes
              Reporter:
              rdesgroppes Régis Desgroppes
            • Votes:
              24 Vote for this issue
              Watchers:
              11 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: