[ 
https://issues.apache.org/jira/browse/CASSANDRA-11218?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15637696#comment-15637696
 ] 

Sam Tunnicliffe commented on CASSANDRA-11218:
---------------------------------------------

I took a step back to re-evaulate exactly what is prioritized and when and it 
turns out that adding priorities to {{CompactionTask}} itself is redundant, and 
actually, in every case it's ignored. Really, it's the 
{{Runnable/WrappedRunnable/Callable}} being submitted to {{CompactionExecutor}} 
that needs to be prioritized and even in cases where these wrap a 
{{CompactionTask}}, these have their own priority explicitly set and do not 
usually inherit it from the task. That is to say, that it's the context of the 
submission which determines the priority, rather than the task itself. Having 
{{CompactionTask}} implement {{Prioritized}} actually makes some stuff pretty 
hard to follow.

Actually, it turns out that this is somewhat academic, as most of the 
priorities are not actually being observed anyway, due to the fact that 
{{CompactionExecutor::submitIfRunning}} wraps everything in a 
{{ListenableFutureTask}} before submission. This effectively hides any priority 
that might be set so {{newTask}} always ends up creating another wrapper layer 
with default priorities. 

I've pushed a fix for this to my branch, and also refactored so that 
{{CompactionTask}} no longer implements {{Prioritized}}, which I think 
simplifies things quite a bit. 

Another thing that was bugging me was that the relationship between 
{{OperationType}} and {{Priority}} is somewhat muddy, as for some operations 
prioritization is a meaningless concept (as indicated by the multiple special 
cases). It feels like we're overloading the concepts somewhat which, for me, 
again makes it much harder to grok. So I propose removing all the priority 
stuff from {{OperationType}} and adding a new enum solely to represent the task 
priorities (also done in my branch).

bq. I've removed the remaining uses of subtype prioritization...I've left the 
logic in place for later

I think we should just rip it out if it's not being used. Like you say, if we 
do come up with some good way to prioritise within types later, then we can 
easily add it back, so leaving half of it hanging around in the meantime is 
only going to be confusing.

The other thing that's obviously missing here is tests, 
{{CompactionPriorityTest}} alone clearly isn't sufficient, seeing as it didn't 
catch the fact that no prioritization was actually being done :) 


> Prioritize Secondary Index rebuild
> ----------------------------------
>
>                 Key: CASSANDRA-11218
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-11218
>             Project: Cassandra
>          Issue Type: Improvement
>          Components: Compaction
>            Reporter: sankalp kohli
>            Assignee: Jeff Jirsa
>            Priority: Minor
>
> We have seen that secondary index rebuild get stuck behind other compaction 
> during a bootstrap and other operations. This causes things to not finish. We 
> should prioritize index rebuild via a separate thread pool or using a 
> priority queue.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Reply via email to