You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ratis.apache.org by GitBox <gi...@apache.org> on 2020/10/29 18:41:23 UTC
[GitHub] [incubator-ratis] amaliujia opened a new pull request #240: RATIS-1118. Remove DataStreamServerRpc.start()
amaliujia opened a new pull request #240:
URL: https://github.com/apache/incubator-ratis/pull/240
https://issues.apache.org/jira/browse/RATIS-1118
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-ratis] amaliujia closed pull request #240: RATIS-1118. Remove DataStreamServerRpc.start()
Posted by GitBox <gi...@apache.org>.
amaliujia closed pull request #240:
URL: https://github.com/apache/incubator-ratis/pull/240
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-ratis] amaliujia commented on pull request #240: RATIS-1118. Remove DataStreamServerRpc.start()
Posted by GitBox <gi...@apache.org>.
amaliujia commented on pull request #240:
URL: https://github.com/apache/incubator-ratis/pull/240#issuecomment-718950463
R @runzhiwang @szetszwo
Do you think wether this change make senses?
The drawback of the `start()` is people must remember to call it, which seems too error prone when there are many `start()`, e.g. it might be easy to forget start one of the rpc.
Unless there is something must be done before the data stream server start, otherwise I think we can remove the `start()` as well.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-ratis] amaliujia edited a comment on pull request #240: RATIS-1118. Remove DataStreamServerRpc.start()
Posted by GitBox <gi...@apache.org>.
amaliujia edited a comment on pull request #240:
URL: https://github.com/apache/incubator-ratis/pull/240#issuecomment-718950463
R @runzhiwang @szetszwo
Do you think whether this change make senses?
The drawback of the `start()` is people must remember to call it, which seems too error prone when there are many `start()`, e.g. it might be easy to forget start one of the rpc.
Unless there is something must be done before the data stream server start, otherwise I think we can remove the `start()` as well.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-ratis] szetszwo commented on pull request #240: RATIS-1118. Remove DataStreamServerRpc.start()
Posted by GitBox <gi...@apache.org>.
szetszwo commented on pull request #240:
URL: https://github.com/apache/incubator-ratis/pull/240#issuecomment-719118827
For servers, they usually has a start() method. I guess servers may involve more resources so that the initialisation often takes a long time. It easy to manage to separate the initialisation and the start methods. e.g. io.grpc.Server has a start method.
In contrast, clients are usually lightweight and connecting to a single server/service.
It is a bigger change to remove the DataStreamServerRpc.start() method since RaftServer also has a start() method. We must make them consistent.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-ratis] amaliujia commented on pull request #240: RATIS-1118. Remove DataStreamServerRpc.start()
Posted by GitBox <gi...@apache.org>.
amaliujia commented on pull request #240:
URL: https://github.com/apache/incubator-ratis/pull/240#issuecomment-719133192
@szetszwo makes sense.
Will close this PR now and if there is a need, we can continue discussing in the future.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org