Mailing-List: contact commits-help@cassandra.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@cassandra.apache.org
Date: Fri, 3 Jul 2015 08:47:05 +0000 (UTC)
From: "Sylvain Lebresne (JIRA)" <jira@apache.org>
To: commits@cassandra.apache.org
Message-ID: <JIRA.12720957.1402601714000.92675.1435913225120@Atlassian.JIRA>
In-Reply-To: <JIRA.12720957.1402601714000@Atlassian.JIRA>
References: <JIRA.12720957.1402601714000@Atlassian.JIRA>
 <JIRA.12720957.1402601714399@arcas>
Subject: [jira] [Commented] (CASSANDRA-7392) Abort in-progress queries that
 time out
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit


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

Sylvain Lebresne commented on CASSANDRA-7392:
---------------------------------------------

A least for ranges and single partition slice queries, aborting in {{queryMemtableAndDiskInternal}} won't buy us much: building the iterator won't take much time at all, it's the reading of the iterator that may take time.

So we do at least need the ability to abort the iterator reading, and for that wrapping the result iterator in {{executeLocally}} as you said sounds to me like the best/simplest option.

That leaves single partition names queries, for which the work is indeed done in {{queryMemtableAndDiskInternal}}. For that, I do would avoid adding it as a field of {{ReadCommand}}, as aborting is more a property of the execution than of the command itself. Maybe we could add it to {{ReadOrderGroup}} but rename that class to something more generic (maybe {{ExecutionController}}?), so it doesn't feel out of place, and that could be convenient place to add more stuff in the future.

I'll remark however that for names queries, the proper way to protect for long queries is also to wrap the iterators read inside of {{queryMemtableAndDiskInternal}}. Only checking for aborting at the begining of handling each memtable/sstable (like in the patch you've linked) is probably not fine-grained enough (in the sense that a names query is likely to ony take a long time if lots of "names" are queried, and if that's the case reading a single sstable could take quite some time).

bq. but it would not abort the index reads

It would actually, in the sense that we don't query the index fully upfront, we do it "on-demand" when the main iterator requires more data.

bq. whether the code is stable or whether there is some refactoring still missing that I should wait for

As far as I'm concerned, the only missing refactoring is CASSANDRA-9705, and that will almost surely not affect any of the code you will touch in this ticket, so you're clear :)


> Abort in-progress queries that time out
> ---------------------------------------
>
>                 Key: CASSANDRA-7392
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-7392
>             Project: Cassandra
>          Issue Type: New Feature
>          Components: Core
>            Reporter: Jonathan Ellis
>            Assignee: Stefania
>             Fix For: 3.x
>
>
> Currently we drop queries that time out before we get to them (because node is overloaded) but not queries that time out while being processed.  (Particularly common for index queries on data that shouldn't be indexed.)  Adding the latter and logging when we have to interrupt one gets us a poor man's "slow query log" for free.


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