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 2021/10/21 09:24:47 UTC
[GitHub] [ignite-3] ibessonov opened a new pull request #405: IGNITE-15691 Common thread pool is used for snapshotting all rocksdb-based partitions.
ibessonov opened a new pull request #405:
URL: https://github.com/apache/ignite-3/pull/405
https://issues.apache.org/jira/browse/IGNITE-15691
--
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
[GitHub] [ignite-3] ibessonov commented on a change in pull request #405: IGNITE-15691 Common thread pool is used for snapshotting all rocksdb-based partitions.
Posted by GitBox <gi...@apache.org>.
ibessonov commented on a change in pull request #405:
URL: https://github.com/apache/ignite-3/pull/405#discussion_r749079985
##########
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:
Done
--
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
[GitHub] [ignite-3] ibessonov commented on a change in pull request #405: IGNITE-15691 Common thread pool is used for snapshotting all rocksdb-based partitions.
Posted by GitBox <gi...@apache.org>.
ibessonov commented on a change in pull request #405:
URL: https://github.com/apache/ignite-3/pull/405#discussion_r749076516
##########
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:
True
--
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
[GitHub] [ignite-3] sashapolo commented on a change in pull request #405: IGNITE-15691 Common thread pool is used for snapshotting all rocksdb-based partitions.
Posted by GitBox <gi...@apache.org>.
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] ibessonov merged pull request #405: IGNITE-15691 Common thread pool is used for snapshotting all rocksdb-based partitions.
Posted by GitBox <gi...@apache.org>.
ibessonov merged pull request #405:
URL: https://github.com/apache/ignite-3/pull/405
--
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
[GitHub] [ignite-3] ibessonov commented on a change in pull request #405: IGNITE-15691 Common thread pool is used for snapshotting all rocksdb-based partitions.
Posted by GitBox <gi...@apache.org>.
ibessonov commented on a change in pull request #405:
URL: https://github.com/apache/ignite-3/pull/405#discussion_r749077500
##########
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:
Honestly, I don't know
--
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