gnodet commented on PR #24985: URL: https://github.com/apache/camel/pull/24985#issuecomment-5088640585
_Claude Code on behalf of gnodet_ ## Review feedback addressed in d999aab0c95 All actionable findings from @oscerd and @davsclaus have been addressed: ### 1. Incomplete `disabled` sweep (oscerd, confirmed by davsclaus) Added `volatile` to: - `BaseDelegateProcessorSupport.disabled` — base of `FilterProcessor`, `LoopProcessor`, `CatchProcessor`, `FinallyProcessor`, `DelayProcessorSupport`, etc. - `WrapProcessor.disabled` — installed by policy/transacted reifiers Both are written from JMX via `ManagedProcessor.enable()`/`disable()` and read on the routing hot path at `CamelInternalProcessor:305`. ### 2. `statisticsEnabled` in `DefaultStreamCachingStrategy` (oscerd) Added `volatile` to `UtilizationStatistics.statisticsEnabled` — written from `ManagedStreamCachingStrategy.setStatisticsEnabled`, read on the caching path. ### 3. `ScheduledPollConsumer` inconsistency (oscerd) Added `volatile` to `initialDelay`, `timeUnit`, and `useFixedDelay` for consistency with `delay` which was already volatile. All four are equally JMX-writable via `ManagedScheduledPollConsumer`. ### 4. `setTraceFilter(null)` test (oscerd, confirmed by davsclaus) Added `BacklogTracerFilterClearTest` that verifies the full lifecycle: set filter → trace with filter active → clear filter via `setTraceFilter(null)` → verify messages are traced without filter. Confirms the stale-predicate bug is fixed by the `TraceFilterHolder` pattern. ### 5. `shouldTracePattern` is protected, not private (davsclaus) Acknowledged — the signature change from `shouldTracePattern(NamedNode)` to `shouldTracePattern(NamedNode, String[])` is technically a source/binary break. No known subclasses exist and the class is in `camel-base-engine` (internal), so practical risk is very low. ### Items noted as out-of-scope (follow-up tickets) - `DefaultTracer.traceCounter` should be `AtomicLong` (reverse-direction gap) - `ThrottlingInflightRoutePolicy.setResumePercentOfMax` writer-side atomicity - `DefaultStreamCachingStrategy.updateSpool` has `lock.lock()` instead of `unlock()` (unrelated bug, raised separately by oscerd) -- 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]
