rmannibucau commented on PR #1232: URL: https://github.com/apache/tomee/pull/1232#issuecomment-2208791509
Well there are a lot of things there. If you don't care of the detail just read the last sentence ;). > It is not about the TCK/tests, it is more or less about the requirement to have a context service available to have an managed executor service. Hmm I understand "it is 100% about TCK" - cause requirements are just about them for me and embedded defaults/coded behavior about usability/users. I didn't check recently but when it was added (in 7?) I also added a `Lazy` support for resources but it is way different than this default which disables undesired resources by default (sane default for all cases from embedded daemon to prod ear) and it was still impacting negatively the startup (tests)). So from my perspective - I still support it but use it way less today - it is wrong to keep it by default until bundled (it is ok to set the toggle to something else in the .zip/.tar.gz since people are unlikely to use it if you follow me. > Or would you say, the solution for people needing to configure a custom managed executor service is: configure the toggle and be happy about it? (so actually a doc update, which mention it?) Personally I think we shouldn't enable any default resource but ultimately people willing a tck compatible instance could set it up (this is actually what is done, a ton of flags are needed to respect the spec requirements and nobody cares). Or we just provide a default system.properties in distro which is compliant (people rarely use it I think if they install tomee properly with base/home). So I would enphasize the doc, probably explain a bit more the reasoning and why being explicit there is important and explain that we handle default resources - it is openejb default behavior - as soon as a resource of the same type matches or is known it works (tests of openejb-core use that for ex). Note the name is not important there until you enable exact matching (linking) - another toggle nobody uses probably. The issue specifically with thread pool: there is no default correct thread pool in java. In a pure world where everything is reactive you are either a worker thread (computing intensive) or IO thread (latency friendly but low need) so from there you already can't get a single thread pool or banalised tasks as in EE concurrency. Now in Java we are still with some blocking tasks which are yet another kind of tasks. When you mix it all with a default executor service algorithm - mainly queue based - you can interlock and you can't - as a container - configure it right for any application, so default never works except in rare test cases but it hides the fact you have to configure it for prod (often multiple instances). Side note: tomee keeps a default auto resource creation normally (in `AutoConfig`) from service providers so it should just work smoothly - as tests show. The issue is more an implementation of the factory - wrapper of the actual executor instance - which does a lookup assuming all resources are bound (https://github.com/apache/tomee/blob/main/container/openejb-core/src/main/java/org/apache/openejb/resource/thread/ManagedExecutorServiceImplFactory.java#L74). This one is quite trivial to fix, instead of failling is `context` is empty, null or missing in JNDI, just do a `new ContextServiceImpl` (maybe with a lazy logger info log line to still catch configuration errors with logs - easy to do using from application resources which have an offset). -- 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. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
