natas_hw
/
cassandra
mirror of https://github.com/apache/cassandra
0
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Remove crc_check_chance from CompressionParams 

patch by Stefan Miklosovic; reviewed by Maxwell Guo, Jacek Lewandowski, Branimir Lambov for CASSANDRA-18872
This commit is contained in:
Stefan Miklosovic 2023-10-26 12:21:44 +02:00
parent 7d54a62ccc
commit b59b832eba
No known key found for this signature in database
GPG Key ID: 32F35CB2F546D93E
15 changed files with 68 additions and 127 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
CHANGES.txt
View File

@ -1,4 +1,5 @@
5.0-alpha2
* Remove crc_check_chance from CompressionParams (CASSANDRA-18872)
* Fix schema loading of UDTs inside vectors inside UDTs (CASSANDRA-18964)
* Add cqlsh autocompletion for the vector data type (CASSANDRA-18946)
* Fix nodetool tablehistograms output to avoid printing repeated information and ensure at most two arguments (CASSANDRA-18955)

16
doc/modules/cassandra/pages/developing/cql/cql_singlefile.adoc
View File

@ -579,6 +579,14 @@ link:#compressionOptions[below].
|`caching` |_map_ |_see below_ |Caching options, see
link:#cachingOptions[below].
|`crc_check_chance` |simple|1.0|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.
|===
[[compactionOptions]]
@ -684,14 +692,6 @@ constant]. | | |
compressed by block (to allow random reads). This defines the size (in
KB) of said block. Bigger values may improve the compression rate, but
increases the minimum size of data to be read from disk for a read
|`crc_check_chance` |1.0 |When compression is enabled, each compressed
block includes a checksum of that block for the purpose of detecting
disk bitrot and avoiding the propagation of corruption to other replica.
This option defines the probability with which those checksums are
checked during read. By default they are always checked. Set to 0 to
disable checksum checking and to 0.5 for instance to check them every
other read | | |
|===
[[cachingOptions]]

3
doc/modules/cassandra/pages/developing/cql/ddl.adoc
View File

@ -631,9 +631,6 @@ If the `enabled` option is set to `false`, no other options must be specified.
| `chunk_length_in_kb` | 64 | On disk SSTables are compressed by block (to allow random reads).
This option defines the size (in KB) of said block. See xref:cassandra:developing/cql/ddl.adoc#chunk_note[note] for further information.
| `crc_check_chance` | 1.0 | Determines how likely Cassandra is to verify the checksum on each
compression chunk during reads.
| `compression_level` | 3 | Compression level. Only applicable for `ZstdCompressor`.
Accepts values between `-131072` and `22`.
|===

17
doc/modules/cassandra/pages/managing/operating/compression.adoc
View File

@ -81,11 +81,6 @@ use. The two "fast" compressors are `LZ4Compressor` and
kilobytes of data per compression chunk. The main tradeoff here is that
larger chunk sizes give compression algorithms more context and improve
their ratio, but require reads to deserialize and read more off disk.
* `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.
The `LZ4Compressor` supports the following additional options:
@ -123,7 +118,7 @@ Or
[source,cql]
----
ALTER TABLE keyspace.table
WITH compression = {'class': 'LZ4Compressor', 'chunk_length_in_kb': 64, 'crc_check_chance': 0.5};
WITH compression = {'class': 'LZ4Compressor', 'chunk_length_in_kb': 64};
----
Once enabled, compression can be disabled with `ALTER TABLE` setting
@ -145,6 +140,16 @@ 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. 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.
== Benefits and Uses
Compression's primary benefit is that it reduces the amount of data

24
src/java/org/apache/cassandra/cql3/statements/schema/TableAttributes.java
View File

@ -97,8 +97,8 @@ public final class TableAttributes extends PropertyDefinitions
private TableParams build(TableParams.Builder builder)
{
if (hasOption(Option.ALLOW_AUTO_SNAPSHOT))
builder.allowAutoSnapshot(getBoolean(Option.ALLOW_AUTO_SNAPSHOT.toString(), true));
if (hasOption(ALLOW_AUTO_SNAPSHOT))
builder.allowAutoSnapshot(getBoolean(ALLOW_AUTO_SNAPSHOT.toString(), true));
if (hasOption(BLOOM_FILTER_FP_CHANCE))
builder.bloomFilterFpChance(getDouble(BLOOM_FILTER_FP_CHANCE));
@ -113,20 +113,10 @@ public final class TableAttributes extends PropertyDefinitions
builder.compaction(CompactionParams.fromMap(getMap(COMPACTION)));
if (hasOption(COMPRESSION))
{
//crc_check_chance was "promoted" from a compression property to a top-level-property after #9839,
//so we temporarily accept it to be defined as a compression option, to maintain backwards compatibility
Map<String, String> compressionOpts = getMap(COMPRESSION);
if (compressionOpts.containsKey(CRC_CHECK_CHANCE.toString().toLowerCase()))
{
double crcCheckChance = getDeprecatedCrcCheckChance(compressionOpts);
builder.crcCheckChance(crcCheckChance);
}
builder.compression(CompressionParams.fromMap(getMap(COMPRESSION)));
}
if (hasOption(Option.MEMTABLE))
builder.memtable(MemtableParams.get(getString(Option.MEMTABLE)));
if (hasOption(MEMTABLE))
builder.memtable(MemtableParams.get(getString(MEMTABLE)));
if (hasOption(DEFAULT_TIME_TO_LIVE))
builder.defaultTimeToLive(getInt(DEFAULT_TIME_TO_LIVE));
@ -199,10 +189,4 @@ public final class TableAttributes extends PropertyDefinitions
{
return parseDouble(option.toString(), getString(option));
}
private double getDeprecatedCrcCheckChance(Map<String, String> compressionOpts)
{
String value = compressionOpts.get(CRC_CHECK_CHANCE.toString().toLowerCase());
return parseDouble(CRC_CHECK_CHANCE.toString(), value);
}
}

1
src/java/org/apache/cassandra/db/ColumnFamilyStore.java
View File

@ -2971,6 +2971,7 @@ public class ColumnFamilyStore implements ColumnFamilyStoreMBean, Memtable.Owner
for (ColumnFamilyStore cfs : concatWithIndexes())
{
cfs.crcCheckChance.set(crcCheckChance);
cfs.metadata.setLocalOverrides(cfs.metadata().unbuild().crcCheckChance(crcCheckChance).build());
for (SSTableReader sstable : cfs.getSSTables(SSTableSet.LIVE))
sstable.setCrcCheckChance(crcCheckChance);
}

1
src/java/org/apache/cassandra/io/sstable/format/SSTableReader.java
View File

@ -878,7 +878,6 @@ public abstract class SSTableReader extends SSTable implements UnfilteredSource,
public void setCrcCheckChance(double crcCheckChance)
{
this.crcCheckChance = crcCheckChance;
dfile.compressionMetadata().ifPresent(metadata -> metadata.parameters.setCrcCheckChance(crcCheckChance));
}
/**

4
src/java/org/apache/cassandra/io/sstable/format/SortedTableWriter.java
View File

@ -86,11 +86,14 @@ public abstract class SortedTableWriter<P extends SortedTablePartitionWriter, I
private DecoratedKey lastWrittenKey;
private DataPosition dataMark;
private long lastEarlyOpenLength;
private final Supplier<Double> crcCheckChanceSupplier;
public SortedTableWriter(Builder<P, I, ?, ?> builder, LifecycleNewTracker lifecycleNewTracker, SSTable.Owner owner)
{
super(builder, lifecycleNewTracker, owner);
TableMetadataRef ref = builder.getTableMetadataRef();
crcCheckChanceSupplier = () -> ref.getLocal().params.crcCheckChance;
SequentialWriter dataWriter = null;
I indexWriter = null;
P partitionWriter = null;
@ -351,6 +354,7 @@ public abstract class SortedTableWriter<P extends SortedTablePartitionWriter, I
.withChunkCache(chunkCache)
.withCompressionMetadata(compressionMetadata)
.bufferSize(dataBufferSize)
.withCrcCheckChance(crcCheckChanceSupplier)
.withLengthOverride(lengthOverride)
.complete();
}

5
src/java/org/apache/cassandra/io/sstable/format/big/BigSSTableReaderLoadingBuilder.java
View File

@ -139,7 +139,10 @@ public class BigSSTableReaderLoadingBuilder extends SortedTableReaderLoadingBuil
try (CompressionMetadata compressionMetadata = CompressionInfoComponent.maybeLoad(descriptor, components))
{
builder.setDataFile(dataFileBuilder(builder.getStatsMetadata()).withCompressionMetadata(compressionMetadata).complete());
builder.setDataFile(dataFileBuilder(builder.getStatsMetadata())
.withCompressionMetadata(compressionMetadata)
.withCrcCheckChance(() -> tableMetadataRef.getLocal().params.crcCheckChance)
.complete());
}
if (builder.getFilter() == null)

9
src/java/org/apache/cassandra/io/sstable/format/bti/BtiTableReaderLoadingBuilder.java
View File

@ -70,7 +70,9 @@ public class BtiTableReaderLoadingBuilder extends SortedTableReaderLoadingBuilde
try (PartitionIndex index = PartitionIndex.load(partitionIndexFileBuilder(), tableMetadataRef.getLocal().partitioner, false);
CompressionMetadata compressionMetadata = CompressionInfoComponent.maybeLoad(descriptor, components);
FileHandle dFile = dataFileBuilder(statsMetadata).withCompressionMetadata(compressionMetadata).complete();
FileHandle dFile = dataFileBuilder(statsMetadata).withCompressionMetadata(compressionMetadata)
.withCrcCheckChance(() -> tableMetadataRef.getLocal().params.crcCheckChance)
.complete();
FileHandle riFile = rowIndexFileBuilder().complete())
{
return PartitionIterator.create(index,
@ -132,7 +134,10 @@ public class BtiTableReaderLoadingBuilder extends SortedTableReaderLoadingBuilde
try (CompressionMetadata compressionMetadata = CompressionInfoComponent.maybeLoad(descriptor, components))
{
builder.setDataFile(dataFileBuilder(builder.getStatsMetadata()).withCompressionMetadata(compressionMetadata).complete());
builder.setDataFile(dataFileBuilder(builder.getStatsMetadata())
.withCompressionMetadata(compressionMetadata)
.withCrcCheckChance(() -> tableMetadataRef.getLocal().params.crcCheckChance)
.complete());
}
}
catch (IOException | RuntimeException | Error ex)

19
src/java/org/apache/cassandra/io/util/CompressedChunkReader.java
View File

@ -20,6 +20,8 @@ package org.apache.cassandra.io.util;
import java.io.IOException;
import java.nio.ByteBuffer;
import java.util.concurrent.ThreadLocalRandom;
import java.util.function.Supplier;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.primitives.Ints;
@ -34,24 +36,27 @@ public abstract class CompressedChunkReader extends AbstractReaderFileProxy impl
{
final CompressionMetadata metadata;
final int maxCompressedLength;
final Supplier<Double> crcCheckChanceSupplier;
protected CompressedChunkReader(ChannelProxy channel, CompressionMetadata metadata)
protected CompressedChunkReader(ChannelProxy channel, CompressionMetadata metadata, Supplier<Double> crcCheckChanceSupplier)
{
super(channel, metadata.dataLength);
this.metadata = metadata;
this.maxCompressedLength = metadata.maxCompressedLength();
this.crcCheckChanceSupplier = crcCheckChanceSupplier;
assert Integer.bitCount(metadata.chunkLength()) == 1; //must be a power of two
}
@VisibleForTesting
public double getCrcCheckChance()
{
return metadata.parameters.getCrcCheckChance();
return crcCheckChanceSupplier.get();
}
boolean shouldCheckCrc()
{
return metadata.parameters.shouldCheckCrc();
double checkChance = getCrcCheckChance();
return checkChance >= 1d || (checkChance > 0d && checkChance > ThreadLocalRandom.current().nextDouble());
}
@Override
@ -88,9 +93,9 @@ public abstract class CompressedChunkReader extends AbstractReaderFileProxy impl
// we read the raw compressed bytes into this buffer, then uncompressed them into the provided one.
private final ThreadLocalByteBufferHolder bufferHolder;
public Standard(ChannelProxy channel, CompressionMetadata metadata)
public Standard(ChannelProxy channel, CompressionMetadata metadata, Supplier<Double> crcCheckChanceSupplier)
{
super(channel, metadata);
super(channel, metadata, crcCheckChanceSupplier);
bufferHolder = new ThreadLocalByteBufferHolder(metadata.compressor().preferredBufferType());
}
@ -172,9 +177,9 @@ public abstract class CompressedChunkReader extends AbstractReaderFileProxy impl
{
protected final MmappedRegions regions;
public Mmap(ChannelProxy channel, CompressionMetadata metadata, MmappedRegions regions)
public Mmap(ChannelProxy channel, CompressionMetadata metadata, MmappedRegions regions, Supplier<Double> crcCheckChanceSupplier)
{
super(channel, metadata);
super(channel, metadata, crcCheckChanceSupplier);
this.regions = regions;
}

12
src/java/org/apache/cassandra/io/util/FileHandle.java
View File

@ -19,6 +19,7 @@ package org.apache.cassandra.io.util;
import java.util.Optional;
import java.util.function.Function;
import java.util.function.Supplier;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.util.concurrent.RateLimiter;
@ -252,6 +253,7 @@ public class FileHandle extends SharedCloseableImpl
public final File file;
private CompressionMetadata compressionMetadata;
private Supplier<Double> crcCheckChanceSupplier = () -> 1.0;
private ChunkCache chunkCache;
private int bufferSize = RandomAccessReader.DEFAULT_BUFFER_SIZE;
private BufferType bufferType = BufferType.OFF_HEAP;
@ -291,6 +293,12 @@ public class FileHandle extends SharedCloseableImpl
return this;
}
public Builder withCrcCheckChance(Supplier<Double> crcCheckChanceSupplier)
{
this.crcCheckChanceSupplier = crcCheckChanceSupplier;
return this;
}
/**
* Set whether to use mmap for reading
*
@ -386,7 +394,7 @@ public class FileHandle extends SharedCloseableImpl
{
regions = mmappedRegionsCache != null ? mmappedRegionsCache.getOrCreate(channel, compressionMetadata)
: MmappedRegions.map(channel, compressionMetadata);
rebuffererFactory = maybeCached(new CompressedChunkReader.Mmap(channel, compressionMetadata, regions));
rebuffererFactory = maybeCached(new CompressedChunkReader.Mmap(channel, compressionMetadata, regions, crcCheckChanceSupplier));
}
else
{
@ -399,7 +407,7 @@ public class FileHandle extends SharedCloseableImpl
{
if (compressionMetadata != null)
{
rebuffererFactory = maybeCached(new CompressedChunkReader.Standard(channel, compressionMetadata));
rebuffererFactory = maybeCached(new CompressedChunkReader.Standard(channel, compressionMetadata, crcCheckChanceSupplier));
}
else
{

44
src/java/org/apache/cassandra/schema/CompressionParams.java
View File

@ -23,7 +23,6 @@ import java.lang.reflect.Method;
import java.util.Collections;
import java.util.HashMap;
import java.util.Map;
import java.util.concurrent.ThreadLocalRandom;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Objects;
@ -31,9 +30,6 @@ import com.google.common.collect.ImmutableMap;
import org.apache.commons.lang3.builder.HashCodeBuilder;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.apache.cassandra.config.CassandraRelevantProperties;
import org.apache.cassandra.config.ParameterizedClass;
import org.apache.cassandra.db.TypeSizes;
@ -48,10 +44,6 @@ import static java.lang.String.format;
public final class CompressionParams
{
private static final Logger logger = LoggerFactory.getLogger(CompressionParams.class);
private static volatile boolean hasLoggedCrcCheckChanceWarning;
public static final int DEFAULT_CHUNK_LENGTH = 1024 * 16;
public static final double DEFAULT_MIN_COMPRESS_RATIO = 0.0; // Since pre-4.0 versions do not understand the
// new compression parameter we can't use a
@ -78,21 +70,12 @@ public final class CompressionParams
DEFAULT_MIN_COMPRESS_RATIO,
Collections.emptyMap());
private static final String CRC_CHECK_CHANCE_WARNING = "The option crc_check_chance was deprecated as a compression option. " +
"You should specify it as a top-level table option instead";
/** @deprecated See CASSANDRA-9839 */
@Deprecated(since = "3.0") public static final String CRC_CHECK_CHANCE = "crc_check_chance";
private final ICompressor sstableCompressor;
private final int chunkLength;
private final int maxCompressedLength; // In content we store max length to avoid rounding errors causing compress/decompress mismatch.
private final double minCompressRatio; // In configuration we store min ratio, the input parameter.
private final ImmutableMap<String, String> otherOptions; // Unrecognized options, can be used by the compressor
// TODO: deprecated, should now be carefully removed. Doesn't affect schema code as it isn't included in equals() and hashCode()
private volatile double crcCheckChance = 1.0;
public static CompressionParams fromMap(Map<String, String> opts)
{
Map<String, String> options = copyOptions(opts);
@ -289,16 +272,6 @@ public final class CompressionParams
return null;
}
if (compressionOptions.containsKey(CRC_CHECK_CHANCE))
{
if (!hasLoggedCrcCheckChanceWarning)
{
logger.warn(CRC_CHECK_CHANCE_WARNING);
hasLoggedCrcCheckChanceWarning = true;
}
compressionOptions.remove(CRC_CHECK_CHANCE);
}
try
{
Method method = compressorClass.getMethod("create", Map.class);
@ -500,23 +473,6 @@ public final class CompressionParams
return String.valueOf(chunkLength() / 1024);
}
public void setCrcCheckChance(double crcCheckChance)
{
this.crcCheckChance = crcCheckChance;
}
public double getCrcCheckChance()
{
return crcCheckChance;
}
public boolean shouldCheckCrc()
{
double checkChance = getCrcCheckChance();
return checkChance >= 1d ||
(checkChance > 0d && checkChance > ThreadLocalRandom.current().nextDouble());
}
@Override
public boolean equals(Object obj)
{

1
src/java/org/apache/cassandra/schema/TableMetadataRef.java
View File

@ -33,7 +33,6 @@ public final class TableMetadataRef
private volatile TableMetadata metadata;
private volatile TableMetadata localTableMetadata;
private volatile CompressionParams localCompressionParams;
TableMetadataRef(TableMetadata metadata)
{

38
test/unit/org/apache/cassandra/cql3/validation/miscellaneous/CrcCheckChanceTest.java
View File

@ -36,31 +36,12 @@ import org.apache.cassandra.io.sstable.format.SSTableReader;
import org.apache.cassandra.io.util.RandomAccessReader;
import org.apache.cassandra.utils.FBUtilities;
public class CrcCheckChanceTest extends CQLTester
{
@Test
public void testChangingCrcCheckChanceNewFormat() throws Throwable
public void testChangingCrcCheckChance()
{
testChangingCrcCheckChance(true);
}
@Test
public void testChangingCrcCheckChanceOldFormat() throws Throwable
{
testChangingCrcCheckChance(false);
}
public void testChangingCrcCheckChance(boolean newFormat) throws Throwable
{
//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;");
execute("CREATE INDEX foo ON %s(v)");
@ -79,10 +60,7 @@ public class CrcCheckChanceTest extends CQLTester
Assert.assertEquals(0.99, indexCfs.getLiveSSTables().iterator().next().getCrcCheckChance(), 0.0);
//Test for stack overflow
if (newFormat)
alterTable("ALTER TABLE %s WITH crc_check_chance = 0.99");
else
alterTable("ALTER TABLE %s WITH compression = {'class': 'LZ4Compressor', 'crc_check_chance': 0.99}");
alterTable("ALTER TABLE %s WITH crc_check_chance = 0.99");
assertRows(execute("SELECT * FROM %s WHERE p=?", "p1"),
row("p1", "k1", "sv1", "v1"),
@ -131,10 +109,7 @@ public class CrcCheckChanceTest extends CQLTester
);
//Alter again via schema
if (newFormat)
alterTable("ALTER TABLE %s WITH crc_check_chance = 0.5");
else
alterTable("ALTER TABLE %s WITH compression = {'class': 'LZ4Compressor', 'crc_check_chance': 0.5}");
alterTable("ALTER TABLE %s WITH crc_check_chance = 0.5");
//We should be able to get the new value by accessing directly the schema metadata
Assert.assertEquals(0.5, cfs.metadata().params.crcCheckChance, 0.0);
@ -168,12 +143,12 @@ public class CrcCheckChanceTest extends CQLTester
}
@Test
public void testDropDuringCompaction() throws Throwable
public void testDropDuringCompaction()
{
CompactionManager.instance.disableAutoCompaction();
//Start with crc_check_chance of 99%
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");
ColumnFamilyStore cfs = Keyspace.open(CQLTester.KEYSPACE).getColumnFamilyStore(currentTable());
@ -202,4 +177,3 @@ public class CrcCheckChanceTest extends CQLTester
}
}
}