You are viewing a plain text version of this content. The canonical link for it is here.
Posted to pr@cassandra.apache.org by "adelapena (via GitHub)" <gi...@apache.org> on 2023/05/03 17:04:46 UTC
[GitHub] [cassandra] adelapena commented on a diff in pull request #1891: Cassandra 14227 trunk
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org