dlmarion commented on code in PR #5111:
URL: https://github.com/apache/accumulo/pull/5111#discussion_r1864256526
##########
assemble/conf/default-group-env.sh:
##########
Review Comment:
> but I understand you to mean that because `accumulo-service` can be used
by itself without the cluster script or config file
>
Yes, that's correct.
> Thinking about the use case where `accumulo-service` is used without the
cluster script or cluster config file, I'm slightly less inclined to suggest
moving these. However, I'm still a little dissatisfied with the possibility of
an unbounded growth (because the number of resource groups is not bounded)
cluttering the main config directory, and the fact that because of the file
naming convention, they won't be ordered such that they will be grouped
together (most UIs and CLI tools to inspect directories do so by ordering the
contents by name by default). That can make inspection of the config directory
quite frustrating, when you have a bunch of these files interleaved with the
other files there.
>
One option would be to change the implementation of `get_group_overrides`
from:
```
if [[ -f "${conf}/${group}-group-env.sh" ]]; then
#shellcheck source=../conf/default-group-env.sh
source "${conf}/${group}-group-env.sh" "${service}"
fi
```
to something like:
```
if [[ -f "${conf}/group-overrides-env.sh" ]]; then
#shellcheck source=../conf/group-overrides-env.sh
source "${conf}/group-overrides-env.sh" "${group}" "${service}"
fi
```
The `group-overrides-env.sh` script would then contain nested switch
statements, first on the `group`, then on the `service` like so:
```
group=$1
service=$2
case "$group" in
default)
case "$service" in
manager)
monitor)
gc)
tserver)
compactor)
sserver)
*)
export ACCUMULO_JAVA_OPTS=""
export PROPERTY_OVERRIDES=()
;;
*)
;;
esac
esac
```
> I'm also wondering if this capability is already possible (or nearly
possible, with slight tweaks) using the existing accumulo-env.sh, which
optionally could source additional files, if the user wanted to break it out by
resource group, without hard-coding that behavior and file naming conventions
into that script.
Maybe? I think we would need to change the way that `JAVA_OPTS` is built in
`accumulo-env.sh`, appending `ACCUMULO_JAVA_OPTS` to `JAVA_OPTS` later in the
script.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
