cassandra-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Aleksey Yeschenko (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (CASSANDRA-10825) OverloadedException is untested
Date Tue, 03 Jan 2017 19:48:58 GMT

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

Aleksey Yeschenko commented on CASSANDRA-10825:
-----------------------------------------------

Ok, this is deeply stupid and also embarrassing.

The reason it never gets updated is that {{hintsInProgress}} isn't a cache, it's an instance
of {{CacheLoader}}. Meaning calls to {{load()}} always return the initial value. Very, very
stupid. Worse, it got committed by me 3 years ago, back when I didn't know any better. In
CASSANDRA-6007.

[~appodictic] Your patch goes way beyond what needs to be fixed, nor do I think it addresses
the original concern from Ariel (the problem lies deeper). You did however fix CASSANDRA-6007
breakage just by introducing the {{LoadingCache}}.

I'd ask you to go with the minimal change here, given that this should be in 2.1+, and get
a v2 that *only* changes {{StorageProxy.hintsInProgress}} from {{CacheLoader}} to the intended
{{LoadingCache}}. Or I can extract it for you instead.

Replacing {{AtomicLong}} with {{Counter}} and introducing new metrics is beyond the scope
of the bug fix - feel free to open a 3.X or trunk ticket for that, separately.

> OverloadedException is untested
> -------------------------------
>
>                 Key: CASSANDRA-10825
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-10825
>             Project: Cassandra
>          Issue Type: Bug
>          Components: Local Write-Read Paths
>            Reporter: Ariel Weisberg
>            Assignee: Edward Capriolo
>         Attachments: jmx-hint.png
>
>
> If you grep test/src and cassandra-dtest you will find that the string OverloadedException
doesn't appear anywhere.
> In CASSANDRA-10477 it was found that there were cases where Paxos should back-pressure
and throw OverloadedException but didn't.
> If OverloadedException is used for functional purposes then we should test that it is
thrown under expected conditions. If there are behaviors driven by catching or tracking OverloadedException
we should test those as well.



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

Mime
View raw message