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 39 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.

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

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>

# Conflicts:
#	nucleus/core/kernel/src/main/java/com/sun/enterprise/v3/server/CommonClassLoaderServiceImpl.java
Signed-off-by: David Matějček <david.matejcek@omnifish.ee>
- useful when you use exports rather than editing asenv.conf

Signed-off-by: David Matějček <david.matejcek@omnifish.ee>
Signed-off-by: David Matějček <david.matejcek@omnifish.ee>
- Caused parallel classloader architecture to osgi, so ie. default-web.xml
  filter for JSPC was worthless, dependencies were always found somehow.
- However it is still useful for jar files with Main executed from command line.

Signed-off-by: David Matějček <david.matejcek@omnifish.ee>
- Stop swallowing exceptions
- Log stacktraces if AS_TRACE or AS_DEBUG enabled
- To get very verbose logs for asadmin:
  - export AS_ADMIN_LOG_FORMATTER=org.glassfish.main.jul.formatter.OneLineFormatter
  - export AS_TRACE=true

Signed-off-by: David Matějček <david.matejcek@omnifish.ee>
Signed-off-by: David Matějček <david.matejcek@omnifish.ee>
- Removed MaxPermSize - not supported on JDK11
- Removed -server and -client; on current JVMs -server is always active
- Added log manager and blocking handler for waiting on instance's
  logging.properties

Signed-off-by: David Matějček <david.matejcek@omnifish.ee>
- Separated bootstrap aka glassfish.jar and glassfish-osgi-bootstrap
- Separated common-util (osgi enabled) and glassfish-jdk-extensions (usual jar)
- Result: We have nonosgi part usable for standard Java classpath and another
  osgi-enabled group of jar files loaded dynamically. It is much easier to
  understand and manage all dependencies.
- Result2: We can enable logging much sooner than before. Maybe even without
  blocking (not implemented yet, to be invented later).
- This commit will not build, I am trying to separate the work to parts.

Signed-off-by: David Matějček <david.matejcek@omnifish.ee>
- For CLI I tried to eliminate what is not required.
- It should be refactored later, so CLI would have much cleaner dependency tree

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>
- 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>
- 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>
@dmatej dmatej force-pushed the gjule-vs-gc branch 13 times, most recently from d1eae53 to 369b5ef Compare November 9, 2024 20:53
- 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>
@dmatej dmatej force-pushed the gjule-vs-gc branch 2 times, most recently from e270372 to 1b4d776 Compare November 9, 2024 23:36
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>
@dmatej dmatej force-pushed the gjule-vs-gc branch 5 times, most recently from 8bdc617 to ce367c2 Compare November 17, 2024 01:10
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