You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@accumulo.apache.org by mikewalch <gi...@git.apache.org> on 2017/02/15 19:04:40 UTC
[GitHub] accumulo pull request #218: ACCUMULO-4528 Accumulo scripts improvements
GitHub user mikewalch opened a pull request:
https://github.com/apache/accumulo/pull/218
ACCUMULO-4528 Accumulo scripts improvements
* Accumulo now determines hostname in Java
* Minimized use of environment variables
* Consolidated scripts by moving service.sh and cluster.sh into
accumulo-service and accumulo-cluster which were moved to contrib/.
Also, moved masters, tservers, tracers, etc files to contrib/conf/
* Removed accumulo-watcher script as restarting services should not be
handled by Accumulo scripts
* NUMA and multiple tservers are no longer configured in scripts but
could be NUMA commands could be added using ACCUMULO_JAVA_PREFIX env
variable.
* Renamed accumulo-env.sh to accumulo.conf
* Moved create-config and build-native from accumulo script to
accumulo-config and accumulo-native scripts in contrib/
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/mikewalch/accumulo script-refactor
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/accumulo/pull/218.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 #218
----
----
---
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 issue #218: ACCUMULO-4528 Accumulo scripts improvements
Posted by milleruntime <gi...@git.apache.org>.
Github user milleruntime commented on the issue:
https://github.com/apache/accumulo/pull/218
+1 On the renaming of accumulo-env.sh to accumulo.conf. The .conf extension is pretty clear what the file is for, its simply for configuration purposes. The -env.sh extension tells me this shell script is setting up the environment but could be doing more then configuration, which makes me nervous just seeing the name of the file. From a user perspective, I would be much more inclined and comfortable to modify a .conf file.
---
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 #218: ACCUMULO-4528 Accumulo scripts improvements
Posted by keith-turner <gi...@git.apache.org>.
Github user keith-turner commented on a diff in the pull request:
https://github.com/apache/accumulo/pull/218#discussion_r101378663
--- Diff: INSTALL.md ---
@@ -25,11 +25,11 @@ source code. Unpack as follows.
tar xzf <some dir>/accumulo-X.Y.Z-bin.tar.gz
cd accumulo-X.Y.Z
-There are three scripts in the the `bin/` directory that are used to manage Accumulo:
+There are three scripts in the tarball distribution that are used to manage Accumulo:
-1. `accumulo` - Runs Accumulo command-line tools and starts Accumulo processes
-2. `accumulo-service` - Runs Accumulo processes as services
-3. `accumulo-cluster` - Manages Accumulo cluster on a single node or several nodes
+1. `bin/accumulo` - Runs Accumulo command-line tools and starts Accumulo processes
+2. `contrib/accumulo-service` - Runs Accumulo processes as services
--- End diff --
What does it mean for a script to be in contrib vs bin?
---
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 #218: ACCUMULO-4528 Accumulo scripts improvements
Posted by ctubbsii <gi...@git.apache.org>.
Github user ctubbsii commented on a diff in the pull request:
https://github.com/apache/accumulo/pull/218#discussion_r101364066
--- Diff: INSTALL.md ---
@@ -76,19 +76,18 @@ When configuring Accumulo the following information about these dependencies
must be provided.
* **Location of Zookeepers** : Provide this by setting `instance.zookeeper.host`
- in `conf/accumulo-site.xml`.
+ in `accumulo-site.xml`.
* **Where to store data** : Provide this by setting `instance.volumes` in
- `conf/accumulo-site.xml`. If your namenode is running at 192.168.1.9:9000
+ `accumulo-site.xml`. If your namenode is running at 192.168.1.9:9000
and you want to store data in `/accumulo` in HDFS, then set
`instance.volumes` to `hdfs://192.168.1.9:9000/accumulo`.
* **Location of Zoookeeper and Hadoop jars** : Setting `ZOOKEEPER_HOME` and
- `HADOOP_PREFIX` in `conf/accumulo-env.sh` will help Accumulo find these
- jars.
+ `HADOOP_PREFIX` in `accumulo.conf` will help Accumulo find these jars.
--- End diff --
Using the default config, this is true, but not necessarily otherwise.
---
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 #218: ACCUMULO-4528 Accumulo scripts improvements
Posted by ctubbsii <gi...@git.apache.org>.
Github user ctubbsii commented on a diff in the pull request:
https://github.com/apache/accumulo/pull/218#discussion_r101363679
--- Diff: INSTALL.md ---
@@ -57,15 +57,15 @@ The script will ask you questions about your set up. Below are some suggestions:
processes like Hadoop, Zookeeper, and the Accumulo client code. If Accumulo
worker processes are swapped out and unresponsive, they may be killed.
-After the `create-config` command is run, the `conf/` directory will contain
-`accumulo-env.sh`, `accumulo-site.xml`, and few a additional files. These files require
+After the `accumulo-config` command is run, the `conf/` directory will contain
+`accumulo.conf`, `accumulo-site.xml`, and few a additional files. These files require
a few edits before starting Accumulo.
### Secret
Accumulo coordination and worker processes can only communicate with each other
if they share the same secret key. To change the secret key set
-`instance.secret` in `conf/accumulo-site.xml`. Changing this secret key from
+`instance.secret` in `accumulo-site.xml`. Changing this secret key from
--- End diff --
I didn't read it that way. I think the previous paragraph already identified the location of the file. This is just referring to it by name, assuming you already read the part which told you where to find it.
---
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 #218: ACCUMULO-4528 Accumulo scripts improvements
Posted by mikewalch <gi...@git.apache.org>.
Github user mikewalch commented on a diff in the pull request:
https://github.com/apache/accumulo/pull/218#discussion_r101530931
--- Diff: INSTALL.md ---
@@ -25,11 +25,11 @@ source code. Unpack as follows.
tar xzf <some dir>/accumulo-X.Y.Z-bin.tar.gz
cd accumulo-X.Y.Z
-There are three scripts in the the `bin/` directory that are used to manage Accumulo:
+There are three scripts in the tarball distribution that are used to manage Accumulo:
-1. `accumulo` - Runs Accumulo command-line tools and starts Accumulo processes
-2. `accumulo-service` - Runs Accumulo processes as services
-3. `accumulo-cluster` - Manages Accumulo cluster on a single node or several nodes
+1. `bin/accumulo` - Runs Accumulo command-line tools and starts Accumulo processes
+2. `contrib/accumulo-service` - Runs Accumulo processes as services
--- End diff --
I thought about having a contrib and a bin directory. I think there should only be a bin/ directory. Two directories will confuse people. We can communicate that scripts are optional/add-on in the documentation rather than the directory structure. To limit the number of scripts in bin, we can combine many of the scripts in contrib into one or two scripts. If a script doesn't work or we don't want to support it any more the in the tarball distribution, it should either be removed or move to a contrib directory in the repo that doesn't get added to the distribution. Contrib scripts should not be distributed. As for the scripts in bin/, downstream packagers can pick and choose what they include.
---
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 issue #218: ACCUMULO-4528 Accumulo scripts improvements
Posted by mikewalch <gi...@git.apache.org>.
Github user mikewalch commented on the issue:
https://github.com/apache/accumulo/pull/218
Thanks everyone for reviewing! Your feedback resulted in a lot of changes that made this PR better. I think I have addressed all comments. Unless someone needs more time to review or wants more changes, I plan on squashing and merging by end of today.
---
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 #218: ACCUMULO-4528 Accumulo scripts improvements
Posted by ctubbsii <gi...@git.apache.org>.
Github user ctubbsii commented on a diff in the pull request:
https://github.com/apache/accumulo/pull/218#discussion_r101373614
--- Diff: assemble/contrib/tool.sh ---
@@ -26,14 +26,21 @@ contrib=$( cd -P "$( dirname "$SOURCE" )" && pwd )
basedir=$( cd -P "${contrib}"/.. && pwd )
# Stop: Resolve Script Directory
-source "$basedir"/libexec/load-env.sh
+conf="${basedir}/conf"
+if [ ! -f "${conf}/accumulo.conf" ]; then
+ echo "accumulo.conf must exist in $conf"
+ echo "Run 'accumulo-config' to create it or copy it from $conf/examples"
+ echo "Follow the instructions in INSTALL.md to edit it for your environment."
+ exit 1
+fi
+source "$conf/accumulo.conf"
if [[ -z "$HADOOP_PREFIX" ]] ; then
- echo "HADOOP_PREFIX is not set. Please make sure it's set globally or in conf/accumulo-env.sh"
+ echo "HADOOP_PREFIX is not set. Please make sure it's set globally or in $conf/accumulo.conf"
--- End diff --
I think this is currently necessary for the property interpolation in the `accumulo-site.xml` file, but I'd really like to phase out having it as a requirement.
---
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 issue #218: ACCUMULO-4528 Accumulo scripts improvements
Posted by mikewalch <gi...@git.apache.org>.
Github user mikewalch commented on the issue:
https://github.com/apache/accumulo/pull/218
I talked to @milleruntime offline about accumulo-env.sh rename. For this PR, I decided to not do the rename as concerns were brought up about changing name to accumulo.conf. In the end, I would like a better name for accumulo-env.sh but I don't want to force a change until we find a better name.
---
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 #218: ACCUMULO-4528 Accumulo scripts improvements
Posted by ctubbsii <gi...@git.apache.org>.
Github user ctubbsii commented on a diff in the pull request:
https://github.com/apache/accumulo/pull/218#discussion_r101373362
--- Diff: assemble/contrib/tool.sh ---
@@ -26,14 +26,21 @@ contrib=$( cd -P "$( dirname "$SOURCE" )" && pwd )
basedir=$( cd -P "${contrib}"/.. && pwd )
# Stop: Resolve Script Directory
-source "$basedir"/libexec/load-env.sh
+conf="${basedir}/conf"
+if [ ! -f "${conf}/accumulo.conf" ]; then
+ echo "accumulo.conf must exist in $conf"
--- End diff --
Why wouldn't this file be optional?
---
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 #218: ACCUMULO-4528 Accumulo scripts improvements
Posted by milleruntime <gi...@git.apache.org>.
Github user milleruntime commented on a diff in the pull request:
https://github.com/apache/accumulo/pull/218#discussion_r101361758
--- Diff: INSTALL.md ---
@@ -57,15 +57,15 @@ The script will ask you questions about your set up. Below are some suggestions:
processes like Hadoop, Zookeeper, and the Accumulo client code. If Accumulo
worker processes are swapped out and unresponsive, they may be killed.
-After the `create-config` command is run, the `conf/` directory will contain
-`accumulo-env.sh`, `accumulo-site.xml`, and few a additional files. These files require
+After the `accumulo-config` command is run, the `conf/` directory will contain
+`accumulo.conf`, `accumulo-site.xml`, and few a additional files. These files require
a few edits before starting Accumulo.
### Secret
Accumulo coordination and worker processes can only communicate with each other
if they share the same secret key. To change the secret key set
-`instance.secret` in `conf/accumulo-site.xml`. Changing this secret key from
+`instance.secret` in `accumulo-site.xml`. Changing this secret key from
--- End diff --
Did you want to move accumulo-site.xml out of conf?
---
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 #218: ACCUMULO-4528 Accumulo scripts improvements
Posted by mikewalch <gi...@git.apache.org>.
Github user mikewalch commented on a diff in the pull request:
https://github.com/apache/accumulo/pull/218#discussion_r101782473
--- Diff: INSTALL.md ---
@@ -40,13 +40,13 @@ Accumulo has some optional native code that improves its performance and
stability. Before configuring Accumulo, attempt to build this native code
with the following command.
- accumulo build-native
+ ./contrib/accumulo-native
If the command fails, its OK to continue with setup and resolve the issue later.
Run the command below to create configuration for Accumulo in `conf/`:
- accumulo create-config
+ ./contrib/accumulo-config
--- End diff --
This was moved to `bin/accumulo-util create-config`
---
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 #218: ACCUMULO-4528 Accumulo scripts improvements
Posted by mikewalch <gi...@git.apache.org>.
Github user mikewalch commented on a diff in the pull request:
https://github.com/apache/accumulo/pull/218#discussion_r101784185
--- Diff: assemble/conf/examples/generic_logger.properties ---
@@ -15,7 +15,7 @@
# Write out everything at the DEBUG level to the debug log
log4j.appender.A2=org.apache.log4j.RollingFileAppender
-log4j.appender.A2.File=${org.apache.accumulo.core.dir.log}/${org.apache.accumulo.core.application}_${accumulo.service.instance}${org.apache.accumulo.core.ip.localhost.hostname}_fromprops.debug.log
+log4j.appender.A2.File=${org.apache.accumulo.core.dir.log}/${org.apache.accumulo.core.application}${accumulo.service.instance}_${org.apache.accumulo.core.ip.localhost.hostname}_fromprops.debug.log
--- End diff --
This should be done in a separate PR. Created issue https://issues.apache.org/jira/browse/ACCUMULO-4588
---
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 #218: ACCUMULO-4528 Accumulo scripts improvements
Posted by mikewalch <gi...@git.apache.org>.
Github user mikewalch commented on a diff in the pull request:
https://github.com/apache/accumulo/pull/218#discussion_r101785204
--- Diff: INSTALL.md ---
@@ -76,19 +76,18 @@ When configuring Accumulo the following information about these dependencies
must be provided.
* **Location of Zookeepers** : Provide this by setting `instance.zookeeper.host`
- in `conf/accumulo-site.xml`.
+ in `accumulo-site.xml`.
* **Where to store data** : Provide this by setting `instance.volumes` in
- `conf/accumulo-site.xml`. If your namenode is running at 192.168.1.9:9000
+ `accumulo-site.xml`. If your namenode is running at 192.168.1.9:9000
and you want to store data in `/accumulo` in HDFS, then set
`instance.volumes` to `hdfs://192.168.1.9:9000/accumulo`.
* **Location of Zoookeeper and Hadoop jars** : Setting `ZOOKEEPER_HOME` and
- `HADOOP_PREFIX` in `conf/accumulo-env.sh` will help Accumulo find these
- jars.
+ `HADOOP_PREFIX` in `accumulo.conf` will help Accumulo find these jars.
--- End diff --
Improve documentation in 1e8d87cd311
---
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 #218: ACCUMULO-4528 Accumulo scripts improvements
Posted by ctubbsii <gi...@git.apache.org>.
Github user ctubbsii commented on a diff in the pull request:
https://github.com/apache/accumulo/pull/218#discussion_r101576229
--- Diff: INSTALL.md ---
@@ -25,11 +25,11 @@ source code. Unpack as follows.
tar xzf <some dir>/accumulo-X.Y.Z-bin.tar.gz
cd accumulo-X.Y.Z
-There are three scripts in the the `bin/` directory that are used to manage Accumulo:
+There are three scripts in the tarball distribution that are used to manage Accumulo:
-1. `accumulo` - Runs Accumulo command-line tools and starts Accumulo processes
-2. `accumulo-service` - Runs Accumulo processes as services
-3. `accumulo-cluster` - Manages Accumulo cluster on a single node or several nodes
+1. `bin/accumulo` - Runs Accumulo command-line tools and starts Accumulo processes
+2. `contrib/accumulo-service` - Runs Accumulo processes as services
--- End diff --
Seems reasonable.
---
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 #218: ACCUMULO-4528 Accumulo scripts improvements
Posted by milleruntime <gi...@git.apache.org>.
Github user milleruntime commented on a diff in the pull request:
https://github.com/apache/accumulo/pull/218#discussion_r101582008
--- Diff: INSTALL.md ---
@@ -57,15 +57,15 @@ The script will ask you questions about your set up. Below are some suggestions:
processes like Hadoop, Zookeeper, and the Accumulo client code. If Accumulo
worker processes are swapped out and unresponsive, they may be killed.
-After the `create-config` command is run, the `conf/` directory will contain
-`accumulo-env.sh`, `accumulo-site.xml`, and few a additional files. These files require
+After the `accumulo-config` command is run, the `conf/` directory will contain
+`accumulo.conf`, `accumulo-site.xml`, and few a additional files. These files require
a few edits before starting Accumulo.
### Secret
Accumulo coordination and worker processes can only communicate with each other
if they share the same secret key. To change the secret key set
-`instance.secret` in `conf/accumulo-site.xml`. Changing this secret key from
+`instance.secret` in `accumulo-site.xml`. Changing this secret key from
--- End diff --
Got it. I took the changes out of context.
---
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 #218: ACCUMULO-4528 Accumulo scripts improvements
Posted by mikewalch <gi...@git.apache.org>.
Github user mikewalch commented on a diff in the pull request:
https://github.com/apache/accumulo/pull/218#discussion_r101610417
--- Diff: assemble/contrib/tool.sh ---
@@ -26,14 +26,21 @@ contrib=$( cd -P "$( dirname "$SOURCE" )" && pwd )
basedir=$( cd -P "${contrib}"/.. && pwd )
# Stop: Resolve Script Directory
-source "$basedir"/libexec/load-env.sh
+conf="${basedir}/conf"
+if [ ! -f "${conf}/accumulo.conf" ]; then
+ echo "accumulo.conf must exist in $conf"
--- End diff --
I made it optional in 7f9da9f12ce5940
---
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 #218: ACCUMULO-4528 Accumulo scripts improvements
Posted by ctubbsii <gi...@git.apache.org>.
Github user ctubbsii commented on a diff in the pull request:
https://github.com/apache/accumulo/pull/218#discussion_r101363305
--- Diff: INSTALL.md ---
@@ -40,13 +40,13 @@ Accumulo has some optional native code that improves its performance and
stability. Before configuring Accumulo, attempt to build this native code
with the following command.
- accumulo build-native
+ ./contrib/accumulo-native
If the command fails, its OK to continue with setup and resolve the issue later.
Run the command below to create configuration for Accumulo in `conf/`:
- accumulo create-config
+ ./contrib/accumulo-config
--- End diff --
Is this an optional command? Is it possible to run Accumulo without doing this? If not, then I don't think contrib is the right place.
---
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 #218: ACCUMULO-4528 Accumulo scripts improvements
Posted by mikewalch <gi...@git.apache.org>.
Github user mikewalch commented on a diff in the pull request:
https://github.com/apache/accumulo/pull/218#discussion_r101609853
--- Diff: INSTALL.md ---
@@ -40,13 +40,13 @@ Accumulo has some optional native code that improves its performance and
stability. Before configuring Accumulo, attempt to build this native code
with the following command.
- accumulo build-native
+ ./contrib/accumulo-native
--- End diff --
I pushed a commit where I moved the command to `bin/accumulo-util build-native`
---
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 #218: ACCUMULO-4528 Accumulo scripts improvements
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/accumulo/pull/218
---
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 #218: ACCUMULO-4528 Accumulo scripts improvements
Posted by ctubbsii <gi...@git.apache.org>.
Github user ctubbsii commented on a diff in the pull request:
https://github.com/apache/accumulo/pull/218#discussion_r101365020
--- Diff: assemble/conf/examples/generic_logger.properties ---
@@ -15,7 +15,7 @@
# Write out everything at the DEBUG level to the debug log
log4j.appender.A2=org.apache.log4j.RollingFileAppender
-log4j.appender.A2.File=${org.apache.accumulo.core.dir.log}/${org.apache.accumulo.core.application}_${accumulo.service.instance}${org.apache.accumulo.core.ip.localhost.hostname}_fromprops.debug.log
+log4j.appender.A2.File=${org.apache.accumulo.core.dir.log}/${org.apache.accumulo.core.application}${accumulo.service.instance}_${org.apache.accumulo.core.ip.localhost.hostname}_fromprops.debug.log
--- End diff --
Rather than produce these system properties in Accumulo code, we should use the log4j feature to do `${sys:HOSTNAME}` (might be log4j2 only, but that's okay... we should switch to that)
---
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 #218: ACCUMULO-4528 Accumulo scripts improvements
Posted by mikewalch <gi...@git.apache.org>.
Github user mikewalch commented on a diff in the pull request:
https://github.com/apache/accumulo/pull/218#discussion_r101365500
--- Diff: INSTALL.md ---
@@ -57,15 +57,15 @@ The script will ask you questions about your set up. Below are some suggestions:
processes like Hadoop, Zookeeper, and the Accumulo client code. If Accumulo
worker processes are swapped out and unresponsive, they may be killed.
-After the `create-config` command is run, the `conf/` directory will contain
-`accumulo-env.sh`, `accumulo-site.xml`, and few a additional files. These files require
+After the `accumulo-config` command is run, the `conf/` directory will contain
+`accumulo.conf`, `accumulo-site.xml`, and few a additional files. These files require
a few edits before starting Accumulo.
### Secret
Accumulo coordination and worker processes can only communicate with each other
if they share the same secret key. To change the secret key set
-`instance.secret` in `conf/accumulo-site.xml`. Changing this secret key from
+`instance.secret` in `accumulo-site.xml`. Changing this secret key from
--- End diff --
It's still located in conf for the tarball distribution but I changed the manual to not include directory/path of the file as accumulo-site.xml could be in other locations like `/etc/accumulo` in downstream packaging.
---
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 #218: ACCUMULO-4528 Accumulo scripts improvements
Posted by ctubbsii <gi...@git.apache.org>.
Github user ctubbsii commented on a diff in the pull request:
https://github.com/apache/accumulo/pull/218#discussion_r101365517
--- Diff: assemble/conf/examples/generic_logger.properties ---
@@ -24,7 +24,7 @@ log4j.appender.A2.layout.ConversionPattern=%d{ISO8601} [%-8c{2}] %-5p: %m%n
# Write out INFO and higher to the regular log
log4j.appender.A3=org.apache.log4j.RollingFileAppender
-log4j.appender.A3.File=${org.apache.accumulo.core.dir.log}/${org.apache.accumulo.core.application}_${accumulo.service.instance}${org.apache.accumulo.core.ip.localhost.hostname}_fromprops.log
+log4j.appender.A3.File=${org.apache.accumulo.core.dir.log}/${org.apache.accumulo.core.application}${accumulo.service.instance}_${org.apache.accumulo.core.ip.localhost.hostname}_fromprops.log
--- End diff --
Maybe we shouldn't enable the rolling file appender in the default configs. Users can add that if they want to. A better option would probably be to use the syslog appender by default.
---
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 #218: ACCUMULO-4528 Accumulo scripts improvements
Posted by mikewalch <gi...@git.apache.org>.
Github user mikewalch commented on a diff in the pull request:
https://github.com/apache/accumulo/pull/218#discussion_r101784212
--- Diff: assemble/conf/examples/generic_logger.properties ---
@@ -24,7 +24,7 @@ log4j.appender.A2.layout.ConversionPattern=%d{ISO8601} [%-8c{2}] %-5p: %m%n
# Write out INFO and higher to the regular log
log4j.appender.A3=org.apache.log4j.RollingFileAppender
-log4j.appender.A3.File=${org.apache.accumulo.core.dir.log}/${org.apache.accumulo.core.application}_${accumulo.service.instance}${org.apache.accumulo.core.ip.localhost.hostname}_fromprops.log
+log4j.appender.A3.File=${org.apache.accumulo.core.dir.log}/${org.apache.accumulo.core.application}${accumulo.service.instance}_${org.apache.accumulo.core.ip.localhost.hostname}_fromprops.log
--- End diff --
This should be done in a separate PR. Created issue https://issues.apache.org/jira/browse/ACCUMULO-4588
---
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 #218: ACCUMULO-4528 Accumulo scripts improvements
Posted by ctubbsii <gi...@git.apache.org>.
Github user ctubbsii commented on a diff in the pull request:
https://github.com/apache/accumulo/pull/218#discussion_r101399584
--- Diff: INSTALL.md ---
@@ -25,11 +25,11 @@ source code. Unpack as follows.
tar xzf <some dir>/accumulo-X.Y.Z-bin.tar.gz
cd accumulo-X.Y.Z
-There are three scripts in the the `bin/` directory that are used to manage Accumulo:
+There are three scripts in the tarball distribution that are used to manage Accumulo:
-1. `accumulo` - Runs Accumulo command-line tools and starts Accumulo processes
-2. `accumulo-service` - Runs Accumulo processes as services
-3. `accumulo-cluster` - Manages Accumulo cluster on a single node or several nodes
+1. `bin/accumulo` - Runs Accumulo command-line tools and starts Accumulo processes
+2. `contrib/accumulo-service` - Runs Accumulo processes as services
--- End diff --
In my view, `contrib/` should be for add-ons/wrappers which could be separate projects on their own, but which we are currently maintaining to provide convenient functionality.
I think it makes sense to have a basic launch script in `bin/`, which minimally wraps the execution of java. That minimal wrapper wouldn't make sense as its own project and thus (by my opinion) shouldn't be in `contrib/`.
I'm far less opinionated on the convenience scripts for producing the example configs and for executing `make` on the unpacked native tarball. They could probably go either place.
---
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 #218: ACCUMULO-4528 Accumulo scripts improvements
Posted by ctubbsii <gi...@git.apache.org>.
Github user ctubbsii commented on a diff in the pull request:
https://github.com/apache/accumulo/pull/218#discussion_r101363077
--- Diff: INSTALL.md ---
@@ -40,13 +40,13 @@ Accumulo has some optional native code that improves its performance and
stability. Before configuring Accumulo, attempt to build this native code
with the following command.
- accumulo build-native
+ ./contrib/accumulo-native
--- End diff --
I'm not sure contrib is the best place for this. I think of contrib items as stuff that adds on to, but is unknown to, the core project. The native libraries are something that are optional, but known to Accumulo, and tightly coupled with it. I think of contrib stuff as optional stuff many users can ignore. But, native libraries are something we strongly recommend.
---
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.
---