cpoerschke commented on a change in pull request #2263:
URL: https://github.com/apache/lucene-solr/pull/2263#discussion_r568436589
##########
File path: solr/bin/solr
##########
@@ -2115,6 +2128,15 @@ function start_solr() {
SOLR_OPTS+=($AUTHC_OPTS)
fi
+ # 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
+ if [[ -n "$SOLR_HEAP_DUMP_DIR" ]]; then
+ SOLR_OPTS+=("-XX:+HeapDumpOnOutOfMemoryError")
+ SOLR_OPTS+=("-XX:HeapDumpPath=$SOLR_HEAP_DUMP_DIR/solr-$(date
+%s)-pid$$.hprof")
Review comment:
How about also optionally supporting customisation of the file name e.g.
via a `SOLR_HEAP_DUMP_FILE` variable? Reasons users might wish to customise:
* inclusion of `SOLR_PORT` in the file name to more easily differentiate
dumps for different Solr instances on the same machine
* preference of (say) `date -u '+%Y%m%d-%H%M%S'` over `date +%s` for the
timestamp
* always use the same dump file as a way to limit the amount of disk space
successive OOMs can use up (a colleague of mine had this insight)
* omission of the pid and restriction of the timestamp e.g. to `date -u
'+%Y%m%d'` so that at most one OOM file per day would exist
* omission of the pid to avoid confusion when running in the background
(because the the pid would be that of the shell script and not that of the Solr
JVM, I think)
----------------------------------------------------------------
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
