You are viewing a plain text version of this content. The canonical link for it is here.
Posted to pr@cassandra.apache.org by GitBox <gi...@apache.org> on 2023/01/10 21:22:27 UTC

[GitHub] [cassandra] maedhroz commented on a diff in pull request #2051: Cassandra 18118 3.11

maedhroz commented on code in PR #2051:
URL: https://github.com/apache/cassandra/pull/2051#discussion_r1066359957


##########
src/java/org/apache/cassandra/db/Memtable.java:
##########
@@ -387,9 +397,16 @@ public Partition getPartition(DecoratedKey key)
         return partitions.get(key);
     }
 
+    /**
+     * Returns the minTS if one available, otherwise -1.
+     *
+     * EncodingStats uses a synthetic epoch TS at 2015. We don't want to leak that (CASSANDRA-18118) so we return -1 instead.
+     *
+     * @return The minTS or -1 if none available
+     */
     public long getMinTimestamp()
     {
-        return minTimestamp;
+        return minTimestamp != EncodingStats.NO_STATS.minTimestamp ? minTimestamp : -1;

Review Comment:
   > while debugging or if it got logged somewhere, reads to the eyes just like any other valid TS
   
   Yeah, makes sense. In that case, my only ask is that we pull out a constant to use across the new test, `getFullyExpiredSSTables()`, and `getMinTimestamp()`...maybe call it `NO_TIMESTAMP`. WDYT?



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