You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@accumulo.apache.org by GitBox <gi...@apache.org> on 2021/09/01 15:11:46 UTC
[GitHub] [accumulo] keith-turner commented on a change in pull request #2238: Add compaction coordinator and compactor to cluster start / stop scripts
keith-turner commented on a change in pull request #2238:
URL: https://github.com/apache/accumulo/pull/2238#discussion_r700286619
##########
File path: assemble/bin/accumulo-cluster
##########
@@ -43,52 +43,68 @@ function invalid_args {
exit 1
}
-function verify_config {
+function parse_fail {
+ echo "Failed to parse ${conf}/cluster.yml"
+ exit 1
+}
+
+function parse_config {
if [[ -f ${conf}/slaves ]]; then
echo "ERROR: A 'slaves' file was found in ${conf}/"
- echo "Accumulo now reads tablet server hosts from 'tservers' and requires that the 'slaves' file not be present to reduce confusion."
+ echo "Accumulo now uses cluster host configuration information from 'cluster.yml' and requires that the 'slaves' file not be present to reduce confusion."
echo "Please rename the 'slaves' file to 'tservers' or remove it if both exist."
Review comment:
This message is no longer correct.
##########
File path: assemble/bin/accumulo-cluster
##########
@@ -43,52 +43,68 @@ function invalid_args {
exit 1
}
-function verify_config {
+function parse_fail {
+ echo "Failed to parse ${conf}/cluster.yml"
+ exit 1
+}
+
+function parse_config {
if [[ -f ${conf}/slaves ]]; then
echo "ERROR: A 'slaves' file was found in ${conf}/"
- echo "Accumulo now reads tablet server hosts from 'tservers' and requires that the 'slaves' file not be present to reduce confusion."
+ echo "Accumulo now uses cluster host configuration information from 'cluster.yml' and requires that the 'slaves' file not be present to reduce confusion."
echo "Please rename the 'slaves' file to 'tservers' or remove it if both exist."
exit 1
fi
- if [[ ! -f ${conf}/tservers ]]; then
- echo "ERROR: A 'tservers' file was not found at ${conf}/tservers"
- echo "Please make sure it exists and is configured with tablet server hosts."
+ if [[ ! -f ${conf}/cluster.yml ]]; then
+ echo "ERROR: A 'cluster.yml' file was not found at ${conf}/cluster.yml"
+ echo "Please make sure it exists and is configured with the host information. Run 'create-config' to create an example configuration."
Review comment:
```suggestion
echo "Please make sure it exists and is configured with the host information. Run 'accumulo-cluster create-config' to create an example configuration."
```
##########
File path: assemble/bin/accumulo-cluster
##########
@@ -43,52 +43,68 @@ function invalid_args {
exit 1
}
-function verify_config {
+function parse_fail {
+ echo "Failed to parse ${conf}/cluster.yml"
+ exit 1
+}
+
+function parse_config {
if [[ -f ${conf}/slaves ]]; then
Review comment:
This could check for existence of `slave` or `tservers` files now.
##########
File path: assemble/bin/accumulo-cluster
##########
@@ -147,84 +164,128 @@ function start_all() {
start_tservers
fi
- grep -Ev '(^#|^\s*$)' "${conf}/$manager_file" | while read -r host; do
- start_service "$host" manager
+ for manager in $MANAGER_HOSTS; do
+ start_service "$manager" manager
+ done
+
+ for gc in $GC_HOSTS; do
+ start_service "$gc" gc
done
- grep -Ev '(^#|^\s*$)' "${conf}/gc" | while read -r host; do
- start_service "$host" gc
+ for monitor in $MONITOR_HOSTS; do
+ start_service "$monitor" monitor
done
- grep -Ev '(^#|^\s*$)' "${conf}/tracers" | while read -r host; do
- start_service "$host" tracer
+ for tracer in $TRACER_HOSTS; do
+ start_service "$tracer" tracer
+ done
+
+ for coordinator in $COORDINATOR_HOSTS; do
+ start_service "$coordinator" compaction-coordinator
+ done
+
+ for queue in $COMPACTION_QUEUES; do
+ Q="COMPACTOR_HOSTS_${queue}"
+ for compactor in "${!Q}"; do
+ start_service "$compactor" compactor "-q" "$queue"
+ done
done
- start_service "$monitor" monitor
}
function start_here() {
local_hosts="$(hostname -a 2> /dev/null) $(hostname) localhost 127.0.0.1 $(get_ip)"
+
for host in $local_hosts; do
- if grep -q "^${host}\$" "${conf}/tservers"; then
- start_service "$host" tserver
- break
- fi
+ for tserver in $TSERVER_HOSTS; do
+ if echo "$tserver" | grep -q "^${host}\$"; then
+ start_service "$host" tserver
+ break
+ fi
+ done
done
for host in $local_hosts; do
- if grep -q "^${host}\$" "${conf}/$manager_file"; then
- start_service "$host" manager
- break
- fi
+ for manager in $MANAGER_HOSTS; do
+ if echo "$manager" | grep -q "^${host}\$"; then
+ start_service "$host" manager
+ break
+ fi
+ done
done
for host in $local_hosts; do
- if grep -q "^${host}\$" "${conf}/gc"; then
- start_service "$host" gc
- break
- fi
+ for gc in $GC_HOSTS; do
+ if echo "$gc" | grep -q "^${host}\$"; then
+ start_service "$host" gc
+ break
+ fi
+ done
done
for host in $local_hosts; do
- if [[ "$host" == "$monitor" ]]; then
- start_service "$host" monitor
- break
- fi
+ for monitor in $MONITOR_HOSTS; do
+ if echo "$monitor" | grep -q "^${host}\$"; then
+ start_service "$host" monitor
+ break
+ fi
+ done
done
for host in $local_hosts; do
- if grep -q "^${host}\$" "${conf}/tracers"; then
- start_service "$host" tracer
- break
- fi
+ for tracer in $TRACER_HOSTS; do
+ if echo "$tracer" | grep -q "^${host}\$"; then
+ start_service "$host" tracer
+ break
+ fi
+ done
done
+
+ for host in $local_hosts; do
+ for coordinator in $COORDINATOR_HOSTS; do
+ if echo "$coordinator" | grep -q "^${host}\$"; then
+ start_service "$coordinator" compaction-coordinator
+ fi
+ done
+ done
+
+ for queue in $COMPACTION_QUEUES; do
+ for host in $local_hosts; do
+ Q="COMPACTOR_HOSTS_${queue}"
+ for compactor in "${!Q}"; do
Review comment:
What are these two lines doing? I don't understand the `"${!Q}"` bash syntax. If not easy/quick to explain, don't worry about it.
##########
File path: assemble/bin/accumulo-cluster
##########
@@ -43,52 +43,68 @@ function invalid_args {
exit 1
}
-function verify_config {
+function parse_fail {
+ echo "Failed to parse ${conf}/cluster.yml"
+ exit 1
+}
+
+function parse_config {
if [[ -f ${conf}/slaves ]]; then
echo "ERROR: A 'slaves' file was found in ${conf}/"
- echo "Accumulo now reads tablet server hosts from 'tservers' and requires that the 'slaves' file not be present to reduce confusion."
+ echo "Accumulo now uses cluster host configuration information from 'cluster.yml' and requires that the 'slaves' file not be present to reduce confusion."
echo "Please rename the 'slaves' file to 'tservers' or remove it if both exist."
exit 1
fi
- if [[ ! -f ${conf}/tservers ]]; then
- echo "ERROR: A 'tservers' file was not found at ${conf}/tservers"
- echo "Please make sure it exists and is configured with tablet server hosts."
+ if [[ ! -f ${conf}/cluster.yml ]]; then
+ echo "ERROR: A 'cluster.yml' file was not found at ${conf}/cluster.yml"
+ echo "Please make sure it exists and is configured with the host information. Run 'create-config' to create an example configuration."
exit 1
fi
- unset manager1
- if [[ -f "${conf}/$manager_file" ]]; then
- manager1=$(grep -Ev '(^#|^\s*$)' "${conf}/$manager_file" | head -1)
+ trap 'rm -f "$CONFIG_FILE"' EXIT
+ CONFIG_FILE=$(mktemp) || exit 1
+ ${accumulo_cmd} org.apache.accumulo.core.conf.cluster.ClusterConfigParser ${conf}/cluster.yml > $CONFIG_FILE || parse_fail
+ . $CONFIG_FILE
Review comment:
This is really neat how the java code creates bash code that can be sourced.
##########
File path: assemble/bin/accumulo-cluster
##########
@@ -43,52 +43,68 @@ function invalid_args {
exit 1
}
-function verify_config {
+function parse_fail {
+ echo "Failed to parse ${conf}/cluster.yml"
+ exit 1
+}
+
+function parse_config {
if [[ -f ${conf}/slaves ]]; then
echo "ERROR: A 'slaves' file was found in ${conf}/"
- echo "Accumulo now reads tablet server hosts from 'tservers' and requires that the 'slaves' file not be present to reduce confusion."
+ echo "Accumulo now uses cluster host configuration information from 'cluster.yml' and requires that the 'slaves' file not be present to reduce confusion."
echo "Please rename the 'slaves' file to 'tservers' or remove it if both exist."
exit 1
fi
- if [[ ! -f ${conf}/tservers ]]; then
- echo "ERROR: A 'tservers' file was not found at ${conf}/tservers"
- echo "Please make sure it exists and is configured with tablet server hosts."
+ if [[ ! -f ${conf}/cluster.yml ]]; then
+ echo "ERROR: A 'cluster.yml' file was not found at ${conf}/cluster.yml"
+ echo "Please make sure it exists and is configured with the host information. Run 'create-config' to create an example configuration."
exit 1
fi
- unset manager1
- if [[ -f "${conf}/$manager_file" ]]; then
- manager1=$(grep -Ev '(^#|^\s*$)' "${conf}/$manager_file" | head -1)
+ trap 'rm -f "$CONFIG_FILE"' EXIT
+ CONFIG_FILE=$(mktemp) || exit 1
+ ${accumulo_cmd} org.apache.accumulo.core.conf.cluster.ClusterConfigParser ${conf}/cluster.yml > $CONFIG_FILE || parse_fail
Review comment:
For the case where the tmp dir is full, the error message in parse_fail could be slightly misleading. Someone may look for non-existent errors in the yaml. However I am not sure its worthwhile improving the error message for this case.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org