You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cassandra.apache.org by "Giacomo Lo Giusto (JIRA)" <ji...@apache.org> on 2019/07/18 01:11:00 UTC

[jira] [Updated] (CASSANDRA-15235) Have Verifier check whether cells are out-of-order

     [ https://issues.apache.org/jira/browse/CASSANDRA-15235?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Giacomo Lo Giusto updated CASSANDRA-15235:
------------------------------------------
    Description: 
Hello,

This change was tested only for version {{*2.2.13*}}.

We noticed the {{nodetool verify -e}} command was not able to detect corrupt {{SSTables}} that exhibited out-of-order cells within a row. 
This is in contrast to the {{nodetool scrub}} command, which was able to detect and scrub such corrupted data files.

The proposed changes (see attached patch) include:
 * Reusing Scrub's {{OrderCheckerIterator}} in the Verifier (for its _extended_ use).
 * Some added logging to better debug what was the cause of the verification failure and which key first showed the issue.
 * Added unit tests for the Verifier ({{VerifyTest.java}}).

(Some other unrelated test where sometimes failing on our end and were therefore changed to enhance their deterministic behavior).

Please let me know if the change has value and is correct and safe for all possible configurations. Should we introduce an extra flag to enable the extra cell ordering check?
In the {{Verifier}} code there was this line (n. 189) that seemed to suggest that the newly introduced check was in fact an intended behavior all along, although we could not replicate this behavior neither in unit test nor with our production data:
{code:java}
//mimic the scrub read path
new SSTableIdentityIterator(sstable, dataFile, key, true);
{code}

Thanks in advance for your feedback and consideration,
Giacomo

  was:
Hello,

This change was tested only for version {{*2.2.13*}}.

We noticed the {{nodetool verify -e}} command was not able to detect corrupt {{SSTables}} that exhibited out-of-order cells within a row. 
 This is in contrast to the {{nodetool scrub}} command, which was able to detect and scrub such corrupted data files.

The proposed changes (see attached patch) include:
 * Reusing Scrub's {{OrderCheckerIterator}} in the Verifier (for its _extended_ use).
 * Some added logging to better debug what was the cause of the verification failure and which key first showed the issue.
 * Added unit tests for the Verifier ({{VerifyTest.java}}).

(Some other unrelated test where sometimes failing on our end and where therefore changed to enhance their deterministic behavior).

Please let me know if the change has value and is correct and safe for all possible configurations. Should we introduce an extra flag to enable the extra cell ordering check?
 In the {{Verifier}} code there was this line (n. 189) that seemed to suggest that the newly introduced check was in fact an intended behavior all along, although we could not replicate this behavior neither in unit test nor with our production data:
{code:java}
//mimic the scrub read path
new SSTableIdentityIterator(sstable, dataFile, key, true);
{code}

Thanks in advance for your feedback and consideration,
Giacomo


> Have Verifier check whether cells are out-of-order
> --------------------------------------------------
>
>                 Key: CASSANDRA-15235
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-15235
>             Project: Cassandra
>          Issue Type: Improvement
>            Reporter: Giacomo Lo Giusto
>            Priority: Normal
>         Attachments: verifier.patch
>
>
> Hello,
> This change was tested only for version {{*2.2.13*}}.
> We noticed the {{nodetool verify -e}} command was not able to detect corrupt {{SSTables}} that exhibited out-of-order cells within a row. 
> This is in contrast to the {{nodetool scrub}} command, which was able to detect and scrub such corrupted data files.
> The proposed changes (see attached patch) include:
>  * Reusing Scrub's {{OrderCheckerIterator}} in the Verifier (for its _extended_ use).
>  * Some added logging to better debug what was the cause of the verification failure and which key first showed the issue.
>  * Added unit tests for the Verifier ({{VerifyTest.java}}).
> (Some other unrelated test where sometimes failing on our end and were therefore changed to enhance their deterministic behavior).
> Please let me know if the change has value and is correct and safe for all possible configurations. Should we introduce an extra flag to enable the extra cell ordering check?
> In the {{Verifier}} code there was this line (n. 189) that seemed to suggest that the newly introduced check was in fact an intended behavior all along, although we could not replicate this behavior neither in unit test nor with our production data:
> {code:java}
> //mimic the scrub read path
> new SSTableIdentityIterator(sstable, dataFile, key, true);
> {code}
> Thanks in advance for your feedback and consideration,
> Giacomo



--
This message was sent by Atlassian JIRA
(v7.6.14#76016)

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cassandra.apache.org
For additional commands, e-mail: commits-help@cassandra.apache.org