adelapena commented on code in PR #1891:
URL: https://github.com/apache/cassandra/pull/1891#discussion_r1183939639


##########
src/java/org/apache/cassandra/io/sstable/format/big/BigFormat.java:
##########
@@ -69,11 +73,19 @@ public class BigFormat extends 
AbstractSSTableFormat<BigTableReader, BigTableWri
 {
     private final static Logger logger = 
LoggerFactory.getLogger(BigFormat.class);
 
+    // Feature flag env variable to use extended TTL up to 2106 with the new 
oa sstable format. See c14227
+    public static enum TTL_MODE {LEGACY_TTL_2038, COMPATIBILITY, 
EXTENDED_TTL_2106};
+    @VisibleForTesting
+    public static final String USE_OA_SSTABLE_FORMAT = 
"cassandra.use_oa_sstable_format";
+    private static final String ttlFeatureFlagValue = 
System.getProperty(USE_OA_SSTABLE_FORMAT, "").toUpperCase();
+    public final static TTL_MODE ttlMode = 
EnumUtils.getEnumMap(TTL_MODE.class).keySet().contains(ttlFeatureFlagValue) ? 
TTL_MODE.valueOf(ttlFeatureFlagValue)
+                                                                               
                                        : TTL_MODE.LEGACY_TTL_2038;
+
     public static final BigFormat instance = new BigFormat();
 
     private final Version latestVersion = new BigVersion(this, 
BigVersion.current_version);
     private final BigTableReaderFactory readerFactory = new 
BigTableReaderFactory();
-    private final BigTableWriterFactory writerFactory = new 
BigTableWriterFactory();
+    private final BigTableWriterFactory writerFactory = new 
BigTableWriterFactory(); 

Review Comment:
   Nit: extra whitespace at the end of the line



##########
src/java/org/apache/cassandra/io/sstable/format/big/BigFormat.java:
##########
@@ -69,11 +73,19 @@ public class BigFormat extends 
AbstractSSTableFormat<BigTableReader, BigTableWri
 {
     private final static Logger logger = 
LoggerFactory.getLogger(BigFormat.class);
 
+    // Feature flag env variable to use extended TTL up to 2106 with the new 
oa sstable format. See c14227
+    public static enum TTL_MODE {LEGACY_TTL_2038, COMPATIBILITY, 
EXTENDED_TTL_2106};
+    @VisibleForTesting
+    public static final String USE_OA_SSTABLE_FORMAT = 
"cassandra.use_oa_sstable_format";
+    private static final String ttlFeatureFlagValue = 
System.getProperty(USE_OA_SSTABLE_FORMAT, "").toUpperCase();
+    public final static TTL_MODE ttlMode = 
EnumUtils.getEnumMap(TTL_MODE.class).keySet().contains(ttlFeatureFlagValue) ? 
TTL_MODE.valueOf(ttlFeatureFlagValue)

Review Comment:
   This can use `containsKey` instead of `keySet`+`contains`. Anyway, I think 
that `CassandraRelevantProperties` has utility methods for this. Those methods 
can also be used to accept case insensitive property values, which might be 
useful.



##########
src/java/org/apache/cassandra/net/MessagingService.java:
##########
@@ -211,8 +213,9 @@ public class MessagingService extends 
MessagingServiceMBeanImpl
     public static final int VERSION_30 = 10;
     public static final int VERSION_3014 = 11;
     public static final int VERSION_40 = 12;
+    public static final int VERSION_50 = 13; // c14227 TTL overflow, 'uint' 
timestamps
     public static final int minimum_version = VERSION_30;
-    public static final int current_version = VERSION_40;
+    public static final int current_version = 
BigFormat.ttlMode.equals(TTL_MODE.LEGACY_TTL_2038) ? VERSION_40 : VERSION_50;

Review Comment:
   ```suggestion
       public static final int current_version = BigFormat.ttlMode == 
TTL_MODE.LEGACY_TTL_2038 ? VERSION_40 : VERSION_50;
   ```



##########
src/java/org/apache/cassandra/io/sstable/format/big/BigFormat.java:
##########
@@ -69,11 +73,19 @@ public class BigFormat extends 
AbstractSSTableFormat<BigTableReader, BigTableWri
 {
     private final static Logger logger = 
LoggerFactory.getLogger(BigFormat.class);
 
+    // Feature flag env variable to use extended TTL up to 2106 with the new 
oa sstable format. See c14227
+    public static enum TTL_MODE {LEGACY_TTL_2038, COMPATIBILITY, 
EXTENDED_TTL_2106};
+    @VisibleForTesting
+    public static final String USE_OA_SSTABLE_FORMAT = 
"cassandra.use_oa_sstable_format";

Review Comment:
   I'm not sure whether the name of the property should be 
`use_oa_sstable_format`, when its accepted values are `LEGACY_TTL_2038`, 
`COMPATIBILITY` and `EXTENDED_TTL_2106`. 
   
   It seems that using `oa` or not is a consequence of the chosen TTL mode. 
Perhaps the property should be named just `ttl_mode`? So the sstable format 
will depend on the chosen ttl mode.



##########
src/java/org/apache/cassandra/io/sstable/format/big/BigFormat.java:
##########
@@ -363,7 +375,7 @@ public BigTableWriter.Builder builder(Descriptor descriptor)
     // we always incremented the major version.
     static class BigVersion extends Version
     {
-        public static final String current_version = "nc";
+        public static final String current_version = 
BigFormat.ttlMode.equals(TTL_MODE.LEGACY_TTL_2038) ? "nc": "oa";

Review Comment:
   ```suggestion
           public static final String current_version = BigFormat.ttlMode == 
TTL_MODE.LEGACY_TTL_2038 ? "nc" : "oa";
   ```



##########
src/java/org/apache/cassandra/io/sstable/format/big/BigFormat.java:
##########
@@ -69,11 +73,19 @@ public class BigFormat extends 
AbstractSSTableFormat<BigTableReader, BigTableWri
 {
     private final static Logger logger = 
LoggerFactory.getLogger(BigFormat.class);
 
+    // Feature flag env variable to use extended TTL up to 2106 with the new 
oa sstable format. See c14227
+    public static enum TTL_MODE {LEGACY_TTL_2038, COMPATIBILITY, 
EXTENDED_TTL_2106};
+    @VisibleForTesting
+    public static final String USE_OA_SSTABLE_FORMAT = 
"cassandra.use_oa_sstable_format";
+    private static final String ttlFeatureFlagValue = 
System.getProperty(USE_OA_SSTABLE_FORMAT, "").toUpperCase();

Review Comment:
   +1 to move this to `CassandraRelevantProperties`.



##########
src/java/org/apache/cassandra/io/sstable/format/big/BigFormat.java:
##########
@@ -69,11 +73,19 @@ public class BigFormat extends 
AbstractSSTableFormat<BigTableReader, BigTableWri
 {
     private final static Logger logger = 
LoggerFactory.getLogger(BigFormat.class);
 
+    // Feature flag env variable to use extended TTL up to 2106 with the new 
oa sstable format. See c14227
+    public static enum TTL_MODE {LEGACY_TTL_2038, COMPATIBILITY, 
EXTENDED_TTL_2106};

Review Comment:
   ```suggestion
       public enum TTL_MODE {LEGACY_TTL_2038, COMPATIBILITY, EXTENDED_TTL_2106}
   ```



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