HoustonPutman commented on a change in pull request #2055:
URL: https://github.com/apache/lucene-solr/pull/2055#discussion_r516805804
##########
File path: solr/bin/solr
##########
@@ -2172,6 +2172,12 @@ function start_solr() {
SOLR_OPTS+=($AUTHC_OPTS)
fi
+ # If a heap dump directory is specified, enable it in SOLR_OPTS
+ if [[ -n "$SOLR_HEAP_DUMP_DIR" ]]; then
Review comment:
I kind of like the idea of having a boolean flag to turn on heap dumps,
and have a default location in the logs directory. This logic would be fine,
but having an additional check above that checks that `SOLR_HEAP_DUMP_DIR` is
empty and `SOLR_HEAP_DUMP` is true, then sets
`SOLR_HEAP_DUMP_DIR="${SOLR_LOG_DIR}/dumps"`.
Just a thought, for ease of use.
##########
File path: solr/docker/include/scripts/solr-fg
##########
@@ -15,21 +15,19 @@ if [[ -z "${OOM:-}" ]]; then
OOM='none'
fi
case "$OOM" in
Review comment:
Should this also be moved to bin/solr?
##########
File path: solr/bin/solr
##########
@@ -2172,6 +2172,12 @@ function start_solr() {
SOLR_OPTS+=($AUTHC_OPTS)
fi
Review comment:
```suggestion
# If a heap dump directory is specified, enable it in SOLR_OPTS
if [[ -z "$SOLR_HEAP_DUMP_DIR" ]] && [[ "$SOLR_HEAP_DUMP" == "true" ]]; then
SOLR_HEAP_DUMP_DIR="${SOLR_LOGS_DIR}/dumps"
fi
```
I like the idea of having a boolean flag that lets you say that you want a
heap dump at the default location of heap dumps. Not a necessity, just a
thought.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org
