Repository: cassandra Updated Branches: refs/heads/trunk 587c67e3b -> 17a358c2c
Add more testing of uncompressed chunks (CASSANDRA-10520), fix problem with min_compress_ratio: 1 and disallow ratio < 1 patch by Branimir Lambov; reviewed by Dimitar Dimitrov for CASSANDRA-13703 Project: http://git-wip-us.apache.org/repos/asf/cassandra/repo Commit: http://git-wip-us.apache.org/repos/asf/cassandra/commit/17a358c2 Tree: http://git-wip-us.apache.org/repos/asf/cassandra/tree/17a358c2 Diff: http://git-wip-us.apache.org/repos/asf/cassandra/diff/17a358c2 Branch: refs/heads/trunk Commit: 17a358c2cc2c583c3e0fa046ca8dee6d743ad1c5 Parents: 587c67e Author: Branimir Lambov <branimir.lam...@datastax.com> Authored: Thu Jul 20 18:07:49 2017 +0300 Committer: Branimir Lambov <branimir.lam...@datastax.com> Committed: Fri Sep 15 11:14:18 2017 +0300 ---------------------------------------------------------------------- CHANGES.txt | 1 + .../io/compress/CompressedSequentialWriter.java | 4 ++-- .../io/util/CompressedChunkReader.java | 13 ++++++------ .../cassandra/schema/CompressionParams.java | 21 +++++++++++++++++--- .../cql3/validation/operations/AlterTest.java | 12 +++++++++++ .../cql3/validation/operations/CreateTest.java | 12 ++++++++++- 6 files changed, 50 insertions(+), 13 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/cassandra/blob/17a358c2/CHANGES.txt ---------------------------------------------------------------------- diff --git a/CHANGES.txt b/CHANGES.txt index fbd08e5..afd1c7f 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,4 +1,5 @@ 4.0 + * Fix problem with min_compress_ratio: 1 and disallow ratio < 1 (CASSANDRA-13703) * Add extra information to SASI timeout exception (CASSANDRA-13677) * Add incremental repair support for --hosts, --force, and subrange repair (CASSANDRA-13818) * Rework CompactionStrategyManager.getScanners synchronization (CASSANDRA-13786) http://git-wip-us.apache.org/repos/asf/cassandra/blob/17a358c2/src/java/org/apache/cassandra/io/compress/CompressedSequentialWriter.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/io/compress/CompressedSequentialWriter.java b/src/java/org/apache/cassandra/io/compress/CompressedSequentialWriter.java index b4ea61f..8955d4f 100644 --- a/src/java/org/apache/cassandra/io/compress/CompressedSequentialWriter.java +++ b/src/java/org/apache/cassandra/io/compress/CompressedSequentialWriter.java @@ -151,7 +151,7 @@ public class CompressedSequentialWriter extends SequentialWriter int compressedLength = compressed.position(); uncompressedSize += buffer.position(); ByteBuffer toWrite = compressed; - if (compressedLength > maxCompressedLength) + if (compressedLength >= maxCompressedLength) { toWrite = buffer; compressedLength = buffer.position(); @@ -240,7 +240,7 @@ public class CompressedSequentialWriter extends SequentialWriter // Repopulate buffer from compressed data buffer.clear(); compressed.flip(); - if (chunkSize <= maxCompressedLength) + if (chunkSize < maxCompressedLength) compressor.uncompress(compressed, buffer); else buffer.put(compressed); http://git-wip-us.apache.org/repos/asf/cassandra/blob/17a358c2/src/java/org/apache/cassandra/io/util/CompressedChunkReader.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/io/util/CompressedChunkReader.java b/src/java/org/apache/cassandra/io/util/CompressedChunkReader.java index 15f9fa0..5ae083b 100644 --- a/src/java/org/apache/cassandra/io/util/CompressedChunkReader.java +++ b/src/java/org/apache/cassandra/io/util/CompressedChunkReader.java @@ -20,7 +20,6 @@ package org.apache.cassandra.io.util; import java.io.IOException; import java.nio.ByteBuffer; -import java.util.concurrent.ThreadLocalRandom; import com.google.common.annotations.VisibleForTesting; import com.google.common.primitives.Ints; @@ -50,9 +49,9 @@ public abstract class CompressedChunkReader extends AbstractReaderFileProxy impl return metadata.parameters.getCrcCheckChance(); } - public boolean maybeCheckCrc() + boolean shouldCheckCrc() { - return metadata.parameters.maybeCheckCrc(); + return metadata.parameters.shouldCheckCrc(); } @Override @@ -116,7 +115,7 @@ public abstract class CompressedChunkReader extends AbstractReaderFileProxy impl assert position <= fileLength; CompressionMetadata.Chunk chunk = metadata.chunkFor(position); - if (chunk.length <= maxCompressedLength) + if (chunk.length < maxCompressedLength) { ByteBuffer compressed = compressedHolder.get(); assert compressed.capacity() >= chunk.length; @@ -156,7 +155,7 @@ public abstract class CompressedChunkReader extends AbstractReaderFileProxy impl void maybeCheckCrc(CompressionMetadata.Chunk chunk, ByteBuffer content) throws CorruptBlockException { - if (metadata.parameters.maybeCheckCrc()) + if (shouldCheckCrc()) { content.flip(); int checksum = (int) ChecksumType.CRC32.of(content); @@ -202,7 +201,7 @@ public abstract class CompressedChunkReader extends AbstractReaderFileProxy impl try { - if (chunk.length <= maxCompressedLength) + if (chunk.length < maxCompressedLength) metadata.compressor().uncompress(compressedChunk, uncompressed); else uncompressed.put(compressedChunk); @@ -213,7 +212,7 @@ public abstract class CompressedChunkReader extends AbstractReaderFileProxy impl } uncompressed.flip(); - if (maybeCheckCrc()) + if (shouldCheckCrc()) { compressedChunk.position(chunkOffset).limit(chunkOffset + chunk.length); http://git-wip-us.apache.org/repos/asf/cassandra/blob/17a358c2/src/java/org/apache/cassandra/schema/CompressionParams.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/schema/CompressionParams.java b/src/java/org/apache/cassandra/schema/CompressionParams.java index c4b52df..2d60d13 100644 --- a/src/java/org/apache/cassandra/schema/CompressionParams.java +++ b/src/java/org/apache/cassandra/schema/CompressionParams.java @@ -25,6 +25,7 @@ import java.util.HashMap; import java.util.Map; import java.util.concurrent.ThreadLocalRandom; +import com.google.common.annotations.VisibleForTesting; import com.google.common.collect.ImmutableMap; import org.apache.commons.lang3.builder.EqualsBuilder; @@ -125,41 +126,52 @@ public final class CompressionParams return new CompressionParams((ICompressor) null, DEFAULT_CHUNK_LENGTH, Integer.MAX_VALUE, 0.0, Collections.emptyMap()); } + // The shorthand methods below are only used for tests. They are a little inconsistent in their choice of + // parameters -- this is done on purpose to test out various compression parameter combinations. + + @VisibleForTesting public static CompressionParams snappy() { return snappy(DEFAULT_CHUNK_LENGTH); } + @VisibleForTesting public static CompressionParams snappy(int chunkLength) { - return snappy(chunkLength, DEFAULT_MIN_COMPRESS_RATIO); + return snappy(chunkLength, 1.1); } + @VisibleForTesting public static CompressionParams snappy(int chunkLength, double minCompressRatio) { return new CompressionParams(SnappyCompressor.instance, chunkLength, calcMaxCompressedLength(chunkLength, minCompressRatio), minCompressRatio, Collections.emptyMap()); } + @VisibleForTesting public static CompressionParams deflate() { return deflate(DEFAULT_CHUNK_LENGTH); } + @VisibleForTesting public static CompressionParams deflate(int chunkLength) { return new CompressionParams(DeflateCompressor.instance, chunkLength, Integer.MAX_VALUE, 0.0, Collections.emptyMap()); } + @VisibleForTesting public static CompressionParams lz4() { return lz4(DEFAULT_CHUNK_LENGTH); } + @VisibleForTesting public static CompressionParams lz4(int chunkLength) { - return lz4(chunkLength, Integer.MAX_VALUE); + return lz4(chunkLength, chunkLength); } + @VisibleForTesting public static CompressionParams lz4(int chunkLength, int maxCompressedLength) { return new CompressionParams(LZ4Compressor.create(Collections.emptyMap()), chunkLength, maxCompressedLength, calcMinCompressRatio(chunkLength, maxCompressedLength), Collections.emptyMap()); @@ -491,6 +503,9 @@ public final class CompressionParams if (maxCompressedLength < 0) throw new ConfigurationException("Invalid negative " + MIN_COMPRESS_RATIO); + + if (maxCompressedLength > chunkLength && maxCompressedLength < Integer.MAX_VALUE) + throw new ConfigurationException(MIN_COMPRESS_RATIO + " can either be 0 or greater than or equal to 1"); } public Map<String, String> asMap() @@ -522,7 +537,7 @@ public final class CompressionParams return crcCheckChance; } - public boolean maybeCheckCrc() + public boolean shouldCheckCrc() { double checkChance = getCrcCheckChance(); return checkChance > 0d && checkChance > ThreadLocalRandom.current().nextDouble(); http://git-wip-us.apache.org/repos/asf/cassandra/blob/17a358c2/test/unit/org/apache/cassandra/cql3/validation/operations/AlterTest.java ---------------------------------------------------------------------- diff --git a/test/unit/org/apache/cassandra/cql3/validation/operations/AlterTest.java b/test/unit/org/apache/cassandra/cql3/validation/operations/AlterTest.java index 9cfb3c9..c6f255a 100644 --- a/test/unit/org/apache/cassandra/cql3/validation/operations/AlterTest.java +++ b/test/unit/org/apache/cassandra/cql3/validation/operations/AlterTest.java @@ -376,6 +376,15 @@ public class AlterTest extends CQLTester currentTable()), row(map("chunk_length_in_kb", "64", "class", "org.apache.cassandra.io.compress.LZ4Compressor", "min_compress_ratio", "2.0"))); + execute("ALTER TABLE %s WITH compression = { 'sstable_compression' : 'LZ4Compressor', 'min_compress_ratio' : 1 };"); + + assertRows(execute(format("SELECT compression FROM %s.%s WHERE keyspace_name = ? and table_name = ?;", + SchemaConstants.SCHEMA_KEYSPACE_NAME, + SchemaKeyspace.TABLES), + KEYSPACE, + currentTable()), + row(map("chunk_length_in_kb", "64", "class", "org.apache.cassandra.io.compress.LZ4Compressor", "min_compress_ratio", "1.0"))); + execute("ALTER TABLE %s WITH compression = { 'sstable_compression' : 'LZ4Compressor', 'min_compress_ratio' : 0 };"); assertRows(execute(format("SELECT compression FROM %s.%s WHERE keyspace_name = ? and table_name = ?;", @@ -421,6 +430,9 @@ public class AlterTest extends CQLTester assertThrowsConfigurationException("Invalid negative min_compress_ratio", "ALTER TABLE %s WITH compression = { 'class' : 'SnappyCompressor', 'min_compress_ratio' : -1 };"); + + assertThrowsConfigurationException("min_compress_ratio can either be 0 or greater than or equal to 1", + "ALTER TABLE %s WITH compression = { 'class' : 'SnappyCompressor', 'min_compress_ratio' : 0.5 };"); } private void assertThrowsConfigurationException(String errorMsg, String alterStmt) throws Throwable http://git-wip-us.apache.org/repos/asf/cassandra/blob/17a358c2/test/unit/org/apache/cassandra/cql3/validation/operations/CreateTest.java ---------------------------------------------------------------------- diff --git a/test/unit/org/apache/cassandra/cql3/validation/operations/CreateTest.java b/test/unit/org/apache/cassandra/cql3/validation/operations/CreateTest.java index 7eccf13..e3c66c5 100644 --- a/test/unit/org/apache/cassandra/cql3/validation/operations/CreateTest.java +++ b/test/unit/org/apache/cassandra/cql3/validation/operations/CreateTest.java @@ -785,7 +785,17 @@ public class CreateTest extends CQLTester row(map("chunk_length_in_kb", "64", "class", "org.apache.cassandra.io.compress.SnappyCompressor", "min_compress_ratio", "2.0"))); createTable("CREATE TABLE %s (a text, b int, c int, primary key (a, b))" - + " WITH compression = { 'sstable_compression' : 'SnappyCompressor', 'min_compress_ratio' : 0 };"); + + " WITH compression = { 'sstable_compression' : 'SnappyCompressor', 'min_compress_ratio' : 1 };"); + + assertRows(execute(format("SELECT compression FROM %s.%s WHERE keyspace_name = ? and table_name = ?;", + SchemaConstants.SCHEMA_KEYSPACE_NAME, + SchemaKeyspace.TABLES), + KEYSPACE, + currentTable()), + row(map("chunk_length_in_kb", "64", "class", "org.apache.cassandra.io.compress.SnappyCompressor", "min_compress_ratio", "1.0"))); + + createTable("CREATE TABLE %s (a text, b int, c int, primary key (a, b))" + + " WITH compression = { 'sstable_compression' : 'SnappyCompressor', 'min_compress_ratio' : 0 };"); assertRows(execute(format("SELECT compression FROM %s.%s WHERE keyspace_name = ? and table_name = ?;", SchemaConstants.SCHEMA_KEYSPACE_NAME, --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org
