[jira] [Commented] (CASSANDRA-14467) Add option to sanity check tombstones on reads/compaction
[ https://issues.apache.org/jira/browse/CASSANDRA-14467?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16628462#comment-16628462 ] ASF GitHub Bot commented on CASSANDRA-14467: Github user asfgit closed the pull request at: https://github.com/apache/cassandra-dtest/pull/30 > Add option to sanity check tombstones on reads/compaction > - > > Key: CASSANDRA-14467 > URL: https://issues.apache.org/jira/browse/CASSANDRA-14467 > Project: Cassandra > Issue Type: Improvement >Reporter: Marcus Eriksson >Assignee: Marcus Eriksson >Priority: Minor > Labels: pull-request-available > Fix For: 4.x > > > We should add an option to do a quick sanity check of tombstones on reads + > compaction. It should either log the error or throw an exception. -- This message was sent by Atlassian JIRA (v7.6.3#76005) - To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org
[jira] [Commented] (CASSANDRA-14467) Add option to sanity check tombstones on reads/compaction
[ https://issues.apache.org/jira/browse/CASSANDRA-14467?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16622416#comment-16622416 ] Ariel Weisberg commented on CASSANDRA-14467: Seems like I dropped the ball here. +1 assuming the dtests still pass with this check enabled. > Add option to sanity check tombstones on reads/compaction > - > > Key: CASSANDRA-14467 > URL: https://issues.apache.org/jira/browse/CASSANDRA-14467 > Project: Cassandra > Issue Type: Improvement >Reporter: Marcus Eriksson >Assignee: Marcus Eriksson >Priority: Minor > Fix For: 4.x > > > We should add an option to do a quick sanity check of tombstones on reads + > compaction. It should either log the error or throw an exception. -- This message was sent by Atlassian JIRA (v7.6.3#76005) - To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org
[jira] [Commented] (CASSANDRA-14467) Add option to sanity check tombstones on reads/compaction
[ https://issues.apache.org/jira/browse/CASSANDRA-14467?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16526064#comment-16526064 ] Marcus Eriksson commented on CASSANDRA-14467: - yeah, waiting for you to review the dtest changes before closing > Add option to sanity check tombstones on reads/compaction > - > > Key: CASSANDRA-14467 > URL: https://issues.apache.org/jira/browse/CASSANDRA-14467 > Project: Cassandra > Issue Type: Improvement >Reporter: Marcus Eriksson >Assignee: Marcus Eriksson >Priority: Minor > Fix For: 4.x > > > We should add an option to do a quick sanity check of tombstones on reads + > compaction. It should either log the error or throw an exception. -- This message was sent by Atlassian JIRA (v7.6.3#76005) - To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org
[jira] [Commented] (CASSANDRA-14467) Add option to sanity check tombstones on reads/compaction
[ https://issues.apache.org/jira/browse/CASSANDRA-14467?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16525347#comment-16525347 ] Ariel Weisberg commented on CASSANDRA-14467: Committed but still set to patch available? > Add option to sanity check tombstones on reads/compaction > - > > Key: CASSANDRA-14467 > URL: https://issues.apache.org/jira/browse/CASSANDRA-14467 > Project: Cassandra > Issue Type: Improvement >Reporter: Marcus Eriksson >Assignee: Marcus Eriksson >Priority: Minor > Fix For: 4.x > > > We should add an option to do a quick sanity check of tombstones on reads + > compaction. It should either log the error or throw an exception. -- This message was sent by Atlassian JIRA (v7.6.3#76005) - To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org
[jira] [Commented] (CASSANDRA-14467) Add option to sanity check tombstones on reads/compaction
[ https://issues.apache.org/jira/browse/CASSANDRA-14467?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16502436#comment-16502436 ] Marcus Eriksson commented on CASSANDRA-14467: - not that I know of, people will need to rebase to get their existing trunk-branches working > Add option to sanity check tombstones on reads/compaction > - > > Key: CASSANDRA-14467 > URL: https://issues.apache.org/jira/browse/CASSANDRA-14467 > Project: Cassandra > Issue Type: Improvement >Reporter: Marcus Eriksson >Assignee: Marcus Eriksson >Priority: Minor > Fix For: 4.x > > > We should add an option to do a quick sanity check of tombstones on reads + > compaction. It should either log the error or throw an exception. -- This message was sent by Atlassian JIRA (v7.6.3#76005) - To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org
[jira] [Commented] (CASSANDRA-14467) Add option to sanity check tombstones on reads/compaction
[ https://issues.apache.org/jira/browse/CASSANDRA-14467?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16502425#comment-16502425 ] Ariel Weisberg commented on CASSANDRA-14467: The thing about the dtest changes is that it's going to produce a config file that won't work with revisions prior to the introduction of the config option right? Do we have a solution for that? > Add option to sanity check tombstones on reads/compaction > - > > Key: CASSANDRA-14467 > URL: https://issues.apache.org/jira/browse/CASSANDRA-14467 > Project: Cassandra > Issue Type: Improvement >Reporter: Marcus Eriksson >Assignee: Marcus Eriksson >Priority: Minor > Fix For: 4.x > > > We should add an option to do a quick sanity check of tombstones on reads + > compaction. It should either log the error or throw an exception. -- This message was sent by Atlassian JIRA (v7.6.3#76005) - To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org
[jira] [Commented] (CASSANDRA-14467) Add option to sanity check tombstones on reads/compaction
[
https://issues.apache.org/jira/browse/CASSANDRA-14467?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16502412#comment-16502412
]
Marcus Eriksson commented on CASSANDRA-14467:
-
[~aweisberg] did you review the dtest changes? If not, could you just quickly
check the PR above?
the cassandra changes were committed as
{{5d8767765090cd968c39008f76b0cd795d6e5032}}, thanks!
> Add option to sanity check tombstones on reads/compaction
> -
>
> Key: CASSANDRA-14467
> URL: https://issues.apache.org/jira/browse/CASSANDRA-14467
> Project: Cassandra
> Issue Type: Improvement
>Reporter: Marcus Eriksson
>Assignee: Marcus Eriksson
>Priority: Minor
> Fix For: 4.x
>
>
> We should add an option to do a quick sanity check of tombstones on reads +
> compaction. It should either log the error or throw an exception.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
-
To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org
For additional commands, e-mail: commits-h...@cassandra.apache.org
[jira] [Commented] (CASSANDRA-14467) Add option to sanity check tombstones on reads/compaction
[ https://issues.apache.org/jira/browse/CASSANDRA-14467?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16502411#comment-16502411 ] ASF GitHub Bot commented on CASSANDRA-14467: GitHub user krummas opened a pull request: https://github.com/apache/cassandra-dtest/pull/30 CASSANDRA-14467: always enable tombstone validation exceptions during tests You can merge this pull request into a Git repository by running: $ git pull https://github.com/krummas/cassandra-dtest marcuse/14467 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/cassandra-dtest/pull/30.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #30 commit 3dc02646948120e9847347f951d1539c16cef2a9 Author: Marcus Eriksson Date: 2018-05-31T06:41:11Z always enable tombstone validation exceptions during tests > Add option to sanity check tombstones on reads/compaction > - > > Key: CASSANDRA-14467 > URL: https://issues.apache.org/jira/browse/CASSANDRA-14467 > Project: Cassandra > Issue Type: Improvement >Reporter: Marcus Eriksson >Assignee: Marcus Eriksson >Priority: Minor > Fix For: 4.x > > > We should add an option to do a quick sanity check of tombstones on reads + > compaction. It should either log the error or throw an exception. -- This message was sent by Atlassian JIRA (v7.6.3#76005) - To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org
[jira] [Commented] (CASSANDRA-14467) Add option to sanity check tombstones on reads/compaction
[ https://issues.apache.org/jira/browse/CASSANDRA-14467?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16502109#comment-16502109 ] Ariel Weisberg commented on CASSANDRA-14467: +1 with the latest changes. > Add option to sanity check tombstones on reads/compaction > - > > Key: CASSANDRA-14467 > URL: https://issues.apache.org/jira/browse/CASSANDRA-14467 > Project: Cassandra > Issue Type: Improvement >Reporter: Marcus Eriksson >Assignee: Marcus Eriksson >Priority: Minor > Fix For: 4.x > > > We should add an option to do a quick sanity check of tombstones on reads + > compaction. It should either log the error or throw an exception. -- This message was sent by Atlassian JIRA (v7.6.3#76005) - To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org
[jira] [Commented] (CASSANDRA-14467) Add option to sanity check tombstones on reads/compaction
[ https://issues.apache.org/jira/browse/CASSANDRA-14467?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16498155#comment-16498155 ] Ariel Weisberg commented on CASSANDRA-14467: Only remaining nit is that when you reset the setting in the unit tests it's not exception safe so if the test fails it will remain disabled. Otherwise +1. > Add option to sanity check tombstones on reads/compaction > - > > Key: CASSANDRA-14467 > URL: https://issues.apache.org/jira/browse/CASSANDRA-14467 > Project: Cassandra > Issue Type: Improvement >Reporter: Marcus Eriksson >Assignee: Marcus Eriksson >Priority: Minor > Fix For: 4.x > > > We should add an option to do a quick sanity check of tombstones on reads + > compaction. It should either log the error or throw an exception. -- This message was sent by Atlassian JIRA (v7.6.3#76005) - To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org
[jira] [Commented] (CASSANDRA-14467) Add option to sanity check tombstones on reads/compaction
[ https://issues.apache.org/jira/browse/CASSANDRA-14467?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16497918#comment-16497918 ] Marcus Eriksson commented on CASSANDRA-14467: - bq. why is this not just an assertion? It seems very assertion like? Agree, was debating this a bit, but biggest reason I went this way was to have a way of just logging any validation failures. Also, doing this validation is not very expensive, but it is not free, so you might not want to disable all assertions to just disable this check. Push a few new commits addressing the latest round of comments > Add option to sanity check tombstones on reads/compaction > - > > Key: CASSANDRA-14467 > URL: https://issues.apache.org/jira/browse/CASSANDRA-14467 > Project: Cassandra > Issue Type: Improvement >Reporter: Marcus Eriksson >Assignee: Marcus Eriksson >Priority: Minor > Fix For: 4.x > > > We should add an option to do a quick sanity check of tombstones on reads + > compaction. It should either log the error or throw an exception. -- This message was sent by Atlassian JIRA (v7.6.3#76005) - To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org
[jira] [Commented] (CASSANDRA-14467) Add option to sanity check tombstones on reads/compaction
[ https://issues.apache.org/jira/browse/CASSANDRA-14467?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16496749#comment-16496749 ] Ariel Weisberg commented on CASSANDRA-14467: I'm a little late thinking of this, but why is this not just an assertion? It seems very assertion like? I'm just curious why this is different from other things we assert on? Is it expensive to check? I'm digging the defensive additions. I left a few comments on the usage of NoSpamLogger which is a little tricky. > Add option to sanity check tombstones on reads/compaction > - > > Key: CASSANDRA-14467 > URL: https://issues.apache.org/jira/browse/CASSANDRA-14467 > Project: Cassandra > Issue Type: Improvement >Reporter: Marcus Eriksson >Assignee: Marcus Eriksson >Priority: Minor > Fix For: 4.x > > > We should add an option to do a quick sanity check of tombstones on reads + > compaction. It should either log the error or throw an exception. -- This message was sent by Atlassian JIRA (v7.6.3#76005) - To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org
[jira] [Commented] (CASSANDRA-14467) Add option to sanity check tombstones on reads/compaction
[ https://issues.apache.org/jira/browse/CASSANDRA-14467?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16496542#comment-16496542 ] Marcus Eriksson commented on CASSANDRA-14467: - pushed a new commit which throws CorruptSSTableException instead and marks the sstable as suspect to avoid it being included in future compactions > Add option to sanity check tombstones on reads/compaction > - > > Key: CASSANDRA-14467 > URL: https://issues.apache.org/jira/browse/CASSANDRA-14467 > Project: Cassandra > Issue Type: Improvement >Reporter: Marcus Eriksson >Assignee: Marcus Eriksson >Priority: Minor > Fix For: 4.x > > > We should add an option to do a quick sanity check of tombstones on reads + > compaction. It should either log the error or throw an exception. -- This message was sent by Atlassian JIRA (v7.6.3#76005) - To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org
[jira] [Commented] (CASSANDRA-14467) Add option to sanity check tombstones on reads/compaction
[ https://issues.apache.org/jira/browse/CASSANDRA-14467?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16496392#comment-16496392 ] Marcus Eriksson commented on CASSANDRA-14467: - Pushed a dtest branch https://github.com/krummas/cassandra-dtest/commits/marcuse/14467 and enabled it for unit tests as well For the unit tests this seems to be flaky: CASSANDRA-14238 The failing dtest (test_describecluster_more_information_three_datacenters) seems to be unrelated as well, created CASSANDRA-14484 > Add option to sanity check tombstones on reads/compaction > - > > Key: CASSANDRA-14467 > URL: https://issues.apache.org/jira/browse/CASSANDRA-14467 > Project: Cassandra > Issue Type: Improvement >Reporter: Marcus Eriksson >Assignee: Marcus Eriksson >Priority: Minor > Fix For: 4.x > > > We should add an option to do a quick sanity check of tombstones on reads + > compaction. It should either log the error or throw an exception. -- This message was sent by Atlassian JIRA (v7.6.3#76005) - To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org
[jira] [Commented] (CASSANDRA-14467) Add option to sanity check tombstones on reads/compaction
[ https://issues.apache.org/jira/browse/CASSANDRA-14467?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16495476#comment-16495476 ] Ariel Weisberg commented on CASSANDRA-14467: Created a pull request with review comments https://github.com/apache/cassandra/pull/228 Also can you run the tests again there were some failures. It's been fairly green for me lately on trunk. > Add option to sanity check tombstones on reads/compaction > - > > Key: CASSANDRA-14467 > URL: https://issues.apache.org/jira/browse/CASSANDRA-14467 > Project: Cassandra > Issue Type: Improvement >Reporter: Marcus Eriksson >Assignee: Marcus Eriksson >Priority: Minor > Fix For: 4.x > > > We should add an option to do a quick sanity check of tombstones on reads + > compaction. It should either log the error or throw an exception. -- This message was sent by Atlassian JIRA (v7.6.3#76005) - To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org
[jira] [Commented] (CASSANDRA-14467) Add option to sanity check tombstones on reads/compaction
[ https://issues.apache.org/jira/browse/CASSANDRA-14467?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16490666#comment-16490666 ] Marcus Eriksson commented on CASSANDRA-14467: - [~cscotta] yes, correct, it also checks if the localDeletionTime for partition level deletions is less than zero > Add option to sanity check tombstones on reads/compaction > - > > Key: CASSANDRA-14467 > URL: https://issues.apache.org/jira/browse/CASSANDRA-14467 > Project: Cassandra > Issue Type: Improvement >Reporter: Marcus Eriksson >Assignee: Marcus Eriksson >Priority: Minor > Fix For: 4.x > > > We should add an option to do a quick sanity check of tombstones on reads + > compaction. It should either log the error or throw an exception. -- This message was sent by Atlassian JIRA (v7.6.3#76005) - To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org
[jira] [Commented] (CASSANDRA-14467) Add option to sanity check tombstones on reads/compaction
[ https://issues.apache.org/jira/browse/CASSANDRA-14467?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16489734#comment-16489734 ] C. Scott Andreas commented on CASSANDRA-14467: -- Thanks Marcus! To confirm my understanding, looks like this approach identifies a tombstone as potentially invalid when: *Cell:* * A cell's TTL is less than zero; * Its local deletion time is less than zero; * Or a TTL is set but its local deletion time is Integer.MAX_VALUE *Row:* * Primary key has a TTL less than zero; * Primary key's local expiration time is less than zero; * Row's deletion timestamp < 0 (!>=0); * Or if the column data it contains has invalid deletions (either via complexDeletion validation for range tombstones, or as defined using the Cell criteria above for cells) Does that summary look correct to you? > Add option to sanity check tombstones on reads/compaction > - > > Key: CASSANDRA-14467 > URL: https://issues.apache.org/jira/browse/CASSANDRA-14467 > Project: Cassandra > Issue Type: Improvement >Reporter: Marcus Eriksson >Assignee: Marcus Eriksson >Priority: Minor > Fix For: 4.x > > > We should add an option to do a quick sanity check of tombstones on reads + > compaction. It should either log the error or throw an exception. -- This message was sent by Atlassian JIRA (v7.6.3#76005) - To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org
