You are viewing a plain text version of this content. The canonical link for it is here.
Posted to pr@cassandra.apache.org by "smiklosovic (via GitHub)" <gi...@apache.org> on 2023/09/21 08:33:30 UTC
[GitHub] [cassandra] smiklosovic opened a new pull request, #2712: CASSANDRA-18872 5.0 removal of crc_check_chance from compression params
smiklosovic opened a new pull request, #2712:
URL: https://github.com/apache/cassandra/pull/2712
Thanks for sending a pull request! Here are some tips if you're new here:
* Ensure you have added or run the [appropriate tests](https://cassandra.apache.org/_/development/testing.html) for your PR.
* Be sure to keep the PR description updated to reflect all changes.
* Write your PR title to summarize what this PR proposes.
* If possible, provide a concise example to reproduce the issue for a faster review.
* Read our [contributor guidelines](https://cassandra.apache.org/_/development/index.html)
* If you're making a documentation change, see our [guide to documentation contribution](https://cassandra.apache.org/_/development/documentation.html)
Commit messages should follow the following format:
```
<One sentence description, usually Jira title or CHANGES.txt summary>
<Optional lengthier description (context on patch)>
patch by <Authors>; reviewed by <Reviewers> for CASSANDRA-#####
Co-authored-by: Name1 <email1>
Co-authored-by: Name2 <email2>
```
The [Cassandra Jira](https://issues.apache.org/jira/projects/CASSANDRA/issues/)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSANDRA-18872 5.0 removal of crc_check_chance from compression params [cassandra]
Posted by "smiklosovic (via GitHub)" <gi...@apache.org>.
smiklosovic commented on code in PR #2712:
URL: https://github.com/apache/cassandra/pull/2712#discussion_r1372155090
##########
doc/modules/cassandra/pages/managing/operating/compression.adoc:
##########
@@ -145,6 +140,17 @@ immediately, the operator can trigger an SSTable rewrite using
`nodetool scrub` or `nodetool upgradesstables -a`, both of which will
rebuild the SSTables on disk, re-compressing the data in the process.
+== Other options
+
+* `crc_check_chance` (default: `1.0`): determines how likely Cassandra
+is to verify the checksum on each compression chunk during reads to
+protect against data corruption. Unless you have profiles indicating
+this is a performance problem it is highly encouraged not to turn this
+off as it is Cassandra's only protection against bitrot. This option was part
Review Comment:
sure
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSANDRA-18872 5.0 removal of crc_check_chance from compression params [cassandra]
Posted by "smiklosovic (via GitHub)" <gi...@apache.org>.
smiklosovic commented on code in PR #2712:
URL: https://github.com/apache/cassandra/pull/2712#discussion_r1372153463
##########
doc/modules/cassandra/pages/developing/cql/cql_singlefile.adoc:
##########
@@ -579,6 +579,14 @@ link:#compressionOptions[below].
|`caching` |_map_ |_see below_ |Caching options, see
link:#cachingOptions[below].
+
+|`crc_check_chance` |simple|1.0|When compression is enabled, each compressed
Review Comment:
sure, works for me.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSANDRA-18872 5.0 removal of crc_check_chance from compression params [cassandra]
Posted by "Maxwell-Guo (via GitHub)" <gi...@apache.org>.
Maxwell-Guo commented on code in PR #2712:
URL: https://github.com/apache/cassandra/pull/2712#discussion_r1351368754
##########
test/unit/org/apache/cassandra/cql3/validation/miscellaneous/CrcCheckChanceTest.java:
##########
@@ -36,31 +36,12 @@
import org.apache.cassandra.io.util.RandomAccessReader;
import org.apache.cassandra.utils.FBUtilities;
-
public class CrcCheckChanceTest extends CQLTester
{
-
-
- @Test
- public void testChangingCrcCheckChanceNewFormat() throws Throwable
- {
- testChangingCrcCheckChance(true);
- }
-
@Test
- public void testChangingCrcCheckChanceOldFormat() throws Throwable
- {
- testChangingCrcCheckChance(false);
- }
-
-
- public void testChangingCrcCheckChance(boolean newFormat) throws Throwable
+ public void testChangingCrcCheckChance()
{
- //Start with crc_check_chance of 99%
- if (newFormat)
- createTable("CREATE TABLE %s (p text, c text, v text, s text static, PRIMARY KEY (p, c)) WITH compression = {'class': 'LZ4Compressor'} AND crc_check_chance = 0.99;");
- else
- createTable("CREATE TABLE %s (p text, c text, v text, s text static, PRIMARY KEY (p, c)) WITH compression = {'class': 'LZ4Compressor', 'crc_check_chance' : 0.99}");
+ createTable("CREATE TABLE %s (p text, c text, v text, s text static, PRIMARY KEY (p, c)) WITH compression = {'class': 'LZ4Compressor'} AND crc_check_chance = 0.99;");
Review Comment:
What will happen if the user still set the crc_check_chance through compression ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSANDRA-18872 5.0 removal of crc_check_chance from compression params [cassandra]
Posted by "blambov (via GitHub)" <gi...@apache.org>.
blambov commented on code in PR #2712:
URL: https://github.com/apache/cassandra/pull/2712#discussion_r1371527733
##########
doc/modules/cassandra/pages/managing/operating/compression.adoc:
##########
@@ -145,6 +140,17 @@ immediately, the operator can trigger an SSTable rewrite using
`nodetool scrub` or `nodetool upgradesstables -a`, both of which will
rebuild the SSTables on disk, re-compressing the data in the process.
+== Other options
+
+* `crc_check_chance` (default: `1.0`): determines how likely Cassandra
+is to verify the checksum on each compression chunk during reads to
+protect against data corruption. Unless you have profiles indicating
+this is a performance problem it is highly encouraged not to turn this
+off as it is Cassandra's only protection against bitrot. This option was part
Review Comment:
How about "In earlier versions of Cassandra a duplicate of this option existed in the compression configuration. The latter was deprecated in Cassandra 3.0 and removed in Cassandra 5.0."?
##########
doc/modules/cassandra/pages/developing/cql/cql_singlefile.adoc:
##########
@@ -579,6 +579,14 @@ link:#compressionOptions[below].
|`caching` |_map_ |_see below_ |Caching options, see
link:#cachingOptions[below].
+
+|`crc_check_chance` |simple|1.0|When compression is enabled, each compressed
Review Comment:
This looks a bit incorrect. We do generate checksums for all data files, but we only check them on reads with compressed files (in part because it's impossible to do for the memory-mapped access mode). Perhaps
"This option defines the probability with which checksums should be checked during reads to detect bit rot and prevent the propagation of corruption to other replicas. The default value is 1 to apply a checksum every time a data chunk is read. Set to 0 to disable checksum checking and to 0.5 for instance to check every other read.
Due to technical limitations we only currently apply this for compressed files. If compression is not enabled on the table, no checksums will be verified."?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSANDRA-18872 5.0 removal of crc_check_chance from compression params [cassandra]
Posted by "smiklosovic (via GitHub)" <gi...@apache.org>.
smiklosovic closed pull request #2712: CASSANDRA-18872 5.0 removal of crc_check_chance from compression params
URL: https://github.com/apache/cassandra/pull/2712
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org