[GitHub] [bookkeeper] mauricebarnum commented on a change in pull request #2775: [STREAMSTORAGE] support an optional time to live (TTL) on a per table basis

[GitBox <gi...@apache.org>]

mauricebarnum commented on a change in pull request #2775:
URL: https://github.com/apache/bookkeeper/pull/2775#discussion_r695729575



##########
File path: stream/statelib/src/main/java/org/apache/bookkeeper/statelib/impl/kv/RocksdbKVStore.java
##########
@@ -367,11 +376,35 @@ protected void openRocksdb(StateStoreSpec spec) throws StateStoreException {
             }
 
             List<ColumnFamilyHandle> cfHandles = Lists.newArrayListWithExpectedSize(2);
-            RocksDB db = RocksDB.open(
-                options,
-                dbDir.getAbsolutePath(),
-                Lists.newArrayList(metaDesc, dataDesc),
-                cfHandles);
+
+            RocksDB db = TtlDB.open(

Review comment:
       Makes sense. I was hoping to eventually remove the conditional logic, but with no practical way to upgrade existing DBs…
   
   I’ll simplify the logic and change the error message to indicate what is wrong.  This will also enable a BK rollback if the option hasn’t been used yet to create any new tables with a ttl.
   
   I need to think about a way to fail opening of a ttl db with the old code. Using different column family names should suffice. 




-- 
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: issues-unsubscribe@bookkeeper.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org