Skip to content
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

Redesign of GlassFish bootstrap #25183

Draft
wants to merge 38 commits into
base: master
Choose a base branch
from
Draft

Conversation

dmatej
Copy link
Contributor

@dmatej dmatej commented Oct 12, 2024

Main feature changes

  • Fixes JDK loggers can initialize the JDK LogManager before GlassFishLogManager is set #25133 - we use the standard JVM option, however complete initialization is finished by HK2 service.
  • ClassLoaders reflect boot phases
  • All our classloaders have names
  • asenv.conf respects existing environment
  • Server's logging manager is set by JVM option
  • All our classloaders are "parallel capable".
  • Asadmin and nadmin and startserv scripts use JPMS

Bootstrap libs

  • glassfish.jar split to non-osgi and osgi part
  • glassfish-jdk-extensions split from common-util and doesn't support osgi.
  • simple-glassfish-api goes to command-line cp and doesn't support osgi.
  • 4 libs support JPMS: glassfish.jar, glassfish-jdk-extensions, glassfish-jul-extension, simple-glassfish-api
  • these 4 libs are used as JPMS modules.
  • *.cli jars were moved to install root as they are simply executable jars.

Bootstrap design

  • Basically still the same, but more comprehensible.
  • Executable JAR (admin-cli.jar), started by asadmin script or startserv script starts appserver-cli.jar
  • That runs GlassFishMainLauncher which prepares command line to run the server, runs GlassFishMain and stops itself.
  • GlassFishMain does some basic stuff and initializes OSGi and HK2 environment.
  • Server started.

Appclient changes

  • JPMS broke many Ant tests, to fix them I had to reimplement also parts of appclient.
  • Removed my 2 years old fix to run single class, because it fixed tests, but was not correct
  • Created new classloaders as replacements of classpath in the appclient script
  • Moved classes, created interfaces to disconnect gf-client and gf-client-module
    • Now they are not connected via classpath on command line and manifest any more
    • gf-client serves just to run CliBootstrap which generates command for the appclient script
    • The script then runs gf-client yet once with the Java agent to enable class instrumentation, to create the client container and set the classloader hierarchy.
    • *Facade classes then use the thread classloader to get the container and launch it.
  • JWS did not and will not work for now
  • There is few other ways to run the client, fortunately the test coverage is good here.

Related changes

  • Were mostly pushed in own PRs this week, but some are here, some are waiting, but this should be a safe point.
  • asenv.conf on linux respects external env variables. I don't know how to do that on Windows, so there it is as it was before.
  • Added reproducer test for JDK loggers can initialize the JDK LogManager before GlassFishLogManager is set #25133
  • Classpath is not added to manifests. It caused issues with classloading, ie defaul-web.xml filter was ignored, required classes were still found by parent classloader. The manifest works just if files are when expected which is the case of the server, but then something else was not working - responsibility of classloader hierarchy. See Fixed default-web.xml #25179
  • Changes in appclient led to a discovery of several other changes - I will create PRs individually.

Future PRs

  • Next PR will disassemble SystemPropertyConstants and add enum making initialization of env and sys props easier.
  • glassfish-jdk-extensions now contains just two classes, we will go through common-util and move here more things. Basically it should contain implementations which might be implemented in future JDK versions. It will never have dependencies on other GF libs.
  • common-util will remain dependent on HK2, OSGI, whatever. It is used quite often, we should optimize it.
  • we will get rid of felix-framework.jar because it overlaps with osgi-core.jar, or we have to repackage it. We have to try which solution is the best. JPMS hates it.
  • Might be possible to start the logging immediately after start, I am not sure with that now, I need to prototype it first.
  • I have around 10 discovered minor issues on paper, should be faster to fix.

Review

Probably is better to go through commits.

git remote add dmatej git@github.com:dmatej/glassfish.git
git checkout -b gjule-vs-gc dmatej/gjule-vs.gc
git diff -w -M git show -M -w dmatej/gjule-vs-gc^^^^^^^^^

Notes

  • Startup speed is nearly the same as with previous version - measured around some 50 times, this version has the best result, but we are talking about change in less then 10 ms of around 2500 millis (the time including CLI).
  • Breaking changes - breaking in meaning of jar file api compatibility, but even if someone would copy older domain.xml, it should work without issues (I did not try it). I don't expect anyone using GlassFish jar files directly except realms and login modules.

@dmatej dmatej added this to the 7.0.19 milestone Oct 12, 2024
@dmatej dmatej self-assigned this Oct 12, 2024
@dmatej dmatej added enhancement New feature or request code cleaning breaking change Changes something users / app devs bug Something isn't working labels Oct 12, 2024
@dmatej dmatej force-pushed the gjule-vs-gc branch 3 times, most recently from 6e9b092 to 4c37d35 Compare October 12, 2024 12:58
@dmatej
Copy link
Contributor Author

dmatej commented Oct 12, 2024

Wow, it took me a while to reproduce it - the reason is that JDK17 and JDK21 resolve Manifest classpath in different way! I have to find where and why it changed ...

EDIT: known issues:

  • ResourceBundle Control is not supported with modules (caused NPE) - fixed locally.
  • GlassFish doesn't support JPMS modules on command line (restarts, reposts, etc) - I am fixing now
  • JDK 11 - 17 doesn't support referring JPMS modules on classpath in manifest. JDK21 does and I am using it most often, so I thought everything is fine. Previous point resolves this, we simply have to stop using manifests to specify classpath.

EDIT2: All resolved. I will also run all TCK tests locally. I noticed that servlet TCK executed one of cli jars instead of using script, so I expect yet some issues to resolve ... until that I will keep it as a draft.

@dmatej
Copy link
Contributor Author

dmatej commented Oct 14, 2024

Ok, another round, seems we don't have any mavenized tests for gfclient, tbd soon ...

Signed-off-by: David Matějček <david.matejcek@omnifish.ee>
Signed-off-by: David Matějček <david.matejcek@omnifish.ee>
- When I remove the jar file from any of those two lines, GF cannot start.

Signed-off-by: David Matějček <david.matejcek@omnifish.ee>
- just 4 jars to start GlassFish are required:
  - glassfish.jar contains non-osgi GlassFishMain class and Launcher interface
  - glassfish-jul-extensions.jar for logging in every phase with same set of
    available features.
  - glassfish-jdk-extensions - basic tools used on all layers: i18n class,
    classloader, we will migrate more in the future.
  - simple-glassfish-api.jar - basic GF apis, no other dependencies.

Signed-off-by: David Matějček <david.matejcek@omnifish.ee>
Signed-off-by: David Matějček <david.matejcek@omnifish.ee>
Signed-off-by: David Matějček <david.matejcek@omnifish.ee>
…sh.jar

Signed-off-by: David Matějček <david.matejcek@omnifish.ee>
- removed Control, unsupported with JPMS
- removed dead code
- added env option to enable logging of all failed lookups

Signed-off-by: David Matějček <david.matejcek@omnifish.ee>
Signed-off-by: David Matějček <david.matejcek@omnifish.ee>
Signed-off-by: David Matějček <david.matejcek@omnifish.ee>
Signed-off-by: David Matějček <david.matejcek@omnifish.ee>
Signed-off-by: David Matějček <david.matejcek@omnifish.ee>
Signed-off-by: David Matějček <david.matejcek@omnifish.ee>
- felix framework is supplied by other jpms modules which don't overlap
  osgi.core
- logging annotations are used just by maven compiler plugin and they are not
  used in runtime.

Signed-off-by: David Matějček <david.matejcek@omnifish.ee>
- AS_INSTALL must be absolute env option despite it can be also autodetected,
  but then it would not be possible to use it in scripts (chicken-egg problem).
- classpaths in manifests sometimes don't work on JDK11-17, not sure why
  exactly.

Signed-off-by: David Matějček <david.matejcek@omnifish.ee>
Signed-off-by: David Matějček <david.matejcek@omnifish.ee>
- If not present, use all jars in the modules directory.

Signed-off-by: David Matějček <david.matejcek@omnifish.ee>
Signed-off-by: David Matějček <david.matejcek@omnifish.ee>
Signed-off-by: David Matějček <david.matejcek@omnifish.ee>
Signed-off-by: David Matějček <david.matejcek@omnifish.ee>
- https://bugs.openjdk.org/browse/JDK-8273473

Signed-off-by: David Matějček <david.matejcek@omnifish.ee>
Signed-off-by: David Matějček <david.matejcek@omnifish.ee>
- Path.of expects a path using slash as a separator, so old windows paths are
  invalid for this. We always have to use File first.

Signed-off-by: David Matějček <david.matejcek@omnifish.ee>
- Globals is not used in local commands and might not be available on classpath

Signed-off-by: David Matějček <david.matejcek@omnifish.ee>
Signed-off-by: David Matějček <david.matejcek@omnifish.ee>
Signed-off-by: David Matějček <david.matejcek@omnifish.ee>
Signed-off-by: David Matějček <david.matejcek@omnifish.ee>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change Changes something users / app devs bug Something isn't working code cleaning enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

JDK loggers can initialize the JDK LogManager before GlassFishLogManager is set
1 participant