[
https://issues.apache.org/jira/browse/RATIS-337?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16641350#comment-16641350
]
Tsz Wo Nicholas Sze edited comment on RATIS-337 at 10/8/18 5:01 AM:
--------------------------------------------------------------------
Thanks, [~jnp] for the comments.
> ... Do you see major perf impact if checkLeaderState is synchronized in
> RaftServerImpl?
checkLeaderState(..) is called in the first few statements in
submitClientRequestAsync(..). If it is synchronized, it will hurt the
performance.
> As long as commit-index is updated, this will happen in the background, is
> this really needed at shutting down of leader thread?
It seems okay. I just have copied the TODO from the earlier code. Let me think
about it in details.
> This method doesn't need to be static.
It is better to be static -- it is clear that it does not use any fields in the
class. Let me move it to ServerProtoUtils.
was (Author: szetszwo):
Thanks, [~jnp] for the comments.
> ... Do you see major perf impact if checkLeaderState is synchronized in
> RaftServerImpl?
checkLeaderState(..) is called in the first few statements in
submitClientRequestAsync(..). If it is synchronized, it will hurt the
performance.
> As long as commit-index is updated, this will happen in the background, is
> this really needed at shutting down of leader thread?
I just have copied the TODO from the earlier code. Let me think about it in
details.
> This method doesn't need to be static.
It is better to be static -- it is clear that it does not use any fields in the
class. Let me move it to ServerProtoUtils.
> In RaftServerImpl, leaderState/heartbeatMonitor may be accessed without
> proper null check
> -----------------------------------------------------------------------------------------
>
> Key: RATIS-337
> URL: https://issues.apache.org/jira/browse/RATIS-337
> Project: Ratis
> Issue Type: Bug
> Components: server
> Reporter: Tsz Wo Nicholas Sze
> Assignee: Tsz Wo Nicholas Sze
> Priority: Major
> Attachments: r337_20181008.patch
>
>
> leaderState/heartbeatMonitor is declared as volatile. Some code like below
> won't work since leaderState may be set to null in between.
> {code:java}
> //RaftServerImpl.checkLeaderState(..)
> } else if (leaderState == null || !leaderState.isReady()) {
> {code}
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
