-
Notifications
You must be signed in to change notification settings - Fork 451
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fixes bug with starting multiple group of scan servers #4445
Conversation
When trying to start and stop multiple groups of scan servers using the accumulo-cluster script the groups would interfere with each other. This change fixes that by making the the scan servers use the group in the instance id like compactors do. Ran into this while workiing on apache#4444 and pulled it out as a stand alone fix.
@@ -141,7 +141,7 @@ function control_service() { | |||
ACCUMULO_SERVICE_INSTANCE="" | |||
[[ $service == "tserver" && ${NUM_TSERVERS:-1} -gt 1 ]] && ACCUMULO_SERVICE_INSTANCE=${inst_id} | |||
[[ $service == "compactor" ]] && ACCUMULO_SERVICE_INSTANCE="${inst_id}_${5}" | |||
[[ $service == "sserver" && ${NUM_SSERVERS:-1} -gt 1 ]] && ACCUMULO_SERVICE_INSTANCE=${inst_id} | |||
[[ $service == "sserver" ]] && ACCUMULO_SERVICE_INSTANCE="${inst_id}_${5}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could simplify the code a bit more and only use the sserver1_<group>
or compactor1_<queue>
patterns when there's more than one of each type.
ACCUMULO_SERVICE_INSTANCE=""
# Only increment the service name when more than one service is desired.
[[ last_instance_id -gt 1 ]] && ACCUMULO_SERVICE_INSTANCE="${inst_id}"
[[ $service == "compactor" || $service == "sserver" ]] && ACCUMULO_SERVICE_INSTANCE="${ACCUMULO_SERVICE_INSTANCE}_${5}"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could simplify the code a bit more and only use the sserver1_ or compactor1_ patterns when there's more than one of each type.
There is one thing I like about always adding the 1
even if there is only a single server and that is the log naming. If I start w/ a single server process then I will get a log file w/o the 1
. If I change config to running 2 server process then I will get two logs files with numbers and the log file w/o a number is abandoned.
I will take a stab at simplification.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ddanielr I decided to go ahead and merge this for now because we have both tested this change and did not want to test again after trying to make the code shorter.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested and it fixes the sserver collision.
Added a comment about a further improvement but it's not necessary to fix the bug.
When trying to start and stop multiple groups of scan servers using the accumulo-cluster script the groups would interfere with each other. This change fixes that by making the the scan servers use the group in the instance id like compactors do.
Ran into this while workiing on #4444 and pulled it out as a stand alone fix.