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

      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.

        Attachments

          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: