sanpwc commented on code in PR #1493:
URL: https://github.com/apache/ignite-3/pull/1493#discussion_r1064348761
##########
modules/metastorage/README.md:
##########
@@ -0,0 +1,40 @@
+# Metastorage
+
+The module for store and access to metadata.
+
+The storage is distributed to prevent losing data and based of RAFT. In
typical case, the group consists of only several nodes of the
Review Comment:
> In typical case, the group consists of only several nodes of the...
Could you please rephrase it? Currently it seems to be a little bit
ambiguous. Let's say that meta storage group has it's nodes on top of every
cluster node, however most of such participants are learners (here we should
explain what raft learner is) and only few (3, 5) of them are voting ones.
Basically it's what you've already told above but with greater details.
##########
modules/metastorage/README.md:
##########
@@ -0,0 +1,40 @@
+# Metastorage
+
+The module for store and access to metadata.
+
+The storage is distributed to prevent losing data and based of RAFT. In
typical case, the group consists of only several nodes of the
+cluster (amount of nodes have to be odd 3 or 5). The rest of nodes listens
metadata update but does not participant in voting (RAFT
+listeners).
Review Comment:
listeners -> learners.
##########
modules/metastorage/README.md:
##########
@@ -0,0 +1,40 @@
+# Metastorage
+
+The module for store and access to metadata.
+
+The storage is distributed to prevent losing data and based of RAFT. In
typical case, the group consists of only several nodes of the
+cluster (amount of nodes have to be odd 3 or 5). The rest of nodes listens
metadata update but does not participant in voting (RAFT
+listeners).
+
+## Threading model
+
+There is only one dedicate thread to notify watches. This is a thread with
prefix metastorage-watch-executor.
+The thread is created from executor in KeyValueStorage:
+
+```java
+private final ExecutorService watchExecutor =
Executors.newSingleThreadExecutor(new
NamedThreadFactory("metastorage-watch-executor",LOG));
+```
+
+Also, the storage internally contains a two threads executor to create a
snapshot:
+
+```java
+private final ExecutorService snapshotExecutor =
Executors.newFixedThreadPool(2,new
NamedThreadFactory("metastorage-snapshot-executor",LOG));
+```
+
+Both of the executors have no a node name in their names. The names are
required to identify threads belonged to different Ignite nodes on
+the shared java machine (TODO: IGNITE-18504 Add nodes name in prefix to
Metastorage executors).
+
+### Interface methods
+
+Various operations (*get()*, *getAll()*, *invoke()*) in Metastorage manager
return futures. Those futures are completed when the matched
Review Comment:
As we've discussed let's add few more details here.
##########
modules/metastorage/README.md:
##########
@@ -0,0 +1,40 @@
+# Metastorage
+
+The module for store and access to metadata.
+
+The storage is distributed to prevent losing data and based of RAFT. In
typical case, the group consists of only several nodes of the
+cluster (amount of nodes have to be odd 3 or 5). The rest of nodes listens
metadata update but does not participant in voting (RAFT
+listeners).
+
+## Threading model
+
+There is only one dedicate thread to notify watches. This is a thread with
prefix metastorage-watch-executor.
+The thread is created from executor in KeyValueStorage:
+
+```java
+private final ExecutorService watchExecutor =
Executors.newSingleThreadExecutor(new
NamedThreadFactory("metastorage-watch-executor",LOG));
+```
+
+Also, the storage internally contains a two threads executor to create a
snapshot:
+
+```java
+private final ExecutorService snapshotExecutor =
Executors.newFixedThreadPool(2,new
NamedThreadFactory("metastorage-snapshot-executor",LOG));
+```
+
+Both of the executors have no a node name in their names. The names are
required to identify threads belonged to different Ignite nodes on
+the shared java machine (TODO: IGNITE-18504 Add nodes name in prefix to
Metastorage executors).
+
+### Interface methods
Review Comment:
Initially, there were asyncs on cursors, we do expect this to be reworked
with subscribers, correct? If true this subscribers will probably add some
(one?) new threads. Could you please create a follow-up ticket in order to
update ms thread-model after Alexander will finish his ms-cursors refactoring?
##########
modules/metastorage/src/main/java/org/apache/ignite/internal/metastorage/impl/MetaStorageManagerImpl.java:
##########
@@ -200,7 +200,7 @@ public void onDisappeared(ClusterNode member) {
public void start() {
this.metaStorageSvcFut = cmgMgr.metaStorageNodes()
// use default executor to avoid blocking CMG manager threads
- .thenComposeAsync(metaStorageNodes -> {
+ .thenCompose(metaStorageNodes -> {
Review Comment:
Is that about
> The component is used common ForkJoinPool on start
?
There's a comment
> // use default executor to avoid blocking CMG manager threads
So, do you really think that using thenCompose instead of thenComposeAsync
won't block CMG manager threads?
##########
modules/metastorage/README.md:
##########
@@ -0,0 +1,40 @@
+# Metastorage
+
+The module for store and access to metadata.
+
+The storage is distributed to prevent losing data and based of RAFT. In
typical case, the group consists of only several nodes of the
Review Comment:
Well, that's not only to prevent data loss but also for fault tolerance, etc.
##########
modules/metastorage/README.md:
##########
@@ -0,0 +1,40 @@
+# Metastorage
+
+The module for store and access to metadata.
Review Comment:
There are 5 ms related modules:
- metastorage
- metastorage-api
- metastorage-client
- metastorage-common
- metastorage-server
Given thread-model description covers all of them, is that correct?
--
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]
