[
https://issues.apache.org/jira/browse/CAMEL-24626?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Henrik updated CAMEL-24626:
---------------------------
Description:
Three issues found while fixing CAMEL-24583. None of them is introduced or
fixed there.
h3. 1. A cancelled {{BackgroundTask}} stays in the {{TaskManagerRegistry}}
A task scheduled through {{BackgroundTask.schedule()}} adds itself to the
{{TaskManagerRegistry}} from its first run, and only a run of the task removes
it again. A
caller that cancels the {{Future}} returned by {{schedule()}} therefore leaves
the entry
behind for the life of the {{CamelContext}}: the task keeps showing up as an
internal task,
and it keeps its container reachable.
Affects {{camel-sjms}} ({{SimpleMessageListenerContainer.doStop}}) today, and
{{camel-master}} ({{MasterConsumer}}) since CAMEL-24583.
*Proposed fix:* a {{cancel()}} operation on {{BackgroundTask}} that unschedules
the task and
deregisters it, used by both call sites.
h3. 2. Lock inversion between the consumer and the cluster view in camel-master
{{MasterConsumer}} guards its leadership state with the {{BaseService}} lock:
* {{doStop}} holds that lock and then needs the write lock of the view, through
{{CamelClusterView.removeEventListener}}.
* {{AbstractCamelClusterView}} dispatches events while holding its own read
lock, and the
listener of the consumer then needs the {{BaseService}} lock.
An unlocked {{isRunAllowed()}} fast path in the listener covers the common
case, but a
leadership event that passes that check just before a stop acquires the lock
closes the two
orders into a deadlock.
*Proposed fix:* guard the leadership state with a lock of its own, and never
hold it across a
call into the view.
h3. 3. Exhausted start attempts are not documented
{{backOffMaxAttempts}} defaults to 10 attempts, {{backOffDelay}} apart (5000
millis). A node
that uses up its attempts keeps the leadership and consumes nothing until the
leadership
changes again. That is the documented intent of the option, and
{{backOffMaxAttempts=0}}
already retries for as long as the node is the leader, but neither the
consequence nor the
escape hatch is written down anywhere.
*Proposed fix:* document both in the component documentation.
was:
Three issues found while fixing CAMEL-24583, none of them introduced or fixed
there.
1. `BackgroundTask` scheduled through `schedule()` stays in the
`TaskManagerRegistry` when the caller cancels the returned future. Only a run
of the task removes the entry, and once the schedule is cancelled no run is
coming, so the task is listed as an internal task and keeps its container
reachable for the life of the context. This affects camel-sjms today and
camel-master since CAMEL-24583.
2. `MasterConsumer` guards its leadership state with the `BaseService` lock.
`doStop` holds that lock and needs the write lock of the cluster view to remove
its listener, while `AbstractCamelClusterView` dispatches events holding its
read lock and then needs the consumer lock. A leadership event that passes the
`isRunAllowed` fast path just before a stop acquires the lock closes the two
orders into a deadlock.
3. `backOffMaxAttempts` defaults to 10 attempts at 5s apart. A leader that uses
them up keeps the leadership and consumes nothing until the leadership changes
again. That is the documented intent, and `backOffMaxAttempts=0` already
retries indefinitely, but neither the consequence nor the escape hatch is
documented.
> camel-master: follow-ups to CAMEL-24583 - leadership lock, cancelled task
> registry entries, backoff documentation
> -----------------------------------------------------------------------------------------------------------------
>
> Key: CAMEL-24626
> URL: https://issues.apache.org/jira/browse/CAMEL-24626
> Project: Camel
> Issue Type: Bug
> Components: camel-master, camel-sjms
> Affects Versions: 4.22.0
> Reporter: Henrik
> Priority: Major
>
> Three issues found while fixing CAMEL-24583. None of them is introduced or
> fixed there.
> h3. 1. A cancelled {{BackgroundTask}} stays in the {{TaskManagerRegistry}}
> A task scheduled through {{BackgroundTask.schedule()}} adds itself to the
> {{TaskManagerRegistry}} from its first run, and only a run of the task
> removes it again. A
> caller that cancels the {{Future}} returned by {{schedule()}} therefore
> leaves the entry
> behind for the life of the {{CamelContext}}: the task keeps showing up as an
> internal task,
> and it keeps its container reachable.
> Affects {{camel-sjms}} ({{SimpleMessageListenerContainer.doStop}}) today, and
> {{camel-master}} ({{MasterConsumer}}) since CAMEL-24583.
> *Proposed fix:* a {{cancel()}} operation on {{BackgroundTask}} that
> unschedules the task and
> deregisters it, used by both call sites.
> h3. 2. Lock inversion between the consumer and the cluster view in
> camel-master
> {{MasterConsumer}} guards its leadership state with the {{BaseService}} lock:
> * {{doStop}} holds that lock and then needs the write lock of the view,
> through
> {{CamelClusterView.removeEventListener}}.
> * {{AbstractCamelClusterView}} dispatches events while holding its own read
> lock, and the
> listener of the consumer then needs the {{BaseService}} lock.
> An unlocked {{isRunAllowed()}} fast path in the listener covers the common
> case, but a
> leadership event that passes that check just before a stop acquires the lock
> closes the two
> orders into a deadlock.
> *Proposed fix:* guard the leadership state with a lock of its own, and never
> hold it across a
> call into the view.
> h3. 3. Exhausted start attempts are not documented
> {{backOffMaxAttempts}} defaults to 10 attempts, {{backOffDelay}} apart (5000
> millis). A node
> that uses up its attempts keeps the leadership and consumes nothing until the
> leadership
> changes again. That is the documented intent of the option, and
> {{backOffMaxAttempts=0}}
> already retries for as long as the node is the leader, but neither the
> consequence nor the
> escape hatch is written down anywhere.
> *Proposed fix:* document both in the component documentation.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
