-
Notifications
You must be signed in to change notification settings - Fork 791
SOLR-14361 Solr should own the boot process (SIP-6) #3983
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
base: main
Are you sure you want to change the base?
Conversation
Adapted start script Tests for bootstrap module
License header Tidy ForbiddenAPI UnusedDeclared artifacts Changelog
epugh
left a comment
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.
This is cool.. Somehow when I read your code it's all so simple and coherant. When I thought conceptually about this, I couldn't get my head around it.
I also think this could be a great opportunity to create a start command from scratch. I know what you have is more of a migration, but I think there is a lot of other bash scripting that is cruft as well that oculd be rethought.
| EXAMPLE_DIR="$SOLR_TIP/example" | ||
|
|
||
| # Set default log4j configuration | ||
| LOG4J_PROPS="$DEFAULT_SERVER_DIR/resources/log4j2.xml" |
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.
I guess this is still one of those "chicken/egg" things that you can't configure in the bootstrap?
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.
Yes, guess it needs to be set fairly early, but who knows, could probably be set programatically in main method as well...
| SCRIPT_SOLR_OPTS+=($SOLR_SSL_OPTS "$SSL_PORT_PROP") | ||
| fi | ||
| # SIP-6: SSL configuration is now handled by ServerConfiguration and ConnectorFactory in Java | ||
| # No need to pass SSL properties explicitly - they're read from environment variables |
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.
I suppose eventually we can remove these comments, but nice to see now in draft mode!
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.
Sure, this is just POC filler.
If we go this direction, it will open up for porting most of the start script to Java, except the JVM level props like heap, gc etc.
|
|
||
| if [ ! -e "$SOLR_SERVER_DIR/start.jar" ]; then | ||
| echo -e "\nERROR: start.jar file not found in $SOLR_SERVER_DIR!\nPlease check your --server-dir parameter to set the correct Solr server directory.\n" | ||
| if [ ! -e "$SOLR_SERVER_DIR/solr-start.jar" ]; then |
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.
solr.jar ??? versus solr-start.jar??
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.
Yep, the original SIP text proposes solr.jar here. But I got the idea that if we use solr-start.jar, it will still match a grep for start.jar when listing processes :) Really, since the module name is bootstrap it will probably be called solr-bootstrap-11.0.0.jar in maven. It's just a name, noone will call it manually except those who write their own daemon wrappers or something.
| # OOME is thrown. Program operation after OOME is unpredictable. | ||
| "-XX:+CrashOnOutOfMemoryError" "-XX:ErrorFile=${SOLR_LOGS_DIR}/jvm_crash_%p.log" \ | ||
| "-Djetty.home=$SOLR_SERVER_DIR" "-Dsolr.solr.home=$SOLR_HOME" "-Dsolr.install.dir=$SOLR_TIP" "-Dsolr.install.symDir=$SOLR_TIP_SYM" \ | ||
| "-Dsolr.jetty.home=$SOLR_SERVER_DIR" "-Dsolr.solr.home=$SOLR_HOME" "-Dsolr.install.dir=$SOLR_TIP" "-Dsolr.install.symDir=$SOLR_TIP_SYM" \ |
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.
Any chance that this is an opportunity to rethink all these various flavours of path? Could we just have a single value, and derive everything esle? Do we really need control over these? Also, if we kept them as env variables, couldn't you just read them IN as part of the java code? I find all these various settings quite confusing to keep straight. Last thought in my long run-opn thought... I would love to have the production.properties or production.yml type file define all these, and just pass that into solr-start.jar.
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.
Sure, probably lots of cleanup to be done there. But in this POC my goal is to prove that it is not a huge change to do this thing, and the startup command line is almost the same. I just normalized some sys props here.
| - **Auto-SSL generation**: Automatically generates self-signed certificates for development and testing | ||
| - **Modern protocols**: HTTP/2, HTTP/2 Cleartext (h2c), and ALPN support | ||
| - **Security hardening**: Built-in security headers (CSP, X-Frame-Options, etc.) | ||
| - **Flexible configuration**: 80+ properties configurable via environment variables |
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.
I like this... And maybe this gives me a place to push the "we ship dev.yml, prod.yml and prod-with-security.yml"?
After many years, here is a fresh implementation of SIP-6.
New bootsrap module
This PR adds a new gradle module
solr/bootstrapwhich producessolr-start.jar, which is copied intoserver/in the binary dist, and is a drop-in replacement for Jetty'sstart.jarand all the jetty xml-config-files.SolrStartclass. This basically does everything that is currently done by jetty "modules" defined in xml inserver/etc/*.xml, such as selecting between HTTP and HTTPS, setup requestlogging etc etc.SOLR_SSL_ENABLED=true(this is new, just to prove a benefit)web.xmlOn this foundation we can later configure CORS and memory-locking and other low-level stuff. Or replace Jetty with something else.
Slimmer start script
The
bin/solrscript for linux bow starts Solr using the bootstrap module. It is thinner than the original, as all the SSL logic is handled in java by Java in bootstrap module. It can be made even slimmer now that we own thepublic static void main(String[] args)method, that can do a whole lot of config processing before bootstrapping Jetty.The old script is in
bin/solr-classicfor comparison.POC
This is a POC implemented in parallel with existing
start.jarandbin/solr-classicmechanisms. If we decide to go this route, we'd:start.jarDetailed docs
See solr/bootstrap/README.md for a full writeup of the approach.
https://issues.apache.org/jira/browse/SOLR-14361
Disclaimer Most of this solution is AI generated with Claude Code. Prompts were mine though :)