[GitHub] sijie commented on issue #790: BP-21: New API close inconsistencies

[GitBox <gi...@apache.org>]

sijie commented on issue #790: BP-21:  New API close inconsistencies
URL: https://github.com/apache/bookkeeper/pull/790#issuecomment-347981381
 
 
   a few thoughts here:
   
   - regarding `seal` and `close`, I am not sure how much it would buy us. because the problem I see this with separated `seal` call, what does it mean if seal failed, shall users call close or not? it is very huge change to the bookkeeper, we will have to change all the documentation and have a guide about what is the difference between `seal` and `close` if we are going to make this change.
   
   - regarding `asyncClose` and `close`, it is not true in ReadHandle. it might be not very meaningful in current implementation. However at Twitter, we have been discussing about unwatch from zookeeper server when closing ledger. It is an action to clean up resources at the server side. We haven't done the change. But there is a possibility we need to clean up resources at the server side for different metadata backends in future. so from this point, I would not say 'asyncClose' is not meaningful. `asyncClose` is still need for readonly handle.
   
   - I am also concerned on removing `asyncClose`, because leaving a `close` method there is not good for async programming. because it is a bit unknown whether close would be blocking or unblocking or not. 
   
   I think if the concern is about the inconsistency about "asyncClose" and "close". I would suggest we changing the pattern to have both sync and async version in the interface, just like what pulsar client api does. it should address your inconsistent concern, and we stay with existing semantic regarding close.
   
   ```
   CompletableFuture<T> methodAsync();
   
   T method() throws BKException;
   
   CompletableFuture<Void> closeAsync();
   void close() ...;
   ```

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on 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


With regards,
Apache Git Services