Claudenw commented on code in PR #2282:
URL: https://github.com/apache/cassandra/pull/2282#discussion_r1269074893
##########
src/java/org/apache/cassandra/schema/CompressionParams.java:
##########
@@ -43,189 +44,246 @@
import org.apache.cassandra.io.util.DataInputPlus;
import org.apache.cassandra.io.util.DataOutputPlus;
import org.apache.cassandra.net.MessagingService;
+import org.apache.cassandra.utils.FBUtilities;
import static java.lang.String.format;
+import static java.util.Collections.emptyMap;
+import static
org.apache.cassandra.config.CassandraRelevantProperties.DETERMINISM_SSTABLE_COMPRESSION_DEFAULT;
-@SuppressWarnings("deprecation")
public final class CompressionParams
{
- private static final Logger logger =
LoggerFactory.getLogger(CompressionParams.class);
-
- private static volatile boolean hasLoggedSsTableCompressionWarning;
- private static volatile boolean hasLoggedChunkLengthWarning;
- private static volatile boolean hasLoggedCrcCheckChanceWarning;
-
- public static final int DEFAULT_CHUNK_LENGTH = 1024 * 16;
+ public static final CompressorType DEFAULT_COMPRESSION_TYPE =
CompressorType.lz4;
+ public static final int DEFAULT_CHUNK_LENGTH = 1024 * 16; // in KB
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
//
different default value.
public static final IVersionedSerializer<CompressionParams> serializer =
new Serializer();
public static final String CLASS = "class";
public static final String CHUNK_LENGTH_IN_KB = "chunk_length_in_kb";
+ /**
+ * Requires a DataStorageSpec suffix
+ */
+ public static final String CHUNK_LENGTH = "chunk_length";
+ /**
+ * Requires a DataStorageSpec suffix
+ */
+ public static final String MAX_COMPRESSED_LENGTH = "max_compressed_length";
public static final String ENABLED = "enabled";
public static final String MIN_COMPRESS_RATIO = "min_compress_ratio";
- public static final CompressionParams DEFAULT =
!CassandraRelevantProperties.DETERMINISM_SSTABLE_COMPRESSION_DEFAULT.getBoolean()
- ? noCompression()
- : new
CompressionParams(LZ4Compressor.create(Collections.emptyMap()),
-
DEFAULT_CHUNK_LENGTH,
-
calcMaxCompressedLength(DEFAULT_CHUNK_LENGTH, DEFAULT_MIN_COMPRESS_RATIO),
-
DEFAULT_MIN_COMPRESS_RATIO,
-
Collections.emptyMap());
-
- public static final CompressionParams NOOP = new
CompressionParams(NoopCompressor.create(Collections.emptyMap()),
+ public static final CompressionParams NOOP = new
CompressionParams(NoopCompressor.create(emptyMap()),
// 4
KiB is often the underlying disk block size
1024 *
4,
Integer.MAX_VALUE,
DEFAULT_MIN_COMPRESS_RATIO,
-
Collections.emptyMap());
+
emptyMap());
+
+ // The default compressor is generally fast (LZ4 with 16KiB block size)
+ public static final CompressionParams DEFAULT = new
CompressionParams(LZ4Compressor.create(Collections.emptyMap()),
+
DEFAULT_CHUNK_LENGTH,
+
calcMaxCompressedLength(DEFAULT_CHUNK_LENGTH, DEFAULT_MIN_COMPRESS_RATIO),
+
DEFAULT_MIN_COMPRESS_RATIO,
+
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";
+ private static CompressionParams CALCULATED_DEFAULT;
- @Deprecated public static final String SSTABLE_COMPRESSION =
"sstable_compression";
- @Deprecated public static final String CHUNK_LENGTH_KB = "chunk_length_kb";
- @Deprecated public static final String CRC_CHECK_CHANCE =
"crc_check_chance";
+ @VisibleForTesting
+ static final String TOO_MANY_CHUNK_LENGTH = format("Only one of '%s' or
'%s' may be specified", CHUNK_LENGTH, CHUNK_LENGTH_IN_KB);
private final ICompressor sstableCompressor;
+ /**
+ * The chunk length in KB
+ */
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.
+ /**
+ * The compressed length in KB.
+ * In content we store max length to avoid rounding errors causing
compress/decompress mismatch.
+ */
+ private final int maxCompressedLength;
+ /**
+ * The minimum compression ratio.
+ * In configuration we store min ratio, the input parameter.
+ * Ths is mathematically related to chunkLength and maxCompressedLength in
that
+ * # chunk_length / max_compressed_length = min_compress_ratio
+ */
+ private final double minCompressRatio;
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);
- String sstableCompressionClass;
+ public enum CompressorType
+ {
+ lz4(LZ4Compressor.class.getName(), LZ4Compressor::create),
+ noop(NoopCompressor.class.getName(), NoopCompressor::create),
+ snappy(SnappyCompressor.class.getName(), SnappyCompressor::create),
+ deflate(DeflateCompressor.class.getName(), DeflateCompressor::create),
+ zstd(ZstdCompressor.class.getName(), ZstdCompressor::create),
+ none(null, (opt) -> null);
- if (!opts.isEmpty() && isEnabled(opts) &&
!containsSstableCompressionClass(opts))
- throw new ConfigurationException(format("Missing sub-option '%s'
for the 'compression' option.", CLASS));
+ final String className;
+ final Function<Map<String,String>,ICompressor> creator;
- if (!removeEnabled(options))
+ CompressorType(String className,
Function<Map<String,String>,ICompressor> creator)
{
- sstableCompressionClass = null;
-
- if (!options.isEmpty())
- throw new ConfigurationException(format("If the '%s' option is
set to false no other options must be specified", ENABLED));
+ this.className = className;
+ this.creator = creator;
}
- else
+
+ static CompressorType forClass(String name)
{
- sstableCompressionClass = removeSstableCompressionClass(options);
+ if (name == null)
+ return none;
+
+ for (CompressorType type : CompressorType.values())
+ {
+ if (Objects.equal(type.className, name))
+ return type;
+ }
+ return null;
}
- int chunkLength = removeChunkLength(options);
- double minCompressRatio = removeMinCompressRatio(options);
+ public ICompressor create(Map<String,String> options) {
+ return creator.apply(options);
+ }
+ }
- CompressionParams cp = new CompressionParams(sstableCompressionClass,
options, chunkLength, minCompressRatio);
- cp.validate();
+ public static CompressionParams defaultParams()
+ {
+ CompressionParams result = CALCULATED_DEFAULT;
+ if (result == null)
+ result = CALCULATED_DEFAULT =
fromParameterizedClass(DatabaseDescriptor.getSSTableCompression());
- return cp;
+ return result;
}
- public Class<? extends ICompressor> klass()
+ public static CompressionParams fromParameterizedClass(ParameterizedClass
options)
{
- return sstableCompressor.getClass();
+ if (options == null)
+ return DETERMINISM_SSTABLE_COMPRESSION_DEFAULT.getBoolean() ?
DEFAULT : noCompression();
+
+ return fromClassAndOptions(options.class_name, options.parameters ==
null ? emptyMap() : copyOptions(options.parameters));
}
- public static CompressionParams noCompression()
+ public static CompressionParams fromMap(Map<String, String> opts)
{
- return new CompressionParams((ICompressor) null, DEFAULT_CHUNK_LENGTH,
Integer.MAX_VALUE, 0.0, Collections.emptyMap());
- }
+ Map<String, String> options = copyOptions(opts);
- // 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.
+ String sstableCompressionClass =
removeSstableCompressionClass(options);
- @VisibleForTesting
- public static CompressionParams snappy()
- {
- return snappy(DEFAULT_CHUNK_LENGTH);
+ return fromClassAndOptions(sstableCompressionClass, options);
}
- @VisibleForTesting
- public static CompressionParams snappy(int chunkLength)
+ public static CompressionParams noCompression()
{
- return snappy(chunkLength, 1.1);
+ return new CompressionParams(null, DEFAULT_CHUNK_LENGTH,
Integer.MAX_VALUE, DEFAULT_MIN_COMPRESS_RATIO, emptyMap());
}
- @VisibleForTesting
- public static CompressionParams snappy(int chunkLength, double
minCompressRatio)
+ static int calcMaxCompressedLength(int chunkLength, double
minCompressRatio)
{
- return new CompressionParams(SnappyCompressor.instance, chunkLength,
calcMaxCompressedLength(chunkLength, minCompressRatio), minCompressRatio,
Collections.emptyMap());
+ return (int) Math.ceil(Math.min(chunkLength / minCompressRatio,
Integer.MAX_VALUE));
}
- @VisibleForTesting
- public static CompressionParams deflate()
+ static double calcMinCompressRatio(int chunkLength, int
maxCompressedLength)
{
- return deflate(DEFAULT_CHUNK_LENGTH);
+ if (maxCompressedLength == Integer.MAX_VALUE)
+ return 0;
+ return chunkLength * 1.0 / maxCompressedLength;
}
- @VisibleForTesting
- public static CompressionParams deflate(int chunkLength)
+ private static CompressionParams fromClassAndOptions(String
sstableCompressionClass, Map<String,String> options)
{
- return new CompressionParams(DeflateCompressor.instance, chunkLength,
Integer.MAX_VALUE, 0.0, Collections.emptyMap());
- }
+ final boolean enabled = removeEnabled(options);
- @VisibleForTesting
- public static CompressionParams lz4()
- {
- return lz4(DEFAULT_CHUNK_LENGTH);
- }
+ if (options.containsKey(CHUNK_LENGTH_IN_KB) &&
options.containsKey(CHUNK_LENGTH))
+ throw new ConfigurationException(format(TOO_MANY_CHUNK_LENGTH,
+ CHUNK_LENGTH_IN_KB,
CHUNK_LENGTH));
- @VisibleForTesting
- public static CompressionParams lz4(int chunkLength)
- {
- return lz4(chunkLength, chunkLength);
- }
+ final int chunk_length_in_bytes = removeChunkLength(options);
- @VisibleForTesting
- public static CompressionParams lz4(int chunkLength, int
maxCompressedLength)
- {
- return new
CompressionParams(LZ4Compressor.create(Collections.emptyMap()), chunkLength,
maxCompressedLength, calcMinCompressRatio(chunkLength, maxCompressedLength),
Collections.emptyMap());
- }
+ // 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(format("Can not specify both '%s'
and '%s' for the compressor parameters.",
+ MIN_COMPRESS_RATIO,
MAX_COMPRESSED_LENGTH));
- public static CompressionParams zstd()
- {
- return zstd(DEFAULT_CHUNK_LENGTH);
- }
+ // calculate the max_compressed_length and min_compress_ratio
+ int max_compressed_length_in_bytes;
+ 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_in_bytes = new
DataStorageSpec.IntKibibytesBound(max_compressed_length_str).toBytes();
+ validateMaxCompressedLength(max_compressed_length_in_bytes,
chunk_length_in_bytes);
+ min_compress_ratio =
CompressionParams.calcMinCompressRatio(chunk_length_in_bytes,
max_compressed_length_in_bytes);
+ } 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_in_bytes =
CompressionParams.calcMaxCompressedLength(chunk_length_in_bytes,
min_compress_ratio);
+ }
- public static CompressionParams zstd(Integer chunkLength)
- {
- ZstdCompressor compressor =
ZstdCompressor.create(Collections.emptyMap());
- return new CompressionParams(compressor, chunkLength,
Integer.MAX_VALUE, DEFAULT_MIN_COMPRESS_RATIO, Collections.emptyMap());
- }
+ CompressorType compressorType = null;
+ try
+ {
+ compressorType = CompressorType.valueOf(sstableCompressionClass);
+ }
+ catch (Exception e)
+ {
+ // intentionally empty
+ }
- @VisibleForTesting
- public static CompressionParams noop()
- {
- NoopCompressor compressor =
NoopCompressor.create(Collections.emptyMap());
- return new CompressionParams(compressor, DEFAULT_CHUNK_LENGTH,
Integer.MAX_VALUE, DEFAULT_MIN_COMPRESS_RATIO, Collections.emptyMap());
+ Function<Map<String,String>, ICompressor> creator = compressorType !=
null ? compressorType.creator : (opt) -> {
+ if (sstableCompressionClass != null)
+ {
+ return
FBUtilities.newCompressor(parseCompressorClass(sstableCompressionClass), opt);
+ }
+ else
+ {
+ return
FBUtilities.newCompressor(parseCompressorClass(defaultParams().klass().getName()),
opt);
+ }
+ };
Review Comment:
THis block can be replaced with:
Function<Map<String,String>, ICompressor> creator = null;
// try to set compressor type
CompressorType compressorType = null;
if (!StringUtils.isEmpty(sstableCompressionClass))
{
try
{
compressorType =
CompressorType.valueOf(sstableCompressionClass);
}
catch (IllegalArgumentException expected)
{
compressorType =
CompressorType.forClass(sstableCompressionClass);
}
creator = compressorType != null ? compressorType.creator :
(opt) ->
FBUtilities.newCompressor(parseCompressorClass(sstableCompressionClass), opt);
} else {
creator = (opt) ->
FBUtilities.newCompressor(parseCompressorClass(defaultParams().klass().getName()),
opt);
}
This will reduce the reflection overhead on every lookup.
--
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]
