You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ratis.apache.org by GitBox <gi...@apache.org> on 2022/03/13 09:17:19 UTC
[GitHub] [ratis] codings-dan opened a new pull request #622: RATIS-1551. Suppport listener in setConfiguration
codings-dan opened a new pull request #622:
URL: https://github.com/apache/ratis/pull/622
## What changes were proposed in this pull request?
Sub task to support listener: Suppport listener in setConfiguration
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/RATIS-1551
## How was this patch tested?
UT
--
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: issues-unsubscribe@ratis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ratis] szetszwo commented on pull request #622: RATIS-1551. Suppport listener in setConfiguration
Posted by GitBox <gi...@apache.org>.
szetszwo commented on pull request #622:
URL: https://github.com/apache/ratis/pull/622#issuecomment-1066906369
> ... add corresponding methods to distinguish Listener and follower.
@codings-dan , maybe. The role info (Listener or Follower) should comes from RaftConfiguration. We are currently passing a RaftConfiguration to divdeFollowers(..) in LeaderStateImpl in order to get voterLists. I wonder if it is already good enough, i.e. we can use voterLists to distinguish Listener and Follower. We use voterLists for calculating majority; see getMajorityMin(..).
--
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: issues-unsubscribe@ratis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ratis] codings-dan commented on pull request #622: RATIS-1551. Suppport listener in setConfiguration
Posted by GitBox <gi...@apache.org>.
codings-dan commented on pull request #622:
URL: https://github.com/apache/ratis/pull/622#issuecomment-1070816876
@szetszwo Thanks for helping change the code, the introduction of this variable really reduces a lot of redundant code!
--
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: issues-unsubscribe@ratis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ratis] codings-dan commented on a change in pull request #622: RATIS-1551. Suppport listener in setConfiguration
Posted by GitBox <gi...@apache.org>.
codings-dan commented on a change in pull request #622:
URL: https://github.com/apache/ratis/pull/622#discussion_r829017695
##########
File path: ratis-server/src/main/java/org/apache/ratis/server/impl/LeaderStateImpl.java
##########
@@ -235,6 +235,7 @@ boolean removeAll(Collection<LogAppender> c) {
private volatile ConfigurationStagingState stagingState;
private List<List<RaftPeerId>> voterLists;
private final Map<RaftPeerId, FollowerInfo> peerIdFollowerInfoMap = new ConcurrentHashMap<>();
+ private final Map<RaftPeerId, FollowerInfo> peerIdListenerInfoMap = new ConcurrentHashMap<>();
Review comment:
Yean, adding a data structure is really hard to keep it 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: issues-unsubscribe@ratis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ratis] codings-dan commented on pull request #622: RATIS-1551. Suppport listener in setConfiguration
Posted by GitBox <gi...@apache.org>.
codings-dan commented on pull request #622:
URL: https://github.com/apache/ratis/pull/622#issuecomment-1070816876
--
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: issues-unsubscribe@ratis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ratis] szetszwo commented on pull request #622: RATIS-1551. Suppport listener in setConfiguration
Posted by GitBox <gi...@apache.org>.
szetszwo commented on pull request #622:
URL: https://github.com/apache/ratis/pull/622#issuecomment-1066788841
@codings-dan , It seems that we don't need a different LogAppender for Listeners since LogAppender just send raft log to the other side. So how about we keep using FollowerInfo for Listeners? We may change the interface name to PeerInfo later on.
In LeaderStateImpl, there are senders and voterLists.
- senders could include both Followers and Listeners.
- voterLists only includes Followers.
What do you think?
--
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: issues-unsubscribe@ratis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ratis] codings-dan commented on pull request #622: RATIS-1551. Suppport listener in setConfiguration
Posted by GitBox <gi...@apache.org>.
codings-dan commented on pull request #622:
URL: https://github.com/apache/ratis/pull/622#issuecomment-1066887380
@szetszwo It is a good idea to reuse the FollowerInfo interface. If reused, I think we need to add corresponding methods to distinguish Listener and follower.
--
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: issues-unsubscribe@ratis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ratis] szetszwo merged pull request #622: RATIS-1551. Suppport listener in setConfiguration
Posted by GitBox <gi...@apache.org>.
szetszwo merged pull request #622:
URL: https://github.com/apache/ratis/pull/622
--
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: issues-unsubscribe@ratis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ratis] szetszwo commented on a change in pull request #622: RATIS-1551. Suppport listener in setConfiguration
Posted by GitBox <gi...@apache.org>.
szetszwo commented on a change in pull request #622:
URL: https://github.com/apache/ratis/pull/622#discussion_r828090346
##########
File path: ratis-server/src/main/java/org/apache/ratis/server/impl/LeaderStateImpl.java
##########
@@ -235,6 +235,7 @@ boolean removeAll(Collection<LogAppender> c) {
private volatile ConfigurationStagingState stagingState;
private List<List<RaftPeerId>> voterLists;
private final Map<RaftPeerId, FollowerInfo> peerIdFollowerInfoMap = new ConcurrentHashMap<>();
+ private final Map<RaftPeerId, FollowerInfo> peerIdListenerInfoMap = new ConcurrentHashMap<>();
Review comment:
Let's just use peerIdFollowerInfoMap for now. The idea is to use conf *only* to distinguish Listeners from Followers. If we create more data structures, we need to update them when the conf is changed. It is hard to make everything 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: issues-unsubscribe@ratis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ratis] codings-dan closed pull request #622: RATIS-1551. Suppport listener in setConfiguration
Posted by GitBox <gi...@apache.org>.
codings-dan closed pull request #622:
URL: https://github.com/apache/ratis/pull/622
--
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: issues-unsubscribe@ratis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ratis] codings-dan commented on pull request #622: RATIS-1551. Suppport listener in setConfiguration
Posted by GitBox <gi...@apache.org>.
codings-dan commented on pull request #622:
URL: https://github.com/apache/ratis/pull/622#issuecomment-1070932479
@szetszwo Now that the `SetConfiguration` part has been developed, can we reuse the code of `FollowerInfo` and `FollowerState` in the next step to support Listener, so that the related code of listener is completed.
--
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: issues-unsubscribe@ratis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ratis] codings-dan commented on a change in pull request #622: RATIS-1551. Suppport listener in setConfiguration
Posted by GitBox <gi...@apache.org>.
codings-dan commented on a change in pull request #622:
URL: https://github.com/apache/ratis/pull/622#discussion_r829017695
##########
File path: ratis-server/src/main/java/org/apache/ratis/server/impl/LeaderStateImpl.java
##########
@@ -235,6 +235,7 @@ boolean removeAll(Collection<LogAppender> c) {
private volatile ConfigurationStagingState stagingState;
private List<List<RaftPeerId>> voterLists;
private final Map<RaftPeerId, FollowerInfo> peerIdFollowerInfoMap = new ConcurrentHashMap<>();
+ private final Map<RaftPeerId, FollowerInfo> peerIdListenerInfoMap = new ConcurrentHashMap<>();
Review comment:
Yean, adding a data structure is really hard to keep it 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: issues-unsubscribe@ratis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ratis] codings-dan edited a comment on pull request #622: RATIS-1551. Suppport listener in setConfiguration
Posted by GitBox <gi...@apache.org>.
codings-dan edited a comment on pull request #622:
URL: https://github.com/apache/ratis/pull/622#issuecomment-1070816876
@szetszwo Thanks for helping change the code, the introduction of this variable really reduces a lot of redundant code! I have changed the code, PTAL again, thx!
--
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: issues-unsubscribe@ratis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ratis] szetszwo merged pull request #622: RATIS-1551. Suppport listener in setConfiguration
Posted by GitBox <gi...@apache.org>.
szetszwo merged pull request #622:
URL: https://github.com/apache/ratis/pull/622
--
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: issues-unsubscribe@ratis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ratis] codings-dan edited a comment on pull request #622: RATIS-1551. Suppport listener in setConfiguration
Posted by GitBox <gi...@apache.org>.
codings-dan edited a comment on pull request #622:
URL: https://github.com/apache/ratis/pull/622#issuecomment-1070816876
@szetszwo Thanks for helping change the code, the introduction of this variable really reduces a lot of redundant code! I have changed the code, PTAL again, thx!
--
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: issues-unsubscribe@ratis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org