vldpyatkov commented on code in PR #4005:
URL: https://github.com/apache/ignite-3/pull/4005#discussion_r1706115532
##########
modules/raft/src/main/java/org/apache/ignite/internal/raft/Loza.java:
##########
@@ -202,9 +196,14 @@ public <T extends RaftGroupService> CompletableFuture<T>
startRaftGroupNode(
PeersAndLearners configuration,
RaftGroupListener lsnr,
RaftGroupEventsListener eventsLsnr,
- RaftServiceFactory<T> factory
+ RaftServiceFactory<T> factory,
+ RaftOptionsConfigurator storageConfigurator
Review Comment:
Why did you choose a closure instead of pojo with clear fields?
##########
modules/raft/src/main/java/org/apache/ignite/internal/raft/Loza.java:
##########
@@ -314,25 +325,29 @@ public <T extends RaftGroupService> CompletableFuture<T>
startRaftGroupNodeAndWa
RaftGroupListener lsnr,
RaftGroupEventsListener eventsLsnr,
RaftNodeDisruptorConfiguration disruptorConfiguration,
- @Nullable RaftServiceFactory<T> factory
+ @Nullable RaftServiceFactory<T> factory,
+ RaftOptionsConfigurator storageConfigurator
) throws NodeStoppingException {
if (!busyLock.enterBusy()) {
throw new NodeStoppingException();
}
try {
- CompletableFuture<T> startRaftServiceFuture =
startRaftGroupNodeInternal(
+ RaftGroupOptions raftGroupOptions = RaftGroupOptions.defaults();
+
+ storageConfigurator.configure(raftGroupOptions);
+
+
raftGroupOptions.ownFsmCallerExecutorDisruptorConfig(disruptorConfiguration);
Review Comment:
Why don't we reduce this method from Loza and add this setter to the raft
option configuration closure (msRaftConfigurator)?
##########
modules/replicator/src/main/java/org/apache/ignite/internal/replicator/ReplicaManager.java:
##########
@@ -855,6 +862,8 @@ private RaftGroupOptions groupOptionsForPartition(boolean
isVolatileStorage, Sna
raftGroupOptions.commandsMarshaller(raftCommandsMarshaller);
+ partitionRaftConfigurator.configure(raftGroupOptions);
Review Comment:
Only Loza do this in code above. Why do you think it ought to be done
explicitly before Loza is called?
##########
modules/runner/src/main/java/org/apache/ignite/internal/app/IgniteImpl.java:
##########
@@ -405,6 +407,12 @@ public class IgniteImpl implements Ignite {
/** Default log storage factory for raft. */
private final LogStorageFactory logStorageFactory;
+ private final LogStorageFactory mslogStorageFactory;
Review Comment:
Rename to ms**L**ogStorageFactory
--
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]
