etzel-mes opened a new pull request, #28049: URL: https://github.com/apache/camel/pull/28049
# Description [CAMEL-25713](https://issues.apache.org/jira/browse/CAMEL-25713). A `milo-client` or `milo-browse` connection handled the outcome of its write, method call, read and browse requests, and ran its reconnect job, on `Stack.sharedScheduledExecutor()`, the scheduled executor that Milo shares between all its clients. When a request fails, `SubscriptionManager.handleConnectionFailure` holds the lock of the connection and calls `OpcUaClient.disconnect()`. A CloseSession that is sent after the client noticed the lost connection waits until a reconnect attempt restores the channel or fails, and Milo schedules those attempts on that same executor. That executor has as many threads as the JVM has processors, so with one processor its only thread waits for a task that only it could run. `disconnect()` waits without a timeout, and the CloseSession starts its request timeout only once it has a channel, so nothing ends the wait: the connection is not rebuilt, its consumers stay silent after the server can be reached again, and every call that needs the lock of the connection waits. OPC UA clients often run on small gateways or in containers and pods, and a CPU limit of one or less gives the JVM one processor. For the issue, against Microsoft opc-plc 2.15.4 with the client limited to one CPU, a producer writing every 2 s and the traffic to the server dropped for 30 s, main (a16cccbde62) did not rebuild the connection within 60 s in 3 of 11 runs, each time with `milo-shared-scheduled-executor-0` waiting in `OpcUaClient.disconnect()`. The change: - Each `MiloClientConnection` now has a scheduled thread pool of its own, created by the `ExecutorServiceManager` of the `CamelContext` and shut down when the connection is closed. `MiloClientEndpoint` and `MiloBrowseEndpoint` give the connection their `CamelContext`. Milo still schedules its reconnect attempts on its shared scheduled executor, which a disconnect no longer blocks. - Without virtual threads, the pool has as many threads as Milo's shared scheduled executor, one per processor: like that executor, it starts a thread for each task until it has them all. They end when the connection is closed. The pool belongs to the connection, so that an outage of one server, whose disconnect and connect attempts each hold a thread, does not delay the requests and reconnects of the other connections, nor the keep-alive and reconnect timers that Milo runs on its shared scheduled executor. - Its queue is not bounded, like the queue of Milo's executor, because a rejected reconnect would end the reconnects of the connection. The rejected policy is `Abort`: a request that completes after the connection is closed fails with a `RejectedExecutionException`. With the default `CallerRuns`, a pool that is shut down drops the task, and the request would never complete. - `close()` takes the lock that `init()` runs under, and shuts the pool down even if disposing the connection fails. - `SubscriptionManager` is unchanged apart from one sentence of a Javadoc that said the two executors were the same. - The upgrade guide describes the new threads. - New `MiloClientExecutorTest`: an awaited write completes while every thread of Milo's shared scheduled executor is busy, and the threads of the pool end when the last route that uses the connection stops. Without the change both tests fail: the write times out, and no thread of the pool exists. The first test shows that the outcome of a request no longer needs a thread of Milo's executor; it does not lose the connection. With one processor, or with virtual threads enabled (the `ExecutorServiceManager` then builds a scheduled pool with one worker), the pool has a single thread. A route that runs on it, after an awaited request or for the status of a monitored item that a reconnect could not create, must then not wait for another request of the same connection, for example through a `ProducerTemplate`. With one processor, Milo's shared scheduled executor had the same limit. Calling `disconnectAsync()` in `handleConnectionFailure` would avoid this one wait, but the reconnect job blocks in `connect()` until the server answers or the connect times out, and on Milo's shared scheduled executor that delays the timers of every Milo client in the JVM. The issue mentions that handling the failures on another executor could need a guard against a stale failure starting a second reconnect. The pool never has more threads than Milo's shared scheduled executor, so it adds no such race: with one thread, failures that complete during the disconnect run before the queued reconnect and find it already scheduled; with more threads, the race is the one that main has. A guard needs the failing connection in the four producer blocks that #27956 changes, so it fits better after that PR. CAMEL-25692 proposes an executor owned by the component for the subscription recovery that #27910 added; this pool could serve as that executor. Against the same server, main (e5ae19c3361) and this change ran side by side, each with a consumer and an awaited producer on one connection, and each with its traffic dropped for 30 s. With half a CPU per client (the JVM still sees one processor), main did not rebuild the connection within 60 s in 4 of 12 runs, each time with `milo-shared-scheduled-executor-0` waiting in `OpcUaClient.disconnect()` from `handleConnectionFailure` of a write; with the change, the connection was rebuilt in all 12. With one CPU per client, both rebuilt it in all 18 runs: the hang is a race, and there the CloseSession of main always failed at once. In 6 of the 30 runs the CloseSession of the change waited about 6 s, until a reconnect attempt of the channel failed while the traffic was still dropped; on main that wait does not end. This PR does not change the lines that #27956 (CAMEL-25694, @gnodet) and #28045 (CAMEL-25690) change. It merges with each of them without conflicts in either order, and all three merge together; on top of #27956, the new test and the tests of #27956 pass. `mvn -Psourcecheck clean verify` in `components/camel-milo` passes (81 tests, 1 skipped as on main), and the module build regenerates no files. The module tests passed three more times with `-Dsurefire.rerunFailingTestsCount=0`, once with `-XX:ActiveProcessorCount=1`. @davsclaus @oscerd could you take a look? # 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. # 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. _Claude Code on behalf of etzel-mes_ -- 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]
