allthingssecurity commented on PR #28382:
URL: https://github.com/apache/camel/pull/28382#issuecomment-6395118298
@davsclaus thanks, all three points are in 13c1c62d670c.
1. `TotalRequestsThrottler.processAsynchronously` (`:196`) stores
`TimeSource.currentTimeMillis()` in `CamelThrottlerExchangeQueuedTimestamp`.
`git grep` shows that only `process()` (`:84`) reads that property, so both
"Queued for {}ms" lines now subtract milliseconds from milliseconds.
2. `ConcurrentRequestsThrottler.doThrottle` converts `elapsed` and
`queuedTime` with `Duration.ofNanos(...).toMillis()`, as `:137` already did.
3. I went with option 1, the docs only. Your text is in `test-dst.adoc` and
the catalog copy (`cmp` says they are identical), and the semaphore is gone
from the `alsoRewrite` sentence at `:102`. I tried option 2 first, and it did
not work cleanly:
- With `((Semaphore) semaphore).acquire()` and the class in
`alsoRewrite`, the first exchange failed with a `StackOverflowError`. The
rewrite covers the whole class, `WrappedSemaphore` included.
`WrappedSemaphore.tryAcquire()` calls `super.tryAcquire(0L,
TimeUnit.NANOSECONDS)`. The instrumentation turns that `invokespecial` into
`DstWaits.tryAcquire`, which ends with a virtual `semaphore.tryAcquire()` and
so calls the override again.
- With super calls left alone, the `acquire` wait was virtual, but in a
`throttle(1).concurrentRequestsMode()` route with an asynchronous step after
the throttle, the seed still failed: "no task of the run can end the wait". The
waiting exchange runs inside the pipeline's reactive worker on the loop thread.
The continuation of the exchange that holds the permit gets queued on that same
worker, so it cannot run while the wait is on the stack (the one-stack model).
The hint would only hold for unusual route shapes, so I did not make that claim
in the doc.
The super-call rewrite in `DstInstrumentation` only matters when someone
puts a `Semaphore` subclass like this one in `alsoRewrite`. I left it out of
this PR to keep it focused, and the doc now tells users not to do that.
Points 1 and 2 only change TRACE output, so there is no new test and no
upgrade-guide entry. Tests on this head: camel-core `*Throttl*` (67) and
camel-management `*Throttl*` (12), `TimeSourceTest` (8) and camel-test-dst
(57), all pass. I built the core modules of this branch in the same reactor.
The draft follow-up JIRA for the TRACE mix is dropped, and I answered in the
gnodet-bot thread.
_Claude Code on behalf of allthingssecurity_
--
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]
