You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@ignite.apache.org by "rpuch (via GitHub)" <gi...@apache.org> on 2024/05/15 13:43:11 UTC

Re: [PR] IGNITE-22240 Improve handling for RocksDb resources in DefaultLogStorage and SharedRocksDbInstance [ignite-3]

rpuch commented on code in PR #3755:
URL: https://github.com/apache/ignite-3/pull/3755#discussion_r1601664000


##########
modules/rocksdb-common/src/main/java/org/apache/ignite/internal/rocksdb/ColumnFamily.java:
##########
@@ -69,7 +76,7 @@ private ColumnFamily(RocksDB db, ColumnFamilyHandle handle) throws RocksDBExcept
     public static ColumnFamily create(RocksDB db, ColumnFamilyDescriptor descriptor) throws RocksDBException {

Review Comment:
   How about changing method name to highlight that the result is going to own the column options? But I cannot think of any good name yet... `createOwningOptions` seems a bit too clumsy and specific



##########
modules/rocksdb-common/src/main/java/org/apache/ignite/internal/rocksdb/ColumnFamily.java:
##########
@@ -45,21 +46,27 @@ public class ColumnFamily {
     /** Column family handle. */
     private final ColumnFamilyHandle cfHandle;
 
+    /** Column family options, if exclusive to this CF. */
+    @Nullable
+    private final ColumnFamilyOptions options;

Review Comment:
   How about renaming the field to make it easier to understand that it's something exclusive to this object? Like `ownedOptions` or something similar



##########
modules/storage-rocksdb/src/main/java/org/apache/ignite/internal/storage/rocksdb/instance/SharedRocksDbInstance.java:
##########
@@ -395,7 +407,10 @@ private static byte[] metaPrefix(byte[] metaPrefix, byte[] tableIdBytes) {
     }
 
     private ColumnFamily createSortedIndexCf(byte[] cfName) {
-        ColumnFamilyDescriptor cfDescriptor = new ColumnFamilyDescriptor(cfName, sortedIndexCfOptions(cfName));
+        // TODO: CfOptions not closed!!!!

Review Comment:
   The TODO should either be resolved or mention a Jira issue



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