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


##########
src/java/org/apache/cassandra/db/ColumnFamilyStore.java:
##########
@@ -3078,6 +3078,16 @@ public boolean isAutoSnapshotEnabled()
         return metadata().params.allowAutoSnapshot && 
DatabaseDescriptor.isAutoSnapshot();
     }
 
+    public DurationSpec.IntSecondsBound getAutoSnapshotTtl()
+    {
+        // If it is not set as a table parameter (which is by default 0s), 
then take the default value from descriptor.
+        // It is possible that it is unset in descriptor, which again defaults 
to 0s.
+        // When SnapshotManifest receives ttl with null or 0s, it will not 
write expiration time to manifest file
+        // Hence, table parameter has precedence over descriptor value.
+        DurationSpec.IntSecondsBound ttlFromTable = 
metadata().params.autoSnapshotTtl;

Review Comment:
   `autoSnapshotTtl` is not null because we need to have non-null value when 
e.g. describing a table. It is way easier to work with non-null values. If 
`DatabaseDescriptor.getAutoSnapshotTtl` was null, we would have non-null ttl 
from params which might be of value 0s or we would possibly return null from 
descriptor.
   
   I think that this method should return non-null in every case and making 
special logic about nullity is cumbersome. Hence I prefer to have it non-null 
every time.



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