You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ratis.apache.org by "amaliujia (via GitHub)" <gi...@apache.org> on 2026/10/21 10:18:32 UTC

[PR] RATIS-2767. ReadStream should be closed eventually [ratis]

amaliujia opened a new pull request, #1653:
URL: https://github.com/apache/ratis/pull/1653

   ## What changes were proposed in this pull request?
   
   ReadStream should be closed eventually. Ratis should be the owner to manage the stream. Application should either return normally or throw exceptions.
   
   ## What is the link to the Apache JIRA
   https://issues.apache.org/jira/browse/RATIS-2767
   
   
   ## How was this patch tested?
   
   N/A


-- 
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


Re: [PR] RATIS-2767. ReadStream should be closed eventually [ratis]

Posted by "amaliujia (via GitHub)" <gi...@apache.org>.
amaliujia commented on PR #1653:
URL: https://github.com/apache/ratis/pull/1653#issuecomment-6334042545

   @peterxcli sure applied your idea.


-- 
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


Re: [PR] RATIS-2767. ReadStream should be closed eventually [ratis]

Posted by "amaliujia (via GitHub)" <gi...@apache.org>.
amaliujia commented on PR #1653:
URL: https://github.com/apache/ratis/pull/1653#issuecomment-6332107573

   Take one step further:
   
   I think we need an enhanced API that do not expose a closeable channel. We only need to offer a wrapper of a channel that expose ` public int write(ByteBuffer src) throws IOException`.
   
   The contract between Ratis and the application is either 1) return normally when done 2) or throw proper exceptions. Then both Ratis and applicatiosn do not need to worry about when to close the channel.  Applications focus on write and throw. Ratis cares about stream close or fail and crafts proper responses to the clients.
   


-- 
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


Re: [PR] RATIS-2767. ReadStream should be closed eventually [ratis]

Posted by "peterxcli (via GitHub)" <gi...@apache.org>.
peterxcli commented on PR #1653:
URL: https://github.com/apache/ratis/pull/1653#issuecomment-6334081626

   > Take one step further:
   > 
   > I think we need an enhanced API that do not expose a closeable channel. We only need to offer a wrapper of a channel that expose ` public int write(ByteBuffer src) throws IOException`.
   > 
   > The contract between Ratis and the application is either 1) return normally when done 2) or throw proper exceptions. Then both Ratis and applicatiosn do not need to worry about when to close the channel or if channel is closed already. Applications focus on writes and throws. Ratis cares about stream close/fail and crafts proper responses to the clients.
   
   +1
   
   https://github.com/apache/ratis/blob/99e135afbaa361e96d1f3d3e94b4e122e72fdbbe/ratis-server-api/src/main/java/org/apache/ratis/server/api/DataStreamApi.java#L26-L29
   
   typically when using `java.nio.channels.FileChannel#transferTo(long, long, WritableByteChannel)` we mange the WritableByteChannel as destination in application, but in ratis, the destination is agonistic to server writing data stream response, so manage the lifecycle in ratis(ReadStreamManagement) make more sense and much straigh-forward for users.


-- 
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


Re: [PR] RATIS-2767. ReadStream should be closed eventually [ratis]

Posted by "amaliujia (via GitHub)" <gi...@apache.org>.
amaliujia merged PR #1653:
URL: https://github.com/apache/ratis/pull/1653


-- 
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


Re: [PR] RATIS-2767. ReadStream should be closed eventually [ratis]

Posted by "amaliujia (via GitHub)" <gi...@apache.org>.
amaliujia commented on PR #1653:
URL: https://github.com/apache/ratis/pull/1653#issuecomment-6334437582

   @peterxcli @szetszwo thanks for your review!
   
   @peterxcli I will carry over the proposal as followups.


-- 
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