[
https://issues.apache.org/jira/browse/CASSANDRA-13135?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15876697#comment-15876697
]
Joel Knighton edited comment on CASSANDRA-13135 at 2/21/17 8:57 PM:
--------------------------------------------------------------------
I think this is definitely worth doing from an organizational/efficiency
standpoint. I'm not sure if the current behavior will greatly increase repair
time; termination of the repair session will shut down the task executor used
by the repair jobs, so the queued repair jobs should fail quickly.
The patches look correct, but I have a few thoughts/questions about the
approach. It seems more error prone than necessary for {{cleanupJobs}} to take
a reference to an executor. To me, it looks like we'll only ever want to clean
up jobs from the executor provided in {{start}} and we could store a reference
to the executor in {{start}}. It also might make more sense to handle this in
{{forceShutdown}} rather than in a listener added to the RepairSession if a
reference to the executor is stored.
There's a small typo in {{ActiveRepairService}} - "cacelled repair jobs" should
be "cancelled repair jobs".
If you'd rather stay with the approach in the attached patches rather than
something closer to my questions/comments above, we should remove the comment
above {{forceShutdown}} saying that it will "clear all RepairJobs". This is
currently incorrect and will remain incorrect if we continue to cleanup the
jobs in a listener attached to the RepairSession.
was (Author: jkni):
I think this is definitely worth doing from an organizational/efficiency
standpoint. I'm not sure if this will greatly increase repair time; termination
of the repair session will shut down the task executor used by the repair jobs,
so the queued repair jobs should fail quickly.
The patches look correct, but I have a few thoughts/questions about the
approach. It seems more error prone than necessary for {{cleanupJobs}} to take
a reference to an executor. To me, it looks like we'll only ever want to clean
up jobs from the executor provided in {{start}} and we could store a reference
to the executor in {{start}}. It also might make more sense to handle this in
{{forceShutdown}} rather than in a listener added to the RepairSession if a
reference to the executor is stored.
There's a small typo in {{ActiveRepairService}} - "cacelled repair jobs" should
be "cancelled repair jobs".
If you'd rather stay with the approach in the attached patches rather than
something closer to my questions/comments above, we should remove the comment
above {{forceShutdown}} saying that it will "clear all RepairJobs". This is
currently incorrect and will remain incorrect if we continue to cleanup the
jobs in a listener attached to the RepairSession.
> Forced termination of repair session leaves repair jobs running
> ---------------------------------------------------------------
>
> Key: CASSANDRA-13135
> URL: https://issues.apache.org/jira/browse/CASSANDRA-13135
> Project: Cassandra
> Issue Type: Bug
> Reporter: Yuki Morishita
> Assignee: Yuki Morishita
> Fix For: 2.2.x, 3.0.x, 3.11.x
>
>
> Forced termination of repair session (by failure detector or jmx) keeps
> repair jobs running that the session created after session is terminated.
> This can cause increase in repair time by those unnecessary works left in
> repair job queue.
--
This message was sent by Atlassian JIRA
(v6.3.15#6346)
