You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@brooklyn.apache.org by aledsage <gi...@git.apache.org> on 2014/11/05 14:44:03 UTC

[GitHub] incubator-brooklyn pull request: Fix deadlock starting 3tier app

GitHub user aledsage opened a pull request:

    https://github.com/apache/incubator-brooklyn/pull/301

    Fix deadlock starting 3tier app

    Builds http://brooklyn.builds.cloudsoftcorp.com/job/Brooklyn-Master-Integration/373 and http://brooklyn.builds.cloudsoftcorp.com/job/Brooklyn-Master-Integration/375 were both hit by this.
    
    Taking description from first commit message:
    
    - Calling getShellEnvironment() during jboss.install() was causing it
      to lookup the config, including sysprops. However, that required
      MySql to have a URL before it could resolve.
      On localhost (e.g. integration tests), the jboss.install() held a
      lock (SshMachineLocation.acquireMutex) which meant only one thing
      could be installed at a time - hence prevented MySql from being 
      installed. Therefore deadlock.
    - Fixed by guarding the call to getShellEnvironment(), to only be
      done if env was not explicitly supplied.

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/aledsage/incubator-brooklyn fix/deadlock-starting-3tier-app

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/incubator-brooklyn/pull/301.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #301
    
----
commit b8ec67f7a5e7547f7d2d59f85b3f6699b0352e6e
Author: Aled Sage <al...@gmail.com>
Date:   2014-11-05T13:18:53Z

    SoftwareProcess: don’t getShellEnv() if supplied
    
    - Calling getShellEnvironment() during jboss.install() was causing it
      to lookup the config, including sysprops. However, that required
      MySql to have a URL before it could resolve.
      On localhost (e.g. integration tests), the jboss.install() held a
      lock (SshMachineLocation.acquireMutex) which meant only one thing
      could be installed at a time - hence prevented MySql from being 
      installed. Therefore deadlock.
    - Fixed by guarding the call to getShellEnvironment(), to only be
      done if env was not explicitly supplied.

commit 66cbad2b18f793edf19c34afa41d2028d86785ca
Author: Aled Sage <al...@gmail.com>
Date:   2014-11-05T13:20:21Z

    Tomcat install: don’t resolve config (via env variables)

commit 344067970e305843b1703bc34c38f18659de038f
Author: Aled Sage <al...@gmail.com>
Date:   2014-11-05T13:35:08Z

    fix BrooklynNodeIntegrationTest.testStopPlainThrowsException
    
    - Test was leaving brooklyn process running

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: Fix deadlock starting 3tier app

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/incubator-brooklyn/pull/301


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: Fix deadlock starting 3tier app

Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on the pull request:

    https://github.com/apache/incubator-brooklyn/pull/301#issuecomment-61815391
  
    @ahgittin you'll find this one interesting :-)


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: Fix deadlock starting 3tier app

Posted by richardcloudsoft <gi...@git.apache.org>.
Github user richardcloudsoft commented on the pull request:

    https://github.com/apache/incubator-brooklyn/pull/301#issuecomment-61812320
  
    Fix seems reasonable but from the sound of your description of the problem, it's more like a workaround than a solution. I'm assuming it's because the localhost location is essentially a singleton, and all entities end up using a single SshMachineLocation? Is this a potential problem with many other entities or is it unique to JBoss? Will the same problem happen with SameServerEntity instance?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: Fix deadlock starting 3tier app

Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on the pull request:

    https://github.com/apache/incubator-brooklyn/pull/301#issuecomment-61815161
  
    For localhost, there is a static mutex (hence the problems here). If one is deploying to different machines, it's fine.
    
    For `SameServerEntity`, then yes - it's a risk. The problem happens if the `getEnvironmentVariables()` method of one entity contains config that is an `attributeWhenReady` pointing at the other entity. This includes If the `install` command retrieves the Java system properties. 
    
    The solution when implementing an entity is to use `environmentVariablesReset()` when building the install command, to tell it not to retrieve the `driver.getEnvironmentVariables()` values.
    
    Certainly not ideal as it's very subtle and thus unexpected! Better would be to be more explicit about the environment variables to use for each command.
    
    Merging this PR now.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---