[GitHub] sijie commented on issue #795: BP-22: Separate closing ledgers from opening ledgers

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

sijie commented on issue #795: BP-22: Separate closing ledgers from opening ledgers
URL: https://github.com/apache/bookkeeper/pull/795#issuecomment-348668404
 
 
   @ivankelly - There are too many back and forth here. let me try to summarize my opinion here. so it is clear rather than spreading over multiple comments. 
   
   - I am -1 on `CompletableFuture<Void> seal(ReadHandle handle)`. To me, 1) this call makes me feel it is _mutating_ a readhandle, which I don't feel comfortable. 2) it has similar meaning as `ReadHandle#seal()`, which doesn't sound correct to me. ReadHandle shouldn't any invasive methods.
   - If this BP is proposing separating the `seal` operation from `open` operation, I would prefer what @eolivelli proposed. the `seal` operation should only deal with a `ledgerId` rather than a `ReadHandle`. So a seal operation builder with `ledgerId` works for me. I will give my +1 if that is the case.
   - Assume we agree on a `seal` builder eventually, I would still prefer open builder keep `withRecovery(true)` there. I need this for distributedlog. I am okay with renaming it from `#forceClose`, `#forceSeal`, `sealing` or whatever name we agree on in the open builder.
   
   All these are just my opinions (they can be wrong, but they are based on all the experiences that I deal with ledgers). Let's see other's opinions. 

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