You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@accumulo.apache.org by haydenmarchant <gi...@git.apache.org> on 2014/06/24 15:21:16 UTC

[GitHub] accumulo pull request: Fixes harded coded Sun JVM in config and en...

GitHub user haydenmarchant opened a pull request:

    https://github.com/apache/accumulo/pull/9

    Fixes harded coded Sun JVM in config and env

    Addressing the issue raised in JIRA 2944, this is ensuring that
    Accumulo will be correctly configured for any Java vendor.
    
    A new question is being asked in the bootstrap_config.sh for Java vendor. This is then used to set GC settings, exclude/include JAXP implementation in ACCUMULO_OPTS. Also, default NRG provider is being injected as system property on command line since IBM JVM does not have SUN registered as default provider.

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

    $ git pull https://github.com/haydenmarchant/accumulo ACCUMULO-2944

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

    https://github.com/apache/accumulo/pull/9.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 #9
    
----
commit 6fe2275f8e24df709db1d8db6d7a6cca9a3892dc
Author: haydenmarchant <ha...@gmail.com>
Date:   2014-06-24T11:29:45Z

    Fixes harded coded Sun JVM in config and env
    
    Addressing the issue raised in JIRA 2944, this is ensuring that
    Accumulo will be correctly configured for any Java vendor.
    
    A new question is being asked in the bootstrap_config.sh for Java vendor. This is then used to set GC settings, exclude/include JAXP implementation in ACCUMULO_OPTS. Also, default NRG provider is being injected as system property on command line since IBM JVM does not have SUN registered as default provider.

----


---
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] accumulo pull request: ACCUMULO-2944 Fixes harded coded Sun JVM in...

Posted by busbey <gi...@git.apache.org>.
Github user busbey commented on a diff in the pull request:

    https://github.com/apache/accumulo/pull/9#discussion_r14696949
  
    --- Diff: minicluster/src/main/java/org/apache/accumulo/minicluster/impl/MiniAccumuloClusterImpl.java ---
    @@ -259,7 +259,14 @@ private Process _exec(Class<?> clazz, List<String> extraJvmOpts, String... args)
         for (Entry<String,String> sysProp : config.getSystemProperties().entrySet()) {
           argList.add(String.format("-D%s=%s", sysProp.getKey(), sysProp.getValue()));
         }
    -    argList.addAll(Arrays.asList("-XX:+UseConcMarkSweepGC", "-XX:CMSInitiatingOccupancyFraction=75", "-Dapple.awt.UIElement=true", Main.class.getName(), className));
    +    
    +    String gcPolicyArgs = System.getenv("GC_POLICY_ARGS");
    --- End diff --
    
    so long as there's a ticket filed, I'd be fine with the maven profile going into a follow on.


---
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] accumulo pull request: ACCUMULO-2944 Fixes harded coded Sun JVM in...

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

    https://github.com/apache/accumulo/pull/9#issuecomment-47626599
  
    commits have been squashed and pushed to my branch.


---
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] accumulo pull request: Fixes harded coded Sun JVM in config and en...

Posted by haydenmarchant <gi...@git.apache.org>.
Github user haydenmarchant commented on a diff in the pull request:

    https://github.com/apache/accumulo/pull/9#discussion_r14173113
  
    --- Diff: bin/bootstrap_config.sh ---
    @@ -119,6 +123,86 @@ while [[ "${OVERWRITE}" = "0" ]]; do
     done
     echo "Coppying configuration files to: ${CONF_DIR}"
     
    +if [[ -z "${SIZE}" ]]; then
    --- End diff --
    
    I thought that it would be best to keep all the user input in one chunk.
    
    Do you think that I should revert the relocation of the other selects and just put the JVM down below instead? If so, that's not a problem.


---
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] accumulo pull request: ACCUMULO-2944 Fixes harded coded Sun JVM in...

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

    https://github.com/apache/accumulo/pull/9


---
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] accumulo pull request: ACCUMULO-2944 Fixes harded coded Sun JVM in...

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

    https://github.com/apache/accumulo/pull/9#issuecomment-47677181
  
    Please update the commit message to use the format requested [in the contributor guide][1] (specifically see step 5)
    
    [1]: http://accumulo.apache.org/git.html#contributors


---
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] accumulo pull request: ACCUMULO-2944 Fixes harded coded Sun JVM in...

Posted by haydenmarchant <gi...@git.apache.org>.
GitHub user haydenmarchant reopened a pull request:

    https://github.com/apache/accumulo/pull/9

    ACCUMULO-2944 Fixes harded coded Sun JVM in config and env

    ACCUMULO-2944 add support for multiple java vendors in conf & scripts
    
    Support for multiple java vendors is added by getting the
    bootstrap_config.sh to generate correct configuration depending on 
    specified java vendor. Following details depend on Java vendor
    
    * GC settings
    * Exclude/Include JAXP implementation
    * Default NRG provider
    
     
    A new question is being asked in the bootstrap_config.sh for Java 
    vendor. This is then used to set GC settings, exclude/include JAXP 
    implementation in ACCUMULO_OPTS. Also, default NRG provider is being 
    injected as system property on command line since IBM JVM does not 
    have SUN registered as default provider.

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

    $ git pull https://github.com/haydenmarchant/accumulo ACCUMULO-2944

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

    https://github.com/apache/accumulo/pull/9.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 #9
    
----
commit a20e19fc4f7c7989ba1b50459d9f762063e3e631
Author: Christopher Tubbs <ct...@apache.org>
Date:   2014-03-28T00:35:32Z

    ACCUMULO-1487, ACCUMULO-1491 Stop packaging docs for monitor
    
    Moved docs out of monitor and into docs directory. Added docs to assemblies.
    Remove unnecessary goals from release profile. Remove links from docs to
    apidocs. Restricted rpms/debs from being placed in lib/ and docs/ in tarball.

commit e5070d0ad084f218d283b9daa58246184afdb477
Author: Eric C. Newton <er...@gmail.com>
Date:   2014-03-28T16:58:19Z

    ACCUMULO-2455 1s timeout is too tight for testing

commit d2089c898c64ce2ac6588b9052505dc4c49d971f
Author: Bill Havanki <bh...@cloudera.com>
Date:   2014-03-13T20:34:55Z

    ACCUMULO-2470 Unit tests for server/base module
    
    This commit adds unit tests to code under the server/base module. It also includes changes
    to o.a.a.server.problems.ProblemReport to enable comprehensive testing.

commit 3354b92af1820ca12c9dbe5dc4d0db7d8d0a640a
Author: Mike Drob <md...@cloudera.com>
Date:   2014-03-28T21:24:33Z

    Merge branch '1.5.2-SNAPSHOT' into 1.6.0-SNAPSHOT

commit 9843911d939bafc2808f3edb30903881cb7fa04d
Author: Mike Drob <md...@cloudera.com>
Date:   2014-03-29T04:49:59Z

    Merge branch '1.5.2-SNAPSHOT' into 1.6.0-SNAPSHOT

commit f4a9626a5a5e2a66c201218b13aeef558b63506c
Author: Christopher Tubbs <ct...@apache.org>
Date:   2014-03-31T17:40:35Z

    ACCUMULO-1996 Add some javadocs to SimpleMacIT to define its scope

commit dda2fb06ba6452d5af6bda3809684ec2f3413022
Author: Christopher Tubbs <ct...@apache.org>
Date:   2014-03-31T18:11:36Z

    ACCUMULO-2392 Log the MAC directory used by a test
    
    Add a log message to SimpleMacIT to inform which MiniAccumuloCluster instance is
    in use for each SimpleMacIT test, to make it easier to debug a failed test.
    
    Additionally, renamed a trivial helper method to more accurately reflect its
    purpose.

commit fff852fce6d971e7e76c91ae911fbfca2e26426b
Author: Christopher Tubbs <ct...@apache.org>
Date:   2014-03-31T18:40:26Z

    ACCUMULO-2470 Clean up warnings introduced by previous commit
    
    (Unused fields and imports)

commit 67c15ffa24d2515e5e39562d2128e96094598492
Author: Christopher Tubbs <ct...@apache.org>
Date:   2014-03-31T19:36:49Z

    ACCUMULO-2595 Remove jar for init module

commit 4a77566a314aaa6c2086d8d07d228a073a437777
Author: Christopher Tubbs <ct...@apache.org>
Date:   2014-03-31T19:46:08Z

    ACCUMULO-2595 Remove dep. mgmt entry for accumulo-init jar

commit 4956b15a6f86bca303074b907b2e62f783d395bf
Author: Eric C. Newton <er...@gmail.com>
Date:   2014-04-01T15:45:28Z

    ACCUMULO-2601 remove initialization from rpm

commit 7a68838ac5650dbc1bfe64818506875c4dacc099
Author: Eric C. Newton <er...@gmail.com>
Date:   2014-04-01T17:50:07Z

    ACCUMULO-2601 test for the existence of /accumulo in hdfs

commit 7ac4b54a76b85314a483378e01980c73f891a900
Author: Eric C. Newton <er...@gmail.com>
Date:   2014-04-01T18:46:17Z

    ACCUMULO-2605 list slf4j as a test dependency

commit 3a1b38719c69d34abb27e181fce3bfdb7758bf92
Author: Josh Elser <el...@apache.org>
Date:   2014-03-26T22:04:05Z

    ACCUMULO-2592 Create AccumuloCluster and AccumuloConfig interfaces to allow for proper non-minicluster implementations.
    
    Includes package-level javadocs in lieu of some specific annotation specifying api "experimental" status.

commit 86cafd972793330026c6e4739a2a94320608a333
Author: Josh Elser <el...@apache.org>
Date:   2014-04-01T21:08:00Z

    Merge branch '1.5.2-SNAPSHOT' into 1.6.0-SNAPSHOT
    
    Conflicts:
    	core/src/main/java/org/apache/accumulo/core/client/mapred/InputFormatBase.java
    	core/src/main/java/org/apache/accumulo/core/client/mapreduce/InputFormatBase.java

commit a1814fc0dbdf0fb095ad87385dfb133c5a1f35df
Author: Christopher Tubbs <ct...@apache.org>
Date:   2014-04-01T21:48:12Z

    ACCUMULO-2606 Remove system-level packaging stuff
    
    Removing RPM/DEB build tasks and maven profiles, related documentation,
    and system-specific packaging resources and scripts, in order to defer such
    things to downstream package maintainers.

commit d896bf37d2dcf1834e32425b7364e6d5963e4c12
Author: Sean Busbey <bu...@cloudera.com>
Date:   2014-03-28T21:47:00Z

    ACCUMULO-2590 Updates our Public API declaration to be more explicit.

commit 09388f5671a3979c3c83c5de8f63eb55a3e83fcb
Author: Eric C. Newton <er...@gmail.com>
Date:   2014-04-02T15:02:37Z

    ACCUMULO-2584 use IteratorSettings to push the scan iterator, not a zookeeper setting

commit e2552a89f4436292817e1bf94aa0cfa4f7e88339
Author: Josh Elser <el...@apache.org>
Date:   2014-04-02T18:42:39Z

    ACCUMULO-2592 Fix incorrect javadoc

commit aaa8afb9fe28d6af3437428dc30490fc079f2836
Author: Christopher Tubbs <ct...@apache.org>
Date:   2014-04-02T20:24:11Z

    ACCUMULO-2616 Bump maven plugin/parent POM versions
    
    Change Apache parent POM to version 14. Drop resulting redundant and/or
    unnecessary plugin versions, configuration, and properties.
    
    Update bouncycastle test dependency version and incorporate into the
    dependencyManagement section and fix resulting deprecation warnings.
    
    Drop redundant m2e container in assembly POM. Migrate to non-deprecated
    surefire/failsafe fork settings. Update other plugin versions to latest.

commit 2c1454e73f50a1e5fa2183deb193f58d6a8337a8
Author: Christopher Tubbs <ct...@apache.org>
Date:   2014-04-03T15:11:50Z

    ACCUMULO-2620 Make table names more unique and cleanup unused field.
    
    Support parallel tests better with more better uniqueness in table names.

commit b3f6e0f8f6dda80f98c5b031baaec7061cecf0c5
Author: Mike Drob <md...@cloudera.com>
Date:   2014-04-04T01:57:01Z

    Merge branch '1.5.2-SNAPSHOT' into 1.6.0-SNAPSHOT

commit 6960570252751a6a2fcda6da19755462405f6116
Author: Josh Elser <el...@apache.org>
Date:   2014-04-04T16:32:24Z

    Merge branch '1.5.2-SNAPSHOT' into 1.6.0-SNAPSHOT

commit d137a4f9e49ca05c4967ac3ed09ba3e3206b2ec6
Author: jpmcnamee <jp...@terpmail.umd.edu>
Date:   2014-03-06T20:34:14Z

    ACCUMULO-1395 Generate example configuration
    
    Signed-off-by: Christopher Tubbs <ct...@apache.org>

commit 527913a317eaff502cbf16a8cc8aba810bb20b0a
Author: John Vines <vi...@apache.org>
Date:   2014-04-04T18:58:21Z

    Revert "ACCUMULO-1395 Generate example configuration"
    
    This reverts commit d137a4f9e49ca05c4967ac3ed09ba3e3206b2ec6.

commit 902446659c2d030d787f2c92138afa305d05248d
Author: Josh Elser <el...@apache.org>
Date:   2014-04-04T20:33:11Z

    Merge branch '1.5.2-SNAPSHOT' into 1.6.0-SNAPSHOT
    
    Conflicts:
    	test/src/test/java/org/apache/accumulo/test/ShellServerTest.java

commit c60e3e593809b67b194a4e377e03bdcdb5c8e844
Author: Mike Drob <md...@cloudera.com>
Date:   2014-04-04T23:16:20Z

    Merge branch '1.5.2-SNAPSHOT' into 1.6.0-SNAPSHOT (-sours)

commit 2cb526e5e11a54e6e1932cf37fa715b2cae22533
Author: Christopher Tubbs <ct...@apache.org>
Date:   2014-04-04T23:55:25Z

    ACCUMULO-2455 Remove JVM reuse, causing some test failures
    
      Forked JVMs are reused, but a JVM can be left in a bad state, causing
      a cascade of other test failures. This reuse can speed up the tests,
      but I'm disabling it, until we can figure out which tests leave the
      JVM in a bad (non-reusable) state and whether we can actually avoid
      that or not, such that it'd be safe to turn JVM reuse back on.

commit 957c9d1b34afb8a5b7347921ba2d5b0b22bbc828
Author: Sean Busbey <bu...@cloudera.com>
Date:   2014-04-05T00:27:39Z

    Merge branch '1.5.2-SNAPSHOT' into 1.6.0-SNAPSHOT
    
      Conflicts:
          fate/src/main/java/org/apache/accumulo/fate/AdminUtil.java
          fate/src/main/java/org/apache/accumulo/fate/TStore.java
          server/base/src/main/java/org/apache/accumulo/server/Accumulo.java
          server/src/main/java/org/apache/accumulo/server/master/Master.java
          server/src/main/java/org/apache/accumulo/server/util/MetadataTable.java
          server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServer.java

commit e4aa11e1b1a046dec9116273eb57f053aa68fd3f
Author: Sean Busbey <bu...@cloudera.com>
Date:   2014-04-04T08:35:01Z

    ACCUMULO-2519 Updates Classes added in 1.6.0 for read only fate changes.

----


---
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] accumulo pull request: Fixes harded coded Sun JVM in config and en...

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

    https://github.com/apache/accumulo/pull/9#issuecomment-46995683
  
    > BTW are there any system tests for the bootstrap shell script?
    
    Not that I'm aware of. It would be nice to have some, but it hasn't been a big priority. More patches welcome :)


---
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] accumulo pull request: Fixes harded coded Sun JVM in config and en...

Posted by busbey <gi...@git.apache.org>.
Github user busbey commented on a diff in the pull request:

    https://github.com/apache/accumulo/pull/9#discussion_r14174450
  
    --- Diff: bin/bootstrap_config.sh ---
    @@ -119,6 +123,86 @@ while [[ "${OVERWRITE}" = "0" ]]; do
     done
     echo "Coppying configuration files to: ${CONF_DIR}"
     
    +if [[ -z "${SIZE}" ]]; then
    --- End diff --
    
    That sounds like a good reason to move the user prompts up to before all the initialization.


---
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] accumulo pull request: Fixes harded coded Sun JVM in config and en...

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

    https://github.com/apache/accumulo/pull/9#issuecomment-46993835
  
    Please update the commit message to use the format requested [in the contributor guide][1] (specifically see step 5)
    
    [1]: http://accumulo.apache.org/git.html#contributors


---
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] accumulo pull request: ACCUMULO-2944 Fixes harded coded Sun JVM in...

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

    https://github.com/apache/accumulo/pull/9#issuecomment-47536342
  
    Can you make sure you've pushed the updated version to your ACCUMULO-2944 branch? That will update this PR. Right now it looks like the previous 2 commits are still there.


---
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] accumulo pull request: ACCUMULO-2944 Fixes harded coded Sun JVM in...

Posted by busbey <gi...@git.apache.org>.
Github user busbey commented on a diff in the pull request:

    https://github.com/apache/accumulo/pull/9#discussion_r14422863
  
    --- Diff: conf/templates/accumulo-env.sh ---
    @@ -51,10 +51,11 @@ test -z "$ACCUMULO_TSERVER_OPTS" && export ACCUMULO_TSERVER_OPTS="${POLICY} ${tS
     test -z "$ACCUMULO_MASTER_OPTS"  && export ACCUMULO_MASTER_OPTS="${POLICY} ${masterHigh_masterLow}"
     test -z "$ACCUMULO_MONITOR_OPTS" && export ACCUMULO_MONITOR_OPTS="${POLICY} ${monitorHigh_monitorLow}"
     test -z "$ACCUMULO_GC_OPTS"      && export ACCUMULO_GC_OPTS="${gcHigh_gcLow}"
    -test -z "$ACCUMULO_GENERAL_OPTS" && export ACCUMULO_GENERAL_OPTS="-XX:+UseConcMarkSweepGC -XX:CMSInitiatingOccupancyFraction=75 -Djava.net.preferIPv4Stack=true"
    +test -z "$ACCUMULO_GENERAL_OPTS" && export ACCUMULO_GENERAL_OPTS="${gcPolicy} -Djava.net.preferIPv4Stack=true ${jaxpDocBuilderFactory} ${randomNumGeneratorProviderOverride}"
     test -z "$ACCUMULO_OTHER_OPTS"   && export ACCUMULO_OTHER_OPTS="${otherHigh_otherLow}"
     # what do when the JVM runs out of heap memory
     export ACCUMULO_KILL_CMD='kill -9 %p'
    +export GC_POLICY_ARGS="${gcPolicy}"
    --- End diff --
    
    Yes, that sounds like a god idea.
    
    please either include the MiniAccumuloCluster change in this same patch, or move this environment variable to the patch taht includes said changes.


---
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] accumulo pull request: Fixes harded coded Sun JVM in config and en...

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

    https://github.com/apache/accumulo/pull/9#issuecomment-47075772
  
    > btw, in the bootstrap_config.sh, with regard to options, since the v was taken for version, and j was taken to denote java mm, I defaulted to using x to denote vendor - is that ok?
    
    I don't have a strong opinion on using -x | --vendor for the jvm type. I think it's no more confusing than the other argument names we're currently using. I'd love to change them around and have -j | --jvm be used for this, but that can be a follow on.


---
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] accumulo pull request: ACCUMULO-2944 Fixes harded coded Sun JVM in...

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

    https://github.com/apache/accumulo/pull/9#issuecomment-48513357
  
    something has gone wrong in the rebase, since git now thinks all ~1750 commits are a part of the merge.
    
    would you mind if I just picked off your commit and did the cleanup to make it work on the current dev branch?


---
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] accumulo pull request: ACCUMULO-2944 Fixes harded coded Sun JVM in...

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

    https://github.com/apache/accumulo/pull/9#issuecomment-48436920
  
    Overall patch is looking good. Could you rebase onto the current 1.6.1-SNAPSHOT and resolved the conflict?
    
    The commit message still doesn't start with the jira reference. Could you move it to the beginning of the summary line?


---
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] accumulo pull request: Fixes harded coded Sun JVM in config and en...

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

    https://github.com/apache/accumulo/pull/9#issuecomment-46995128
  
    Indeed. That is definitely a silly bug. I'll fix that.
    
    BTW are there any system tests for the bootstrap shell script?


---
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] accumulo pull request: ACCUMULO-2944 Fixes harded coded Sun JVM in...

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

    https://github.com/apache/accumulo/pull/9#issuecomment-48433782
  
    MiniAccumuloClusterImpl is now in patch.


---
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] accumulo pull request: Fixes harded coded Sun JVM in config and en...

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

    https://github.com/apache/accumulo/pull/9#issuecomment-46996740
  
    > 
    > > BTW are there any system tests for the bootstrap shell script?
    > 
    > Not that I'm aware of. It would be nice to have some, but it hasn't been a big priority. More patches welcome :)
    
    
    Not in this patchset though, please. I think we need to refactor where the shell scripts live to start doing testing. something like src/main/bin or src/main/bash (hadoop does the former). then we could use shunit or the like.


---
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] accumulo pull request: ACCUMULO-2944 Fixes harded coded Sun JVM in...

Posted by haydenmarchant <gi...@git.apache.org>.
Github user haydenmarchant commented on a diff in the pull request:

    https://github.com/apache/accumulo/pull/9#discussion_r14696336
  
    --- Diff: minicluster/src/main/java/org/apache/accumulo/minicluster/impl/MiniAccumuloClusterImpl.java ---
    @@ -259,7 +259,14 @@ private Process _exec(Class<?> clazz, List<String> extraJvmOpts, String... args)
         for (Entry<String,String> sysProp : config.getSystemProperties().entrySet()) {
           argList.add(String.format("-D%s=%s", sysProp.getKey(), sysProp.getValue()));
         }
    -    argList.addAll(Arrays.asList("-XX:+UseConcMarkSweepGC", "-XX:CMSInitiatingOccupancyFraction=75", "-Dapple.awt.UIElement=true", Main.class.getName(), className));
    +    
    +    String gcPolicyArgs = System.getenv("GC_POLICY_ARGS");
    --- End diff --
    
    Since I am not so familiar with maven, my assumption was that a user would manually change the pom to 'inject' the JVM-vendor. If we can automate this by profiles, that makes a lot of sense. I take it that it doesn't change this patch, but rather would be on top of this?


---
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] accumulo pull request: Fixes harded coded Sun JVM in config and en...

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

    https://github.com/apache/accumulo/pull/9#issuecomment-46987882
  
    @madrob, the 2 its were intentional. Since it is possible to preset the vendor from the x parameter we will not enter the first if where the user selects the vendor.
    
    The second if is to set the env variables specifically for each JVM vendor.


---
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] accumulo pull request: Fixes harded coded Sun JVM in config and en...

Posted by busbey <gi...@git.apache.org>.
Github user busbey commented on a diff in the pull request:

    https://github.com/apache/accumulo/pull/9#discussion_r14173383
  
    --- Diff: bin/bootstrap_config.sh ---
    @@ -119,6 +123,86 @@ while [[ "${OVERWRITE}" = "0" ]]; do
     done
     echo "Coppying configuration files to: ${CONF_DIR}"
     
    +if [[ -z "${SIZE}" ]]; then
    --- End diff --
    
    I'd like to minimize the changeset if possible. I can see the argument for keeping user input together.
    
    If you can delay the JVM determination and avoid moving the user prompts, that'd be great. If that further complicates the patch and you'd prefer to leave it where it is, I'm fine with that as well.


---
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] accumulo pull request: ACCUMULO-2944 Fixes harded coded Sun JVM in...

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

    https://github.com/apache/accumulo/pull/9#issuecomment-47695850
  
    The commit message should start with the jira and a short summary.
    
    e.g.
    
    > ACCUMULO-2944  add support for multiple java vendors in conf & scripts.
    > 
    > A new question is being asked in the bootstrap_config.sh for Java vendor. This is then used to set 
    > GC settings, exclude/include JAXP implementation in ACCUMULO_OPTS. Also, default NRG
    > provider is being injected as system property on command line since IBM JVM does not have SUN
    > registered as default provider.



---
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] accumulo pull request: ACCUMULO-2944 Fixes harded coded Sun JVM in...

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

    https://github.com/apache/accumulo/pull/9#issuecomment-48513244
  
    I rebased and changed the commit message. Please let me know that all of this is ok.


---
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] accumulo pull request: Fixes harded coded Sun JVM in config and en...

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

    https://github.com/apache/accumulo/pull/9#issuecomment-46989879
  
    Ugh, github decided that my earlier comment goes to the whole change, not just the line I wanted. If the selected vendor is IBM, then you set RNG_PROVIDER_OVERRIDE_ARGS="" otherwise you set it to RNG_PROVIDER_OVERRIDE_ARGS="-Dcrypto.secure.rng.provider=IBMJCE"
    
    That looks backwards to me.


---
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] accumulo pull request: ACCUMULO-2944 Fixes harded coded Sun JVM in...

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

    https://github.com/apache/accumulo/pull/9#issuecomment-47521313
  
    Requested changes have been made (squashed commit and bug in the if statement.


---
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] accumulo pull request: Fixes harded coded Sun JVM in config and en...

Posted by madrob <gi...@git.apache.org>.
Github user madrob commented on a diff in the pull request:

    https://github.com/apache/accumulo/pull/9#discussion_r14135462
  
    --- Diff: bin/bootstrap_config.sh ---
    @@ -119,6 +123,86 @@ while [[ "${OVERWRITE}" = "0" ]]; do
     done
     echo "Coppying configuration files to: ${CONF_DIR}"
     
    +if [[ -z "${SIZE}" ]]; then
    +  echo "Choose the heap configuration:"
    +  select DIRNAME in 1GB 2GB 3GB 512MB; do
    +    echo "Using '${DIRNAME}' configuration"
    +    SIZE=${DIRNAME}
    +    break
    +  done
    +elif [[ "${SIZE}" != "1GB" && "${SIZE}" != "2GB"  && "${SIZE}" != "3GB" && "${SIZE}" != "512MB" ]]; then
    +  echo "Invalid memory size"
    +  echo "Supported sizes: '1GB' '2GB' '3GB' '512MB'"
    +  exit 1
    +fi
    +
    +if [[ -z "${TYPE}" ]]; then
    +  echo
    +  echo "Choose the Accumulo memory-map type:"
    +  select TYPENAME in Java Native; do
    +    if [[ "${TYPENAME}" == "Native" ]]; then
    +      TYPE="native"
    +      echo "Don't forget to build the native libraries using the bin/build_native_library.sh script"
    +    elif [[ "${TYPENAME}" == "Java" ]]; then
    +      TYPE="jvm"
    +    fi
    +    echo "Using '${TYPE}' configuration"
    +    echo
    +    break
    +  done
    +fi
    +
    +if [[ -z "${HADOOP_VERSION}" ]]; then
    +  echo
    +  echo "Choose the Apache Hadoop version:"
    +  select HADOOP in 'HADOOP 1' 'HADOOP 2' ; do
    +    if [ "${HADOOP}" == "HADOOP 2" ]; then
    +      HADOOP_VERSION="2"
    +    elif [ "${HADOOP}" == "HADOOP 1" ]; then
    +      HADOOP_VERSION="1"
    +    fi
    +    echo "Using Apache Hadoop version '${HADOOP_VERSION}' configuration"
    +    echo
    +    break
    +  done
    +elif [[ "${HADOOP_VERSION}" != "1" && "${HADOOP_VERSION}" != "2" ]]; then
    +  echo "Invalid Apache Hadoop version"
    +  echo "Supported Apache Hadoop versions: '1' '2'"
    +  exit 1
    +fi
    +
    +for var in SIZE TYPE HADOOP_VERSION; do
    +  if [[ -z ${!var} ]]; then
    +    echo "Invalid $var configuration"
    +    exit 1
    +  fi
    +done
    +
    +if [[ -z "${JVM_VENDOR}" ]]; then
    +  echo 
    +  echo "Choose Java vendor:"
    +  select SEL_JVM_VENDOR in Sun OpenJDK IBM Other; do
    +    echo "Using configuration for ${SEL_JVM_VENDOR} JVM"
    +    JVM_VENDOR=${SEL_JVM_VENDOR}
    +    echo
    +    break
    +  done
    +fi
    +
    +if [[ "${JVM_VENDOR}" == "IBM" ]] ; then
    +  GC_NEWSIZE_PREFIX="-Xmns"
    +  GC_MAXNEWSIZE_PREFIX="-Xmnx"
    +  GC_POLICY_ARGS="-Xgcpolicy:gencon"
    +  JAXP_DOCUMENT_BUILDER_FACTORY_ARGS=""
    +  RNG_PROVIDER_OVERRIDE_ARGS=""
    +else
    +  GC_NEWSIZE_PREFIX="-XX:NewSize="
    +  GC_MAXNEWSIZE_PREFIX="-XX:MaxNewSize="
    +  GC_POLICY_ARGS="-XX:+UseConcMarkSweepGC -XX:CMSInitiatingOccupancyFraction=75" 
    +  JAXP_DOCUMENT_BUILDER_FACTORY_ARGS="-Djavax.xml.parsers.DocumentBuilderFactory=com.sun.org.apache.xerces.internal.jaxp.DocumentBuilderFactoryImpl"
    +  RNG_PROVIDER_OVERRIDE_ARGS="-Dcrypto.secure.rng.provider=IBMJCE"
    --- End diff --
    
    This line looks like it belongs in the other branch of the if.


---
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] accumulo pull request: Fixes harded coded Sun JVM in config and en...

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

    https://github.com/apache/accumulo/pull/9#issuecomment-47075463
  
    please squash to a single commit once you have all your feedback so the merge only has the final outcome.


---
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] accumulo pull request: ACCUMULO-2944 Fixes harded coded Sun JVM in...

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

    https://github.com/apache/accumulo/pull/9#issuecomment-47693949
  
    I tried to comply with the contributor guidelines for the Pull Request. Can you be specific to what I am doing wrong?


---
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] accumulo pull request: ACCUMULO-2944 Fixes harded coded Sun JVM in...

Posted by haydenmarchant <gi...@git.apache.org>.
Github user haydenmarchant commented on a diff in the pull request:

    https://github.com/apache/accumulo/pull/9#discussion_r14421467
  
    --- Diff: conf/templates/accumulo-env.sh ---
    @@ -51,10 +51,11 @@ test -z "$ACCUMULO_TSERVER_OPTS" && export ACCUMULO_TSERVER_OPTS="${POLICY} ${tS
     test -z "$ACCUMULO_MASTER_OPTS"  && export ACCUMULO_MASTER_OPTS="${POLICY} ${masterHigh_masterLow}"
     test -z "$ACCUMULO_MONITOR_OPTS" && export ACCUMULO_MONITOR_OPTS="${POLICY} ${monitorHigh_monitorLow}"
     test -z "$ACCUMULO_GC_OPTS"      && export ACCUMULO_GC_OPTS="${gcHigh_gcLow}"
    -test -z "$ACCUMULO_GENERAL_OPTS" && export ACCUMULO_GENERAL_OPTS="-XX:+UseConcMarkSweepGC -XX:CMSInitiatingOccupancyFraction=75 -Djava.net.preferIPv4Stack=true"
    +test -z "$ACCUMULO_GENERAL_OPTS" && export ACCUMULO_GENERAL_OPTS="${gcPolicy} -Djava.net.preferIPv4Stack=true ${jaxpDocBuilderFactory} ${randomNumGeneratorProviderOverride}"
     test -z "$ACCUMULO_OTHER_OPTS"   && export ACCUMULO_OTHER_OPTS="${otherHigh_otherLow}"
     # what do when the JVM runs out of heap memory
     export ACCUMULO_KILL_CMD='kill -9 %p'
    +export GC_POLICY_ARGS="${gcPolicy}"
    --- End diff --
    
    The intention is that this variable will be used by the MiniAccumuloCluster class when the MiniAccumuloClusterImpl class launches the Table Server using Java exec. At the moment, the class has hard-coded the GC params for Sun JVM.  My intention is that it will use System.getenv("GC_POLICY_ARGS").
    
     I did not include this change yet.
    
    If the approach that I've taken makes sense, let me know and I will complete the change to MiniAccumuloClusterImpl
    



---
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] accumulo pull request: Fixes harded coded Sun JVM in config and en...

Posted by haydenmarchant <gi...@git.apache.org>.
Github user haydenmarchant commented on a diff in the pull request:

    https://github.com/apache/accumulo/pull/9#discussion_r14173926
  
    --- Diff: bin/bootstrap_config.sh ---
    @@ -119,6 +123,86 @@ while [[ "${OVERWRITE}" = "0" ]]; do
     done
     echo "Coppying configuration files to: ${CONF_DIR}"
     
    +if [[ -z "${SIZE}" ]]; then
    --- End diff --
    
    Before my change, the sets of variables describing the JVM params were initiated before the user prompts (_1GB_master, _512MB_gc etc......).  However, the JVM settings for the 3GB tservers contains GC settings which depend on JVM vendor. So I had to put the JVM choice before setting the 3GB model. In theory, the JVM vendor could influence other variables of other processes in the future.
    
    So this could be worded differently - the initiation of the process specific JVM parameters are now being done after the user prompts, instead of before.
    
    Is that good enough reason to leave as is?


---
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] accumulo pull request: ACCUMULO-2944 Fixes harded coded Sun JVM in...

Posted by busbey <gi...@git.apache.org>.
Github user busbey commented on a diff in the pull request:

    https://github.com/apache/accumulo/pull/9#discussion_r14414275
  
    --- Diff: conf/templates/accumulo-env.sh ---
    @@ -51,10 +51,11 @@ test -z "$ACCUMULO_TSERVER_OPTS" && export ACCUMULO_TSERVER_OPTS="${POLICY} ${tS
     test -z "$ACCUMULO_MASTER_OPTS"  && export ACCUMULO_MASTER_OPTS="${POLICY} ${masterHigh_masterLow}"
     test -z "$ACCUMULO_MONITOR_OPTS" && export ACCUMULO_MONITOR_OPTS="${POLICY} ${monitorHigh_monitorLow}"
     test -z "$ACCUMULO_GC_OPTS"      && export ACCUMULO_GC_OPTS="${gcHigh_gcLow}"
    -test -z "$ACCUMULO_GENERAL_OPTS" && export ACCUMULO_GENERAL_OPTS="-XX:+UseConcMarkSweepGC -XX:CMSInitiatingOccupancyFraction=75 -Djava.net.preferIPv4Stack=true"
    +test -z "$ACCUMULO_GENERAL_OPTS" && export ACCUMULO_GENERAL_OPTS="${gcPolicy} -Djava.net.preferIPv4Stack=true ${jaxpDocBuilderFactory} ${randomNumGeneratorProviderOverride}"
     test -z "$ACCUMULO_OTHER_OPTS"   && export ACCUMULO_OTHER_OPTS="${otherHigh_otherLow}"
     # what do when the JVM runs out of heap memory
     export ACCUMULO_KILL_CMD='kill -9 %p'
    +export GC_POLICY_ARGS="${gcPolicy}"
    --- End diff --
    
    where does this exported variable actually get used?


---
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] accumulo pull request: Fixes harded coded Sun JVM in config and en...

Posted by busbey <gi...@git.apache.org>.
Github user busbey commented on a diff in the pull request:

    https://github.com/apache/accumulo/pull/9#discussion_r14138959
  
    --- Diff: bin/bootstrap_config.sh ---
    @@ -119,6 +123,86 @@ while [[ "${OVERWRITE}" = "0" ]]; do
     done
     echo "Coppying configuration files to: ${CONF_DIR}"
     
    +if [[ -z "${SIZE}" ]]; then
    --- End diff --
    
    Why move the memory / mmap type/ hadoop code up here? Could you just prompt for the JVM instead?


---
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] accumulo pull request: ACCUMULO-2944 Fixes harded coded Sun JVM in...

Posted by busbey <gi...@git.apache.org>.
Github user busbey commented on a diff in the pull request:

    https://github.com/apache/accumulo/pull/9#discussion_r14696044
  
    --- Diff: minicluster/src/main/java/org/apache/accumulo/minicluster/impl/MiniAccumuloClusterImpl.java ---
    @@ -259,7 +259,14 @@ private Process _exec(Class<?> clazz, List<String> extraJvmOpts, String... args)
         for (Entry<String,String> sysProp : config.getSystemProperties().entrySet()) {
           argList.add(String.format("-D%s=%s", sysProp.getKey(), sysProp.getValue()));
         }
    -    argList.addAll(Arrays.asList("-XX:+UseConcMarkSweepGC", "-XX:CMSInitiatingOccupancyFraction=75", "-Dapple.awt.UIElement=true", Main.class.getName(), className));
    +    
    +    String gcPolicyArgs = System.getenv("GC_POLICY_ARGS");
    --- End diff --
    
    For this to impact the current MAC uses in the ITs, won't we need to have this env variable set? should we be adding jvm-vendor activated profiles to the pom?


---
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] accumulo pull request: Fixes harded coded Sun JVM in config and en...

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

    https://github.com/apache/accumulo/pull/9#issuecomment-47073129
  
    btw, since the v was taken for version, and j was taken to denote java mmm,I defaulted to using x to denote vendor - is that ok?


---
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] accumulo pull request: ACCUMULO-2944 Fixes harded coded Sun JVM in...

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

    https://github.com/apache/accumulo/pull/9


---
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] accumulo pull request: ACCUMULO-2944 Fixes harded coded Sun JVM in...

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

    https://github.com/apache/accumulo/pull/9#issuecomment-48514562
  
    Please do. As you can see I am not just a newbie to accumulo :)
    
    sorry for the mess. (we accidentally removed the remote ACCUMULO-2944 branch while rebasing, so we had to reopen the pull request)


---
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.
---