sashapolo commented on a change in pull request #405:
URL: https://github.com/apache/ignite-3/pull/405#discussion_r735759341
##########
File path:
modules/storage-api/src/main/java/org/apache/ignite/internal/storage/engine/StorageEngine.java
##########
@@ -24,11 +24,24 @@
import org.apache.ignite.configuration.schemas.store.DataRegionConfiguration;
import org.apache.ignite.configuration.schemas.table.TableConfiguration;
import org.apache.ignite.configuration.schemas.table.TableView;
+import org.apache.ignite.internal.storage.StorageException;
/**
- * General storageengine interface.
+ * General storage engine interface.
*/
public interface StorageEngine {
+ /**
+ * Starts the engine.
+ */
+ void start();
+
+ /**
+ * Stops the engine.
+ *
+ * @throws StorageException If an error has occurred during the engine
stop.
+ */
+ void stop() throws StorageException;
+
/**
* Creates new data resion.
Review comment:
```suggestion
* Creates a new data region.
```
##########
File path:
modules/storage-rocksdb/src/main/java/org/apache/ignite/internal/storage/rocksdb/RocksDbStorageEngine.java
##########
@@ -37,6 +43,19 @@
RocksDB.loadLibrary();
}
+ /** */
+ private final ExecutorService threadPool = Executors.newFixedThreadPool(
+ Runtime.getRuntime().availableProcessors(), new
NamedThreadFactory("rocksdb-pool"));
Review comment:
I think `rocksdb-snapshot-pool` or `rocksdb-storage-engine-pool` might
be better names
##########
File path:
modules/storage-api/src/main/java/org/apache/ignite/internal/storage/engine/StorageEngine.java
##########
@@ -24,11 +24,24 @@
import org.apache.ignite.configuration.schemas.store.DataRegionConfiguration;
import org.apache.ignite.configuration.schemas.table.TableConfiguration;
import org.apache.ignite.configuration.schemas.table.TableView;
+import org.apache.ignite.internal.storage.StorageException;
/**
- * General storageengine interface.
+ * General storage engine interface.
*/
public interface StorageEngine {
+ /**
+ * Starts the engine.
+ */
+ void start();
Review comment:
Both `start` and `stop` methods are not invoked anywhere
##########
File path:
modules/storage-rocksdb/src/main/java/org/apache/ignite/internal/storage/rocksdb/RocksDbStorageEngine.java
##########
@@ -37,6 +43,19 @@
RocksDB.loadLibrary();
}
+ /** */
+ private final ExecutorService threadPool = Executors.newFixedThreadPool(
Review comment:
Shall we use a cached thread pool instead?
##########
File path:
modules/storage-rocksdb/src/main/java/org/apache/ignite/internal/storage/rocksdb/RocksDbTableStorage.java
##########
@@ -116,17 +119,20 @@
*
* @param tablePath Path for the directory that stores table data.
* @param tableCfg Table configuration.
+ * @param engine Storage engine.
* @param dataRegion Data region for the table.
* @param indexComparatorFactory Comparators factory for indexes.
*/
public RocksDbTableStorage(
Path tablePath,
TableConfiguration tableCfg,
+ RocksDbStorageEngine engine,
Review comment:
I would argue that passing an `Executor` directly might be a cleaner
design, since `StorageEngine` interface is too powerful
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
