smiklosovic commented on code in PR #2254:
URL: https://github.com/apache/cassandra/pull/2254#discussion_r1176676449


##########
src/java/org/apache/cassandra/schema/CompressionParams.java:
##########
@@ -55,35 +58,44 @@
     private static volatile boolean hasLoggedChunkLengthWarning;
     private static volatile boolean hasLoggedCrcCheckChanceWarning;
 
-    public static final int DEFAULT_CHUNK_LENGTH = 1024 * 16;
+    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()),
                                                                        // 4 
KiB is often the underlying disk block size
                                                                        1024 * 
4,
                                                                        
Integer.MAX_VALUE,
                                                                        
DEFAULT_MIN_COMPRESS_RATIO,
                                                                        
Collections.emptyMap());
 
+    private static final CompressionParams DEFAULT = new 
CompressionParams(LZ4Compressor.create(Collections.<String, String>emptyMap()),
+                                                                       
DEFAULT_CHUNK_LENGTH,
+                                                                       
calcMaxCompressedLength(DEFAULT_CHUNK_LENGTH, DEFAULT_MIN_COMPRESS_RATIO),
+                                                                       
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";
 
+    @VisibleForTesting
+    static final String TOO_MANY_CHUNK_LENGTH = "Only one of 'chunk_length', 
'chunk_length_kb', or 'chunk_length_in_kb' may be specified";
+
     @Deprecated public static final String SSTABLE_COMPRESSION = 
"sstable_compression";

Review Comment:
   @Claudenw I would remove all these deprecated options as part of this PR. At 
least `crc_check_chance` seems to be deprecated in 3.0 so it is eligible for 
deletion. I think that the rest is good to remove too but we would need to 
confirm this as git history is little bit unclear here.
   
   cc @michaelsembwever 



-- 
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]

Reply via email to