Skip to content

FISH-10049 FISH-10286 FISH-10615 Restores EAR fixes and WARlib performance enhancements#7314

Merged
Pandrex247 merged 3 commits intopayara:mainfrom
flowlogix:test-fix-deployment
May 19, 2025
Merged

FISH-10049 FISH-10286 FISH-10615 Restores EAR fixes and WARlib performance enhancements#7314
Pandrex247 merged 3 commits intopayara:mainfrom
flowlogix:test-fix-deployment

Conversation

@lprimak
Copy link
Contributor

@lprimak lprimak commented Apr 25, 2025

Restores functionality reverted from #7032 and #7097 and applies fixes from #7212

Fixes #7201
Fixes #7214
Fixes #7215
Fixes #7233

Supersedes #7212

@lprimak
Copy link
Contributor Author

lprimak commented Apr 25, 2025

@Pandrex247 can you please run the full TCK on this branch? It'll probably supersede #7212 because the other PR is become stale.

@lprimak
Copy link
Contributor Author

lprimak commented Apr 25, 2025

Argh. Test are failing b/c eclipse foundation servers are down including MP repositories. See https://www.eclipsestatus.io

@lprimak
Copy link
Contributor Author

lprimak commented Apr 29, 2025

@Pandrex247 Can you please run the full TCK on this branch (even if you ran it before) as I committed some more changes here.
Thank you

@lprimak
Copy link
Contributor Author

lprimak commented May 1, 2025

@Pandrex247 Please run all TCKs on this branch. This should fix the Web Profile TCK

@lprimak lprimak force-pushed the test-fix-deployment branch from 02abbb7 to 5d35133 Compare May 1, 2025 19:40
@lprimak
Copy link
Contributor Author

lprimak commented May 1, 2025

Just FYI- I am mucking with the commit messages via force-push, the code remains the same

@lprimak lprimak force-pushed the test-fix-deployment branch from 5d35133 to 0df8c90 Compare May 1, 2025 19:51
@lprimak lprimak changed the title Test fix deployment FISH-10049 FISH-10286 FISH-10615 Restores EAR fixes and WARlib performance enhancements May 1, 2025
@lprimak lprimak force-pushed the test-fix-deployment branch from 0df8c90 to af476ab Compare May 1, 2025 20:01
@Pandrex247
Copy link
Member

The Platform TCK is still running, but the Web Profile TCK has passed 🥳

@lprimak
Copy link
Contributor Author

lprimak commented May 3, 2025

Any updates? :) :)

@lprimak lprimak force-pushed the test-fix-deployment branch from af476ab to 67c503c Compare May 5, 2025 01:29
@lprimak
Copy link
Contributor Author

lprimak commented May 5, 2025

if everything succeeded, can you please kick off full TCK again just to make sure I haven't messed anything up?
Thank you.🙏

@Pandrex247
Copy link
Member

It passed 🥳
I will kick off another run for your new changes

@lprimak
Copy link
Contributor Author

lprimak commented May 5, 2025

Last changes just remove unused code from the PR. No need to run the TCK. Ready to merge.

…I-enabled library JARs (payara#7032)

- correctly copy BDA sets for each war in EAR
- Make WAR's CDI beans available in EAR-libs
- read web-fragment.xml from EAR-libs
- processing ear-lib manifest
- de-duplicate BDAs in CDI processing by using LinkedHashSet intead of ArrayList
- made some structures final (cleanup)
- fixed ear and concurrent classloader leaks, including refactored reflection caching

enh: loading JARs from <domain_dir>/lib/warlibs if --properies warlibs=true is specified (payara#7097)

bugfix: EAR deployment bugs found (payara#7212)
- fixed current deployment context - made it a regular thread local, not queue
- actually destroy deployment ClassLoader shareableTemp correctly
- added fish.payara.ear-combined-cdi-deployment property to force combined EAR CDI deployment
- Jax-JS Json-B injection retrives the correct BeanManager based on the actual type of the injection point desired
@lprimak lprimak force-pushed the test-fix-deployment branch from bf19335 to 5463633 Compare May 6, 2025 06:29
@mkarg
Copy link
Contributor

mkarg commented May 7, 2025

@lprimak Is this new PR still compatible with the one I last checked with our real-world EAR? If not, looking at the horrible history of the last PR, we might should wait with the merge for another test drive? 🤔

@lprimak
Copy link
Contributor Author

lprimak commented May 7, 2025

No worries. This PR is a rebase to main plus some unused code cleanup and review suggestions. It contains all our fixes and was tested against all our reproducers.
Feel free to test against Quipsy of course but I feel confident that everything is as it should be.

@mkarg
Copy link
Contributor

mkarg commented May 7, 2025

Thank you, Lenny! Ok, in this case I see no need for more pre-release checks and will test just the final CE 6.2022.5. 😅

@mkarg
Copy link
Contributor

mkarg commented May 9, 2025

@Pandrex247 Will this PR be contained in Payara CE 6.2025.5?

@lprimak
Copy link
Contributor Author

lprimak commented May 9, 2025

My guess is probably not

@lprimak
Copy link
Contributor Author

lprimak commented May 13, 2025

Don't forget about me :)

@Pandrex247
Copy link
Member

@mkarg No this won't be included in 6.2025.5.
@lprimak I am currently deep into a major rework of the codebase (almost finished I hope) - reviewing and getting this merged (along with documenting the new configuration options) is next on my to do list

@lprimak
Copy link
Contributor Author

lprimak commented May 13, 2025

Thank you

@mkarg
Copy link
Contributor

mkarg commented May 14, 2025

@Pandrex247 Will this PR be contained in Payara CE 6.2025.5?

@mkarg No this won't be included in 6.2025.5. @lprimak I am currently deep into a major rework of the codebase (almost finished I hope) - reviewing and getting this merged (along with documenting the new configuration options) is next on my to do list

"Deep rewrite" sounds like it makes sense to let me run a test drive with that before publishing?

@Pandrex247
Copy link
Member

@mkarg Nah it should be harmless - changing maven group IDs and build config around.

@Pandrex247
Copy link
Member

@lprimak
Not to throw a wrench into things, but can you explain to me how warlibs is different to applibs?
I'm trying to write some documentation for you and struggling to understand the difference.

Is it just that when --warlibs is enabled all libraries stored under domain_dir/lib/warlibs will be loaded, whereas applibs have to be added explicitly one by one (--libraries wibbleLib,wobbleLib)?
Is your new option essentially doing --libraries *?
Or is it doing something different?

See payara/Payara-Documentation#598 & flowlogix#6

@Pandrex247
Copy link
Member

Pandrex247 commented May 16, 2025

If --properties warlibs=true is functionally identical to --libraries * it might be worth a follow-up iteration (can be done in a separate PR, I don't want to hold this one up in the name of perfection) to just have --libraries * be an acceptable option over adding a separate domain_dir/lib/warlibs directory.

My mind is currently circling around the following:

  • If the behaviour is as I described, --libraries * seems to be a more "intuitive" way of achieving this over adding a completely separate directory and deployment option
  • I want to do some testing on if this extra directory is actually synchronised correctly: to-resynchronize-library-files

@Pandrex247
Copy link
Member

Reading through #6405 again I can see that you already mentioned including libs in applibs doesn't seem to get them scanned by CDI - so I guess my root question is: why the new folder? Why not just fix/add CDI scanning to applibs? Was it to have some sort of differentiation?

@lprimak
Copy link
Contributor Author

lprimak commented May 17, 2025

Trust me, it was my first thought to "fix" applibs. However this turned out to more complicated and I didn't want to break existing functionality.

The main point behind warlibs is to improve deployment times for developers, and to get instant feedback on code modifications.
It allows to make skinny WARs for development and shift all dependencies into payara domain so they only get loaded (including annotation processing and all initialization) once.

Here is the difference breakdown:

  • adding --applibs x,y,z isn't enough. The libraries also need to be listed in MANIFEST.MF of the "cient"
  • applibs are not cached, as warlibs are, i.e. if you replace lib in applib/ and redeply it'll get updated, warlibs won't.
  • warlibs get annotation and extension scanning and processing, including for CDI, EJB, Servlet and such. applibs don't.

I am not sure where / how applibs are used but I am sure that I couldn't make it work without breaking compatibility.

Regarding "resync", since warlibs feature is for development, I don't think that would apply.

@Pandrex247
Copy link
Member

Thanks for the extra info, I'll look into adding that to the docs in some way.

Regarding "resync", since warlibs feature is for development, I don't think that would apply.

I tested the resync - it appears to work.
I was worried that the extra lib directory wouldn't be synced across to a remote instance (e.g. if the sync of files to the remote instance wasn't just "lib/*"), but they seem to be synced just fine 👌

Pandrex247
Pandrex247 previously approved these changes May 19, 2025
@Pandrex247 Pandrex247 dismissed their stale review May 19, 2025 08:15

Accidental, final tests pending

@Pandrex247 Pandrex247 self-requested a review May 19, 2025 08:16
Copy link
Member

@Pandrex247 Pandrex247 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested the https://github.com/flowlogix/ear-sample and https://github.com/flowlogix/cdi-scanning reproducers
Also tested that the extra warlibs directory get synced to remote instances
All TCKs appear green (can't see any false positives this time)
Tested that Web Profile and Micro don't hang on startup

Thank you very much 🥳

Some follow-up things to do that can be done Later™:

  • Admin console support - needs a new check box or similar on the deploy page
  • add-library command support - add a war option and associated support for it (and update the command's help text)

@Pandrex247 Pandrex247 merged commit 3e5a8c5 into payara:main May 19, 2025
1 check passed
@lprimak lprimak deleted the test-fix-deployment branch May 19, 2025 15:11
@lprimak
Copy link
Contributor Author

lprimak commented May 19, 2025

Well, this was the biggest scope creep in "my" history.
What was supposed to be a 3-day project turned into 9-month, close-to-1000hrs of work for me, not to mention
Markus, Andy, and like 10 other devs who got touched by this.

And still not done.
This tranche culminates in another "3-day" project: #7165

@lprimak
Copy link
Contributor Author

lprimak commented May 19, 2025

Also time to introduced the ecosystem enhancement described here: payara/ecosystem-support#98 to make use of this

@Pandrex247 Pandrex247 changed the title FISH-10049 FISH-10286 FISH-10615 Restores EAR fixes and WARlib performance enhancements FISH-10049 FISH-10286 FISH-10615 FISH-11137 Port EAR Enhancements and War Libs May 26, 2025
@Pandrex247 Pandrex247 changed the title FISH-10049 FISH-10286 FISH-10615 FISH-11137 Port EAR Enhancements and War Libs FISH-10049 FISH-10286 FISH-10615 FISH-11137 Restores EAR fixes and WARlib performance enhancements May 26, 2025
@Pandrex247 Pandrex247 changed the title FISH-10049 FISH-10286 FISH-10615 FISH-11137 Restores EAR fixes and WARlib performance enhancements FISH-10049 FISH-10286 FISH-10615 Restores EAR fixes and WARlib performance enhancements May 26, 2025
Pandrex247 added a commit to Pandrex247/Payara that referenced this pull request Jun 2, 2025
FISH-10049 FISH-10286 FISH-10615 Restores EAR fixes and WARlib performance enhancements
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

3 participants