You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cassandra.apache.org by bl...@apache.org on 2017/09/15 08:15:55 UTC
cassandra git commit: Add more testing of uncompressed chunks
(CASSANDRA-10520),
fix problem with min_compress_ratio: 1 and disallow ratio < 1
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 <br...@datastax.com>
Authored: Thu Jul 20 18:07:49 2017 +0300
Committer: Branimir Lambov <br...@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-unsubscribe@cassandra.apache.org
For additional commands, e-mail: commits-help@cassandra.apache.org