This is an automated email from the ASF dual-hosted git repository.

apolovtsev pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/ignite-3.git


The following commit(s) were added to refs/heads/main by this push:
     new 6705eb6f36 IGNITE-19762 Remove data region remnants from RocksDb 
storage (#3677)
6705eb6f36 is described below

commit 6705eb6f36f57f7fec079c2e04889d2bf0bf15f6
Author: Alexander Polovtcev <alex.polovt...@gmail.com>
AuthorDate: Mon Apr 29 12:44:37 2024 +0300

    IGNITE-19762 Remove data region remnants from RocksDb storage (#3677)
---
 .../storage/rocksdb/RocksDbStorageEngine.java      | 67 ++++++++++++----------
 ...bDataRegion.java => RocksDbStorageProfile.java} | 31 +++++-----
 .../schema/RocksDbProfileConfigurationSchema.java  |  2 +-
 .../instance/SharedRocksDbInstanceCreator.java     |  6 +-
 .../instance/SharedRocksDbInstanceTest.java        | 18 +++---
 5 files changed, 66 insertions(+), 58 deletions(-)

diff --git 
a/modules/storage-rocksdb/src/main/java/org/apache/ignite/internal/storage/rocksdb/RocksDbStorageEngine.java
 
b/modules/storage-rocksdb/src/main/java/org/apache/ignite/internal/storage/rocksdb/RocksDbStorageEngine.java
index 58cd207f6e..ab8ae858eb 100644
--- 
a/modules/storage-rocksdb/src/main/java/org/apache/ignite/internal/storage/rocksdb/RocksDbStorageEngine.java
+++ 
b/modules/storage-rocksdb/src/main/java/org/apache/ignite/internal/storage/rocksdb/RocksDbStorageEngine.java
@@ -30,6 +30,7 @@ import org.apache.ignite.internal.logger.IgniteLogger;
 import org.apache.ignite.internal.logger.Loggers;
 import org.apache.ignite.internal.storage.StorageException;
 import org.apache.ignite.internal.storage.configurations.StorageConfiguration;
+import org.apache.ignite.internal.storage.configurations.StorageProfileView;
 import org.apache.ignite.internal.storage.engine.StorageEngine;
 import org.apache.ignite.internal.storage.engine.StorageTableDescriptor;
 import org.apache.ignite.internal.storage.index.StorageIndexDescriptorSupplier;
@@ -46,24 +47,23 @@ import org.rocksdb.RocksDB;
  */
 public class RocksDbStorageEngine implements StorageEngine {
     /** Engine name. */
-    // TODO: KKK db vs Db
     public static final String ENGINE_NAME = "rocksDb";
 
     private static final IgniteLogger LOG = 
Loggers.forClass(RocksDbStorageEngine.class);
 
     private static class RocksDbStorage implements ManuallyCloseable {
-        final RocksDbDataRegion dataRegion;
+        final RocksDbStorageProfile profile;
 
         final SharedRocksDbInstance rocksDbInstance;
 
-        RocksDbStorage(RocksDbDataRegion dataRegion, SharedRocksDbInstance 
rocksDbInstance) {
-            this.dataRegion = dataRegion;
+        RocksDbStorage(RocksDbStorageProfile profile, SharedRocksDbInstance 
rocksDbInstance) {
+            this.profile = profile;
             this.rocksDbInstance = rocksDbInstance;
         }
 
         @Override
         public void close() throws Exception {
-            IgniteUtils.closeAllManually(rocksDbInstance::stop, 
dataRegion::stop);
+            IgniteUtils.closeAllManually(rocksDbInstance::stop, profile::stop);
         }
     }
 
@@ -82,11 +82,9 @@ public class RocksDbStorageEngine implements StorageEngine {
     private final ScheduledExecutorService scheduledPool;
 
     /**
-     * Mapping from the data region name to the shared RocksDB instance. Most 
likely, the association of shared
-     * instances with regions will be removed/revisited in the future.
+     * Mapping from the storage profile name to the shared RocksDB instance.
      */
-    // TODO IGNITE-19762 Think of proper way to use regions and storages.
-    private final Map<String, RocksDbStorage> storageByRegionName = new 
ConcurrentHashMap<>();
+    private final Map<String, RocksDbStorage> storageByProfileName = new 
ConcurrentHashMap<>();
 
     private final LogSyncer logSyncer;
 
@@ -99,8 +97,13 @@ public class RocksDbStorageEngine implements StorageEngine {
      * @param storagePath Storage path.
      * @param logSyncer Write-ahead log synchronizer.
      */
-    public RocksDbStorageEngine(String nodeName, 
RocksDbStorageEngineConfiguration engineConfig,
-            StorageConfiguration storageConfiguration, Path storagePath, 
LogSyncer logSyncer) {
+    public RocksDbStorageEngine(
+            String nodeName,
+            RocksDbStorageEngineConfiguration engineConfig,
+            StorageConfiguration storageConfiguration,
+            Path storagePath,
+            LogSyncer logSyncer
+    ) {
         this.engineConfig = engineConfig;
         this.storageConfiguration = storageConfiguration;
         this.storagePath = storagePath;
@@ -148,31 +151,33 @@ public class RocksDbStorageEngine implements 
StorageEngine {
 
     @Override
     public void start() throws StorageException {
-        // TODO: IGNITE-17066 Add handling deleting/updating data regions 
configuration
-        storageConfiguration.profiles().value().stream().forEach(p -> {
-            if (p instanceof RocksDbProfileView) {
-                registerDataRegion((RocksDbProfileView) p);
+        // TODO: IGNITE-17066 Add handling deleting/updating storage profiles 
configuration
+        for (StorageProfileView profile : 
storageConfiguration.profiles().value()) {
+            if (profile instanceof RocksDbProfileView) {
+                registerProfile((RocksDbProfileView) profile);
             }
-        });
+        }
     }
 
-    private void registerDataRegion(RocksDbProfileView dataRegionView) {
-        String regionName = dataRegionView.name();
+    private void registerProfile(RocksDbProfileView profileConfig) {
+        String profileName = profileConfig.name();
 
-        var region = new RocksDbDataRegion(dataRegionView);
+        var profile = new RocksDbStorageProfile(profileConfig);
 
-        region.start();
+        profile.start();
 
-        SharedRocksDbInstance rocksDbInstance = newRocksDbInstance(regionName, 
region);
+        SharedRocksDbInstance rocksDbInstance = 
newRocksDbInstance(profileName, profile);
 
-        RocksDbStorage previousStorage = storageByRegionName.put(regionName, 
new RocksDbStorage(region, rocksDbInstance));
+        RocksDbStorage previousStorage = storageByProfileName.put(profileName, 
new RocksDbStorage(profile, rocksDbInstance));
 
-        assert previousStorage == null : regionName;
+        assert previousStorage == null : "Storage already exists for profile: 
" + profileName;
     }
 
-    private SharedRocksDbInstance newRocksDbInstance(String regionName, 
RocksDbDataRegion region) {
+    private SharedRocksDbInstance newRocksDbInstance(String profileName, 
RocksDbStorageProfile profile) {
+        Path dbPath = storagePath.resolve("rocksdb-" + profileName);
+
         try {
-            return new SharedRocksDbInstanceCreator().create(this, region, 
storagePath.resolve("rocksdb-" + regionName));
+            return new SharedRocksDbInstanceCreator().create(this, profile, 
dbPath);
         } catch (Exception e) {
             throw new StorageException("Failed to create new RocksDB 
instance", e);
         }
@@ -181,7 +186,7 @@ public class RocksDbStorageEngine implements StorageEngine {
     @Override
     public void stop() throws StorageException {
         try {
-            IgniteUtils.closeAllManually(storageByRegionName.values());
+            IgniteUtils.closeAllManually(storageByProfileName.values());
         } catch (Exception e) {
             throw new StorageException("Error when stopping the rocksdb 
engine", e);
         }
@@ -200,12 +205,14 @@ public class RocksDbStorageEngine implements 
StorageEngine {
             StorageTableDescriptor tableDescriptor,
             StorageIndexDescriptorSupplier indexDescriptorSupplier
     ) throws StorageException {
-        String regionName = tableDescriptor.getStorageProfile();
+        String profileName = tableDescriptor.getStorageProfile();
 
-        RocksDbStorage storage = storageByRegionName.get(regionName);
+        RocksDbStorage storage = storageByProfileName.get(profileName);
 
         assert storage != null :
-                String.format("RocksDB instance has not yet been created for 
[tableId=%d, region=%s]", tableDescriptor.getId(), regionName);
+                String.format(
+                        "RocksDB instance has not yet been created for 
[tableId=%d, profile=%s]", tableDescriptor.getId(), profileName
+                );
 
         var tableStorage = new RocksDbTableStorage(storage.rocksDbInstance, 
tableDescriptor, indexDescriptorSupplier);
 
@@ -216,7 +223,7 @@ public class RocksDbStorageEngine implements StorageEngine {
 
     @Override
     public void dropMvTable(int tableId) {
-        for (RocksDbStorage rocksDbStorage : storageByRegionName.values()) {
+        for (RocksDbStorage rocksDbStorage : storageByProfileName.values()) {
             rocksDbStorage.rocksDbInstance.destroyTable(tableId);
         }
     }
diff --git 
a/modules/storage-rocksdb/src/main/java/org/apache/ignite/internal/storage/rocksdb/RocksDbDataRegion.java
 
b/modules/storage-rocksdb/src/main/java/org/apache/ignite/internal/storage/rocksdb/RocksDbStorageProfile.java
similarity index 62%
rename from 
modules/storage-rocksdb/src/main/java/org/apache/ignite/internal/storage/rocksdb/RocksDbDataRegion.java
rename to 
modules/storage-rocksdb/src/main/java/org/apache/ignite/internal/storage/rocksdb/RocksDbStorageProfile.java
index 0c44968d57..38629e9f85 100644
--- 
a/modules/storage-rocksdb/src/main/java/org/apache/ignite/internal/storage/rocksdb/RocksDbDataRegion.java
+++ 
b/modules/storage-rocksdb/src/main/java/org/apache/ignite/internal/storage/rocksdb/RocksDbStorageProfile.java
@@ -17,19 +17,18 @@
 
 package org.apache.ignite.internal.storage.rocksdb;
 
-import static org.apache.ignite.internal.util.IgniteUtils.closeAll;
-
 import 
org.apache.ignite.internal.storage.rocksdb.configuration.schema.RocksDbProfileView;
+import org.apache.ignite.internal.util.IgniteUtils;
 import org.rocksdb.Cache;
 import org.rocksdb.LRUCache;
 import org.rocksdb.WriteBufferManager;
 
 /**
- * Data region implementation for {@link RocksDbStorageEngine}. Based on a 
{@link Cache}.
+ * Storage profile implementation for {@link RocksDbStorageEngine}. Based on a 
{@link Cache}.
  */
-public class RocksDbDataRegion {
-    /** Region configuration view. */
-    private final RocksDbProfileView storageProfileView;
+public class RocksDbStorageProfile {
+    /** Profile configuration view. */
+    private final RocksDbProfileView storageProfileConfig;
 
     /** RocksDB cache instance. */
     private Cache cache;
@@ -40,34 +39,34 @@ public class RocksDbDataRegion {
     /**
      * Constructor.
      *
-     * @param storageProfileView Storage profile configuration view.
+     * @param storageProfileConfig Storage profile configuration view.
      */
-    public RocksDbDataRegion(RocksDbProfileView storageProfileView) {
-        this.storageProfileView = storageProfileView;
+    public RocksDbStorageProfile(RocksDbProfileView storageProfileConfig) {
+        this.storageProfileConfig = storageProfileConfig;
     }
 
     /**
-     * Start the rocksDb data region.
+     * Start the profile.
      */
     public void start() {
-        long writeBufferSize = storageProfileView.writeBufferSize();
+        long writeBufferSize = storageProfileConfig.writeBufferSize();
 
-        long totalCacheSize = storageProfileView.size() + writeBufferSize;
+        long totalCacheSize = storageProfileConfig.size() + writeBufferSize;
 
-        cache = new LRUCache(totalCacheSize, 
storageProfileView.numShardBits(), false);
+        cache = new LRUCache(totalCacheSize, 
storageProfileConfig.numShardBits(), false);
 
         writeBufferManager = new WriteBufferManager(writeBufferSize, cache);
     }
 
     /**
-     * Starts the rocksDb data region.
+     * Closes and frees resources associated with this profile.
      */
     public void stop() throws Exception {
-        closeAll(writeBufferManager, cache);
+        IgniteUtils.closeAll(writeBufferManager, cache);
     }
 
     /**
-     * Returns write buffer manager associated with the region.
+     * Returns write buffer manager associated with the profile.
      */
     public WriteBufferManager writeBufferManager() {
         return writeBufferManager;
diff --git 
a/modules/storage-rocksdb/src/main/java/org/apache/ignite/internal/storage/rocksdb/configuration/schema/RocksDbProfileConfigurationSchema.java
 
b/modules/storage-rocksdb/src/main/java/org/apache/ignite/internal/storage/rocksdb/configuration/schema/RocksDbProfileConfigurationSchema.java
index 61a3255134..99b84544ea 100644
--- 
a/modules/storage-rocksdb/src/main/java/org/apache/ignite/internal/storage/rocksdb/configuration/schema/RocksDbProfileConfigurationSchema.java
+++ 
b/modules/storage-rocksdb/src/main/java/org/apache/ignite/internal/storage/rocksdb/configuration/schema/RocksDbProfileConfigurationSchema.java
@@ -24,7 +24,7 @@ import 
org.apache.ignite.internal.storage.configurations.StorageProfileConfigura
 import org.apache.ignite.internal.storage.rocksdb.RocksDbStorageEngine;
 
 /**
- * Data region configuration for {@link RocksDbStorageEngine}.
+ * Storage profile configuration for {@link RocksDbStorageEngine}.
  */
 @PolymorphicConfigInstance("rocksDb")
 public class RocksDbProfileConfigurationSchema extends 
StorageProfileConfigurationSchema {
diff --git 
a/modules/storage-rocksdb/src/main/java/org/apache/ignite/internal/storage/rocksdb/instance/SharedRocksDbInstanceCreator.java
 
b/modules/storage-rocksdb/src/main/java/org/apache/ignite/internal/storage/rocksdb/instance/SharedRocksDbInstanceCreator.java
index 86222e269b..399d51a1a2 100644
--- 
a/modules/storage-rocksdb/src/main/java/org/apache/ignite/internal/storage/rocksdb/instance/SharedRocksDbInstanceCreator.java
+++ 
b/modules/storage-rocksdb/src/main/java/org/apache/ignite/internal/storage/rocksdb/instance/SharedRocksDbInstanceCreator.java
@@ -34,9 +34,9 @@ import org.apache.ignite.internal.storage.StorageException;
 import org.apache.ignite.internal.storage.rocksdb.ColumnFamilyUtils;
 import 
org.apache.ignite.internal.storage.rocksdb.ColumnFamilyUtils.ColumnFamilyType;
 import org.apache.ignite.internal.storage.rocksdb.PartitionDataHelper;
-import org.apache.ignite.internal.storage.rocksdb.RocksDbDataRegion;
 import org.apache.ignite.internal.storage.rocksdb.RocksDbMetaStorage;
 import org.apache.ignite.internal.storage.rocksdb.RocksDbStorageEngine;
+import org.apache.ignite.internal.storage.rocksdb.RocksDbStorageProfile;
 import 
org.apache.ignite.internal.storage.rocksdb.index.AbstractRocksDbIndexStorage;
 import 
org.apache.ignite.internal.storage.rocksdb.index.RocksDbHashIndexStorage;
 import org.apache.ignite.internal.util.IgniteSpinBusyLock;
@@ -63,7 +63,7 @@ public class SharedRocksDbInstanceCreator {
      */
     public SharedRocksDbInstance create(
             RocksDbStorageEngine engine,
-            RocksDbDataRegion region,
+            RocksDbStorageProfile profile,
             Path path
     ) throws RocksDBException, IOException {
         var busyLock = new IgniteSpinBusyLock();
@@ -90,7 +90,7 @@ public class SharedRocksDbInstanceCreator {
                     // Atomic flush must be enabled to guarantee consistency 
between different column families when WAL is disabled.
                     .setAtomicFlush(true)
                     .setListeners(List.of(flusher.listener()))
-                    .setWriteBufferManager(region.writeBufferManager())
+                    .setWriteBufferManager(profile.writeBufferManager())
             );
 
             RocksDB db = add(RocksDB.open(dbOptions, 
path.toAbsolutePath().toString(), cfDescriptors, cfHandles));
diff --git 
a/modules/storage-rocksdb/src/test/java/org/apache/ignite/internal/storage/rocksdb/instance/SharedRocksDbInstanceTest.java
 
b/modules/storage-rocksdb/src/test/java/org/apache/ignite/internal/storage/rocksdb/instance/SharedRocksDbInstanceTest.java
index 863fb1d469..a60ebdee50 100644
--- 
a/modules/storage-rocksdb/src/test/java/org/apache/ignite/internal/storage/rocksdb/instance/SharedRocksDbInstanceTest.java
+++ 
b/modules/storage-rocksdb/src/test/java/org/apache/ignite/internal/storage/rocksdb/instance/SharedRocksDbInstanceTest.java
@@ -49,8 +49,8 @@ import 
org.apache.ignite.internal.configuration.testframework.InjectConfiguratio
 import org.apache.ignite.internal.rocksdb.ColumnFamily;
 import org.apache.ignite.internal.storage.configurations.StorageConfiguration;
 import 
org.apache.ignite.internal.storage.index.StorageSortedIndexDescriptor.StorageSortedIndexColumnDescriptor;
-import org.apache.ignite.internal.storage.rocksdb.RocksDbDataRegion;
 import org.apache.ignite.internal.storage.rocksdb.RocksDbStorageEngine;
+import org.apache.ignite.internal.storage.rocksdb.RocksDbStorageProfile;
 import 
org.apache.ignite.internal.storage.rocksdb.configuration.schema.RocksDbProfileView;
 import 
org.apache.ignite.internal.storage.rocksdb.configuration.schema.RocksDbStorageEngineConfiguration;
 import org.apache.ignite.internal.testframework.IgniteAbstractTest;
@@ -70,7 +70,7 @@ import org.rocksdb.RocksDBException;
 class SharedRocksDbInstanceTest extends IgniteAbstractTest {
     private RocksDbStorageEngine engine;
 
-    private RocksDbDataRegion dataRegion;
+    private RocksDbStorageProfile storageProfile;
 
     private SharedRocksDbInstance rocksDb;
 
@@ -78,15 +78,17 @@ class SharedRocksDbInstanceTest extends IgniteAbstractTest {
     void setUp(
             @InjectConfiguration("mock.profiles.default = {engine = 
\"rocksDb\", size = 16777216, writeBufferSize = 16777216}")
             StorageConfiguration storageConfiguration,
-            @InjectConfiguration RocksDbStorageEngineConfiguration 
engineConfig) throws Exception {
+            @InjectConfiguration RocksDbStorageEngineConfiguration engineConfig
+    ) throws Exception {
         engine = new RocksDbStorageEngine("test", engineConfig, 
storageConfiguration, workDir, mock(LogSyncer.class));
 
         engine.start();
 
-        dataRegion = new RocksDbDataRegion(
-                (RocksDbProfileView) 
storageConfiguration.profiles().get("default").value());
+        var profileConfig = (RocksDbProfileView) 
storageConfiguration.profiles().get("default").value();
 
-        dataRegion.start();
+        storageProfile = new RocksDbStorageProfile(profileConfig);
+
+        storageProfile.start();
 
         rocksDb = createDb();
     }
@@ -95,13 +97,13 @@ class SharedRocksDbInstanceTest extends IgniteAbstractTest {
     void tearDown() throws Exception {
         IgniteUtils.closeAllManually(
                 rocksDb == null ? null : rocksDb::stop,
-                dataRegion == null ? null : dataRegion::stop,
+                storageProfile == null ? null : storageProfile::stop,
                 engine == null ? null : engine::stop
         );
     }
 
     private SharedRocksDbInstance createDb() throws Exception {
-        return new SharedRocksDbInstanceCreator().create(engine, dataRegion, 
workDir);
+        return new SharedRocksDbInstanceCreator().create(engine, 
storageProfile, workDir);
     }
 
     @Test

Reply via email to