allthingssecurity opened a new pull request, #27465: URL: https://github.com/apache/camel/pull/27465
# Description [CAMEL-25073](https://issues.apache.org/jira/browse/CAMEL-25073), item 4 @davsclaus listed this after his deep review of #26959 (CAMEL-25065): thread pools created by the same source get the same object name. For example, an aggregate with `completionTimeout` and `optimisticLocking` creates two pools from its `AggregateProcessor` (the timeout checker and the optimistic locking executor), and a recoverable repository adds a third (the recover checker). Only `threadpools,name="AggregateProcessor(0x...)"` was registered. The other pools were skipped as "already managed" and were not visible in JMX. The ticket has four options. This PR implements the recommended one, (a). The alternatives are listed below; if you prefer one of them, I'll change the PR. Cause: `BaseExecutorServiceManager.onThreadPoolCreated` builds the thread pool id from the source only. For a source that is not a `NamedNode`, a `String` or a `StaticService`, that is the simple class name plus the identity hash, and the source id is `null`. The name of the pool was not used, so all pools of one source got the same object name. Change, in `camel-base-engine`: for such a source the name of the thread pool (as passed to the `ExecutorServiceManager`, sanitized as before) is now the source id. `DefaultManagementObjectNameStrategy` already appends the source id to the MBean name, so each pool gets its own MBean, such as `AggregateProcessor(0x1b2c3d4e)(AggregateTimeoutChecker)`. Its `SourceId` attribute is `AggregateTimeoutChecker`. The `Id` attribute does not change. The javadoc of `LifecycleStrategy.onThreadPoolAdd` (`camel-api`, javadoc only) says what the source id is in that case. Which names change: only the MBean names that contain an identity hash today. A monitoring tool cannot query those by a fixed name anyway, since the hash differs on every run. These names keep their form: - thread pools of EIPs (`NamedNode` sources, such as `threads1(threads)`); - pools created with a `String` source; - pools without a source; - pools of static services such as `DefaultShutdownStrategy` or `DefaultSupervisingRouteController`, whose id is the class name. The upgrade guide has a paragraph under `=== camel-management` about the `SourceId` attribute and the MBean name. Options on the ticket: - (a) The pool name as the source id, for a source that is neither a node nor a String (this PR). This PR also leaves out static services, so their stable names do not change. If you want the same rule for them (a static service that ever creates two pools would collapse in the same way; none in `core` does today), it is a one-line change. - (b) Put the pool name into the id (`AggregateTimeoutChecker(AggregateProcessor(0x...))`). This changes the `Id` attribute too. - (c) Make the name unique on a clash in `JmxManagementLifecycleStrategy` (append a counter). The names would depend on the start order, and the cause would be hidden. - (d) Only document the limitation. Tests: new `ManagedAggregateThreadPoolsTest` (aggregate with `completionTimeout` and `optimisticLocking`). It checks: - both pools are registered, with the pool name as `SourceId` and in the MBean name; - the same after a stop and start of the route; - both are unregistered when the route is removed. It fails on main in two runs: `The timeout checker and the optimistic locking pools should be registered: [...name="AggregateProcessor(0x7db77e7d)"] ==> expected: <2> but was: <1>`. The existing thread pool tests (`ManagedThreadPool*`, `DualManagedThreadPool*`, `ManagedVirtualThreadExecutorTest`, `ManagedRemoveRouteAggregateThreadPoolTest`) still pass unchanged. Whole `camel-management` module: 532 tests, 0 failures, 1 skipped. `camel-core` (with `camel-base-engine` in the build): 8087 tests, 0 failures, 45 skipped. Related CAMEL-25073 PRs, each for one item and based on `main`: item 1 (`camel-management-context-name-object-name`), item 2 (`camel-management-onexception-shared-mbean`), item 3 (`camel-management-masked-endpoint-unregister`) and item 5 (`camel-management-contextonly-components`). All five merge cleanly with `main` and with each other in every pair (`git merge-tree`). Items 1, 2, 3 and 5 change different methods of `JmxManagementLifecycleStrategy`; items 1, 4 and 5 add separate paragraphs under `=== camel-management` in the upgrade guide. With all five merged together, the `camel-management` module passes (553 tests, 0 failures, 1 skipped). None depends on another. # Target - [x] I checked that the commit is targeting the correct branch (Camel 4 uses the `main` branch) # Tracking - [x] If this is a large change, bug fix, or code improvement, I checked there is a [JIRA issue](https://issues.apache.org/jira/browse/CAMEL) filed for the change (usually before you start working on it). # Apache Camel coding standards and style - [x] I checked that each commit in the pull request has a meaningful subject line and body. - [ ] I have run `mvn clean install -DskipTests` locally from root folder and I have committed all auto-generated changes. (I built and tested `core/camel-api`, `core/camel-base-engine` and `core/camel-management`, including the formatter and import-sort plugins. No generated files change. I did not run the full root build.) # AI-assisted contributions - [x] If this PR includes AI-generated code, commits have proper co-authorship attribution (e.g., `Co-authored-by` trailers) and the PR description identifies the AI tool used. This PR was prepared with Claude Code (Claude Opus 5.5). The commit carries a `Co-Authored-By` trailer. _Claude Code on behalf of allthingssecurity_ 🤖 Generated with [Claude Code](https://claude.com/claude-code) -- 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]
