[
https://issues.apache.org/jira/browse/CASSANDRA-7392?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14735974#comment-14735974
]
Ariel Weisberg edited comment on CASSANDRA-7392 at 9/9/15 1:42 AM:
-------------------------------------------------------------------
This looks good. After initial review I don't have much feedback other than
some minor concurrency/performance nits.
* [I don't think the check is necessary, lazySet is
sufficient|https://github.com/stef1927/cassandra/commit/2a3437ebd64bc479f594c286b9ef1a94ee89d07a#diff-7dfb2e2b64bd70dc52bd6c1b04aa9912R55]
* Don't reset MonitorableThreadLocal, that requires retrieving the threadlocal,
and then storing a value in the AR (which would be cheap if lazy set). Set the
state associated with the task to "done" and let the monitoring thread ignore
it. Ideally you only have to get the threadlocal once per request.
* When doing state transitions on the monitoring ref I don't think you need
CAS. You can read and then lazySet and there is a small chance that the logging
will think something was dropped when it wasn't. The primary functionality of
shedding still works and that is always an approximation.
* MonitorableImpl could extend AtomicReference instead of composing
MonitoringState. This would leak the methods into the derived classes. I'm torn
as to whether that pollution is worth it.
Tomorrow I'll review it again and look at the tests and coverage.
was (Author: aweisberg):
This looks good. After initial review I don't have much feedback other than
some minor concurrency/performance nits.
* [I don't think the check is necessary, lazySet is
sufficient|https://github.com/stef1927/cassandra/commit/2a3437ebd64bc479f594c286b9ef1a94ee89d07a#diff-7dfb2e2b64bd70dc52bd6c1b04aa9912R55]
* Don't reset MonitorableThreadLocal, that requires retrieving the threadlocal,
and then storing a value in the AR (which would be cheap if lazy set). Set the
state associated with the task to "done" and let the monitoring thread ignore
it. Ideally you only have to get the threadlocal once per request.
* When doing state transitions on the monitoring ref I don't think you need
CAS. You can read and then lazySet and there is a small chance that the logging
will think something was dropped when it wasn't. The primary functionality of
shedding still works and that is always an approximation.
* MonitorableImpl could extend AtomicReference instead of composing
MonitoringState. This would leak the methods into the derived classes. I'm torn
as to whether that pollution is worth it.
> 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
> Priority: Critical
> 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)
