szetszwo commented on code in PR #733:
URL: https://github.com/apache/ratis/pull/733#discussion_r974016218
##########
ratis-server-api/src/main/java/org/apache/ratis/server/RaftServer.java:
##########
@@ -116,6 +116,11 @@ default RaftGroup getGroup() {
/** @return the internal {@link RaftClient} of this division. */
RaftClient getRaftClient();
+ /** @return the {@link ThreadGroup} the threads of this Division belong
to. */
+ default ThreadGroup getThreadGroup() {
+ return Thread.currentThread().getThreadGroup();
+ }
Review Comment:
Remove the default -- it will never be used.
##########
ratis-server/src/main/java/org/apache/ratis/server/impl/RaftServerImpl.java:
##########
@@ -274,6 +276,10 @@ TimeDuration getSleepDeviationThreshold() {
return sleepDeviationThreshold;
}
+ public ThreadGroup getThreadGroup() {
Review Comment:
Add `@Override`.
##########
ratis-server/src/main/java/org/apache/ratis/server/impl/RaftServerImpl.java:
##########
@@ -95,9 +95,9 @@
import static org.apache.ratis.util.LifeCycle.State.RUNNING;
import static org.apache.ratis.util.LifeCycle.State.STARTING;
-class RaftServerImpl implements RaftServer.Division,
+public class RaftServerImpl implements RaftServer.Division,
Review Comment:
Please remove `public`.
##########
ratis-server/src/main/java/org/apache/ratis/server/impl/RaftServerProxy.java:
##########
@@ -196,9 +197,11 @@ String toString(RaftGroupId groupId,
CompletableFuture<RaftServerImpl> f) {
private final ExecutorService executor;
private final JvmPauseMonitor pauseMonitor;
+ @Nullable
Review Comment:
Let's create a default `ThreadGroup` and make it non-nullable.
##########
ratis-server/src/main/java/org/apache/ratis/server/impl/RaftServerProxy.java:
##########
@@ -379,6 +383,10 @@ public LifeCycle.State getLifeCycleState() {
return lifeCycle.getCurrentState();
}
+ public ThreadGroup getThreadGroup() {
Review Comment:
Remove `public`.
##########
ratis-server/src/main/java/org/apache/ratis/server/impl/ServerImplUtils.java:
##########
@@ -47,26 +47,28 @@ private ServerImplUtils() {
/** Create a {@link RaftServerProxy}. */
public static RaftServerProxy newRaftServer(
RaftPeerId id, RaftGroup group, RaftStorage.StartupOption option,
StateMachine.Registry stateMachineRegistry,
- RaftProperties properties, Parameters parameters) throws IOException {
+ RaftProperties properties, Parameters parameters, ThreadGroup
threadGroup)
Review Comment:
Please reorder the parameters:
```java
ThreadGroup threadGroup, RaftProperties properties, Parameters parameters)
```
##########
ratis-server/src/main/java/org/apache/ratis/server/impl/RaftServerImpl.java:
##########
@@ -197,6 +198,7 @@ public long[] getFollowerNextIndices() {
this.lifeCycle = new LifeCycle(id);
this.stateMachine = stateMachine;
this.role = new RoleInfo(id);
+ this.threadGroup = proxy.getThreadGroup();
Review Comment:
Create a child `ThreadGroup` for each impl:
```java
this.threadGroup = new ThreadGroup(proxy.getThreadGroup(),
getMemberId().toString());
```
--
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]
