You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@ignite.apache.org by GitBox <gi...@apache.org> on 2022/03/17 10:59:07 UTC

[GitHub] [ignite-3] SammyVimes commented on a change in pull request #723: IGNITE-16688 Extract RocksDB snapshots into a separate class

SammyVimes commented on a change in pull request #723:
URL: https://github.com/apache/ignite-3/pull/723#discussion_r828984404



##########
File path: modules/metastorage-server/src/main/java/org/apache/ignite/internal/metastorage/server/persistence/RocksDbKeyValueStorage.java
##########
@@ -179,25 +179,30 @@ public void start() {
 
         ColumnFamilyOptions indexFamilyOptions = new ColumnFamilyOptions(indexOptions);
 
-        List<ColumnFamilyDescriptor> descriptors = Arrays.asList(
+        return List.of(
                 new ColumnFamilyDescriptor(DATA.nameAsBytes(), dataFamilyOptions),
                 new ColumnFamilyDescriptor(INDEX.nameAsBytes(), indexFamilyOptions)
         );
+    }
 
-        var handles = new ArrayList<ColumnFamilyHandle>();
+    private void recreateDb() throws RocksDBException {
+        destroyRocksDb();
 
-        try {
-            // Delete existing data, relying on the raft's snapshot and log playback
-            destroyRocksDb();
+        List<ColumnFamilyDescriptor> descriptors = cfDescriptors();
 
-            this.db = RocksDB.open(options, dbPath.toAbsolutePath().toString(), descriptors, handles);
-        } catch (RocksDBException e) {
-            throw new IgniteInternalException("Failed to start the storage", e);
-        }
+        var handles = new ArrayList<ColumnFamilyHandle>(descriptors.size());
+
+        options = new DBOptions()
+                .setCreateMissingColumnFamilies(true)
+                .setCreateIfMissing(true);
+
+        db = RocksDB.open(options, dbPath.toAbsolutePath().toString(), descriptors, handles);
+
+        data = ColumnFamily.createExisting(db, handles.get(0));

Review comment:
       `recreateDb` is called inside `start`, but here you use `createExisting`. What if column family doesn't exist?

##########
File path: modules/metastorage-server/src/main/java/org/apache/ignite/internal/metastorage/server/persistence/RocksDbKeyValueStorage.java
##########
@@ -179,25 +179,30 @@ public void start() {
 
         ColumnFamilyOptions indexFamilyOptions = new ColumnFamilyOptions(indexOptions);
 
-        List<ColumnFamilyDescriptor> descriptors = Arrays.asList(
+        return List.of(
                 new ColumnFamilyDescriptor(DATA.nameAsBytes(), dataFamilyOptions),

Review comment:
       I wonder if column family options should be closed too

##########
File path: modules/metastorage-server/src/main/java/org/apache/ignite/internal/metastorage/server/persistence/RocksDbKeyValueStorage.java
##########
@@ -179,25 +179,30 @@ public void start() {
 
         ColumnFamilyOptions indexFamilyOptions = new ColumnFamilyOptions(indexOptions);
 
-        List<ColumnFamilyDescriptor> descriptors = Arrays.asList(
+        return List.of(
                 new ColumnFamilyDescriptor(DATA.nameAsBytes(), dataFamilyOptions),
                 new ColumnFamilyDescriptor(INDEX.nameAsBytes(), indexFamilyOptions)
         );
+    }
 
-        var handles = new ArrayList<ColumnFamilyHandle>();
+    private void recreateDb() throws RocksDBException {
+        destroyRocksDb();
 
-        try {
-            // Delete existing data, relying on the raft's snapshot and log playback
-            destroyRocksDb();
+        List<ColumnFamilyDescriptor> descriptors = cfDescriptors();
 
-            this.db = RocksDB.open(options, dbPath.toAbsolutePath().toString(), descriptors, handles);
-        } catch (RocksDBException e) {
-            throw new IgniteInternalException("Failed to start the storage", e);
-        }
+        var handles = new ArrayList<ColumnFamilyHandle>(descriptors.size());
+
+        options = new DBOptions()
+                .setCreateMissingColumnFamilies(true)
+                .setCreateIfMissing(true);
+
+        db = RocksDB.open(options, dbPath.toAbsolutePath().toString(), descriptors, handles);
+
+        data = ColumnFamily.createExisting(db, handles.get(0));

Review comment:
       Oh, I see, you use create missing column families

##########
File path: modules/storage-rocksdb/src/main/java/org/apache/ignite/internal/storage/rocksdb/RocksDbTableStorage.java
##########
@@ -405,7 +369,7 @@ private ColumnFamilyDescriptor cfDescriptorFromName(String cfName) {
      * Creates a descriptor of the "partition" Column Family.
      */
     private static ColumnFamilyDescriptor partitionCfDescriptor() {
-        return new ColumnFamilyDescriptor(PARTITION_CF_NAME.getBytes(StandardCharsets.UTF_8), new ColumnFamilyOptions());
+        return new ColumnFamilyDescriptor(PARTITION_CF_NAME.getBytes(UTF_8), new ColumnFamilyOptions());

Review comment:
       seems like this method is not used




-- 
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: notifications-unsubscribe@ignite.apache.org

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