cassandra-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Ariel Weisberg (JIRA)" <j...@apache.org>
Subject [jira] [Comment Edited] (CASSANDRA-9102) Consistency levels such as non-local quorum need better tests
Date Mon, 22 Jun 2015 17:29:00 GMT

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

Ariel Weisberg edited comment on CASSANDRA-9102 at 6/22/15 5:28 PM:
--------------------------------------------------------------------

Sorry I am late to commenting on this. I think this is adding a lot of coverage we don't have.
The classes that concern me on the read and write path are AbstractReadExecutor, and the StorageProxy
read/write path. My beef with the coverage there was that there is no unit test coverage before
this task.

As far as I can tell those have little/no unit test coverage because they aren't testable
due to an inability to mock the entire thing. So that leads to a dtest trying to hit all those
paths. Even though it's a dtest it would be helpful for it to behave more like a unit test
when testing each branch. So rather then having sessions racing and checking for the correct
result of the race (which may or may not race all) construct a scenario where you know specific
code will run. This will make it fail every time instead of just some of the time when there
is a bug. I think checking for races, or testing functionality in a racy way, would be better
handled by the kitchen sink harness which runs for a long time and not on every commit.

Maybe from a unit testing (via dtests) perspective we want a way to blackhole writes to part
of the cluster to force recovery paths without failing nodes? Otherwise from a dtest you would
have to kill a node, do the write, and then bring it back, in order to know the state of the
node when you go to have it participate in a read/write. This is all working around the fact
that you can't write it as a unit test without invasive refactoring (and maybe that is the
answer). I am kind of warming up to the idea of being able to ask a node to do something like
that from a dtest.


was (Author: aweisberg):
Sorry I am late to commenting on this. I think this is adding a lot of coverage we don't have.
The classes that concern me on the read and write path are AbstractReadExecutor, and the StorageProxy
read/write path. My beef with the coverage there was that there is no unit test coverage before
this task.

As far as I can tell those have little/no unit test coverage because they aren't testable
due to an inability to mock the entire thing. So that leads to a dtest trying to hit all those
paths. Even though it's a dtest it would be helpful for it to behave more like a unit test
when testing each branch. So rather then having sessions racing and checking for the correct
result of the race (which may or may not race all) construct a scenario where you know specific
code will run. This will make it fail every time instead of just some of the time when there
is a bug. I think our focus for that kind of race would be better handled by the kitchen sink
harness which runs for a long time and not on every commit.

Maybe from a unit testing (via dtests) perspective we want a way to blackhole writes to part
of the cluster to force recovery paths without failing nodes? Otherwise from a dtest you would
have to kill a node, do the write, and then bring it back, in order to know the state of the
node when you go to have it participate in a read/write. This is all working around the fact
that you can't write it as a unit test without invasive refactoring (and maybe that is the
answer). I am kind of warming up to the idea of being able to ask a node to do something like
that from a dtest.

> Consistency levels such as non-local quorum need better tests
> -------------------------------------------------------------
>
>                 Key: CASSANDRA-9102
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-9102
>             Project: Cassandra
>          Issue Type: Test
>            Reporter: Ariel Weisberg
>            Assignee: Stefania
>
> We didn't catch unit testing for this functionality. There is dtest consistency_test
but it doesn't cover non-local functionality.



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

Mime
View raw message