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