smiklosovic commented on code in PR #2254:
URL: https://github.com/apache/cassandra/pull/2254#discussion_r1176705606
##########
src/java/org/apache/cassandra/schema/CompressionParams.java:
##########
@@ -97,128 +109,185 @@
// 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);
-
- String sstableCompressionClass;
-
- if (!opts.isEmpty() && isEnabled(opts) &&
!containsSstableCompressionClass(opts))
- throw new ConfigurationException(format("Missing sub-option '%s'
for the 'compression' option.", CLASS));
- if (!removeEnabled(options))
- {
- sstableCompressionClass = null;
+ public enum CompressorType
+ {
+ lz4("LZ4Compressor"),
+ none(null),
+ noop("NoopCompressor"),
+ snappy("SnappyCompressor"),
+ deflate("DeflateCompressor"),
+ zstd("ZstdCompressor");
- if (!options.isEmpty())
- throw new ConfigurationException(format("If the '%s' option is
set to false no other options must be specified", ENABLED));
- }
- else
- {
- sstableCompressionClass = removeSstableCompressionClass(options);
+ String className;
+ CompressorType(String className) {
+ this.className = className;
}
- int chunkLength = removeChunkLength(options);
- double minCompressRatio = removeMinCompressRatio(options);
-
- CompressionParams cp = new CompressionParams(sstableCompressionClass,
options, chunkLength, minCompressRatio);
- cp.validate();
-
- return cp;
+ static CompressorType forClass(String name) {
+ for (CompressorType type : CompressorType.values()) {
+ if (Objects.equal(type.className, name)) {
+ return type;
+ }
+ }
+ return null;
+ }
}
- public Class<? extends ICompressor> klass()
+ public static CompressionParams defaultParams()
{
- return sstableCompressor.getClass();
+ return
fromParameterizedClass(DatabaseDescriptor.getSSTableCompressionOptions());
}
- public static CompressionParams noCompression()
+ public static CompressionParams fromParameterizedClass(ParameterizedClass
options)
{
- return new CompressionParams((ICompressor) null, DEFAULT_CHUNK_LENGTH,
Integer.MAX_VALUE, 0.0, Collections.emptyMap());
- }
+ if (options == null)
+ {
+ return
CassandraRelevantProperties.DETERMINISM_SSTABLE_COMPRESSION_DEFAULT.getBoolean()
+ ? DEFAULT
+ : noCompression();
+ }
- // 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.
+ if (options.parameters != null &&
options.parameters.containsKey(SSTABLE_COMPRESSION))
+ throw new ConfigurationException(format("The '%s' option must not
be used with the ParameterizedClass constructor",
+ SSTABLE_COMPRESSION));
- @VisibleForTesting
- public static CompressionParams snappy()
- {
- return snappy(DEFAULT_CHUNK_LENGTH);
+ return fromClassAndOptions(options.class_name, options.parameters ==
null ? Collections.emptyMap() : copyOptions(options.parameters));
}
- @VisibleForTesting
- public static CompressionParams snappy(int chunkLength)
+ private static String invalidValue( String param, Object value)
{
- return snappy(chunkLength, 1.1);
+ return format("Invalid '%s' value for the 'compression' option: %s",
param, value);
}
- @VisibleForTesting
- public static CompressionParams snappy(int chunkLength, double
minCompressRatio)
- {
- return new CompressionParams(SnappyCompressor.instance, chunkLength,
calcMaxCompressedLength(chunkLength, minCompressRatio), minCompressRatio,
Collections.emptyMap());
+ private static String invalidValue( String param, String extraText, Object
value) {
+ return format("Invalid '%s' value for the 'compression' option. %s:
%s", param, extraText, value );
}
- @VisibleForTesting
- public static CompressionParams deflate()
+ public static CompressionParams fromMap(Map<String, String> opts)
{
- return deflate(DEFAULT_CHUNK_LENGTH);
- }
+ Map<String, String> options = copyOptions(opts);
- @VisibleForTesting
- public static CompressionParams deflate(int chunkLength)
- {
- return new CompressionParams(DeflateCompressor.instance, chunkLength,
Integer.MAX_VALUE, 0.0, Collections.emptyMap());
- }
+ String sstableCompressionClass =
removeSstableCompressionClass(options);
- @VisibleForTesting
- public static CompressionParams lz4()
- {
- return lz4(DEFAULT_CHUNK_LENGTH);
+ return fromClassAndOptions(sstableCompressionClass, options);
}
- @VisibleForTesting
- public static CompressionParams lz4(int chunkLength)
+ private static CompressionParams fromClassAndOptions(String
sstableCompressionClass, Map<String,String> options)
{
- return lz4(chunkLength, chunkLength);
- }
+ boolean enabled = true;
- @VisibleForTesting
- public static CompressionParams lz4(int chunkLength, int
maxCompressedLength)
- {
- return new
CompressionParams(LZ4Compressor.create(Collections.emptyMap()), chunkLength,
maxCompressedLength, calcMinCompressRatio(chunkLength, maxCompressedLength),
Collections.emptyMap());
- }
+ if (!removeEnabled(options))
+ {
+ enabled = false;
+ }
- public static CompressionParams zstd()
- {
- return zstd(DEFAULT_CHUNK_LENGTH);
+ int chunk_length_in_kb = removeChunkLength(options);
+
+ // figure out how we calculate the max_compressed_length and
min_compress_ratio
+ if (options.containsKey(MIN_COMPRESS_RATIO) &&
options.containsKey(MAX_COMPRESSED_LENGTH))
+ {
+ throw new ConfigurationException("Can not specify both
'min_compress_ratio' and 'max_compressed_length' for the compressor
parameters.");
+ }
+
+ // calculate the max_compressed_length and min_compress_ratio
+ int max_compressed_length;
+ double min_compress_ratio;
+ String max_compressed_length_str = options.remove(
MAX_COMPRESSED_LENGTH);
+ if (!StringUtils.isBlank(max_compressed_length_str))
+ {
+ try
+ {
+ max_compressed_length = new
DataStorageSpec.IntKibibytesBound(max_compressed_length_str).toKibibytes();
+ validateMaxCompressedLength( max_compressed_length,
chunk_length_in_kb);
+ min_compress_ratio =
CompressionParams.calcMinCompressRatio(chunk_length_in_kb,
max_compressed_length);
+ } catch (IllegalArgumentException e) {
+ throw new ConfigurationException(invalidValue(
MAX_COMPRESSED_LENGTH, e.getMessage(), max_compressed_length_str ));
+ }
+ }
+ else
+ {
+ min_compress_ratio = removeMinCompressRatio(options);
+ validateMinCompressRatio( min_compress_ratio );
+ max_compressed_length =
CompressionParams.calcMaxCompressedLength(chunk_length_in_kb,min_compress_ratio);
+ }
+
+ // try to set compressor type
+ CompressorType compressorType =
StringUtils.isEmpty(sstableCompressionClass)?CompressorType.lz4:null;
+ if (compressorType == null)
+ {
+ try
+ {
+ compressorType =
CompressorType.valueOf(sstableCompressionClass);
+ }
+ catch (IllegalArgumentException expected)
+ {
+ compressorType =
CompressorType.forClass(sstableCompressionClass);
+ }
+ }
+
+ CompressionParams cp = null;
+ if (compressorType != null)
+ {
+
+ switch (compressorType)
+ {
+ case none:
+ cp = new CompressionParams((ICompressor) null,
chunk_length_in_kb, max_compressed_length, min_compress_ratio, options);
+ break;
+ case lz4:
+ cp = new
CompressionParams(enabled?LZ4Compressor.create(options):null,
chunk_length_in_kb, max_compressed_length, min_compress_ratio, options);
+ break;
+ case snappy:
+ cp = new
CompressionParams(enabled?SnappyCompressor.instance:null, chunk_length_in_kb,
max_compressed_length, min_compress_ratio, options);
Review Comment:
shouldnt be here `SnappyCompressor.create(options)` instead of
`SnappyCompressor.instance`? Same for DeflateCompressor.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]