[GitHub] sijie commented on a change in pull request #795: BP-22: Separate closing ledgers from opening ledgers

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

sijie commented on a change in pull request #795: BP-22: Separate closing ledgers from opening ledgers
URL: https://github.com/apache/bookkeeper/pull/795#discussion_r154388705
 
 

 ##########
 File path: site/bps/BP-22-separate-closing-ledgers-from-opening-ledgers.md
 ##########
 @@ -0,0 +1,97 @@
+---
+title: "BP-22: Separate closing ledgers from opening ledgers"
+issue: https://github.com/apache/bookkeeper/794
+state: "Under Discussion"
+release: "4.7.0"
+---
+
+### Motivation
+
+In the beginning there was openLedger. Opening a ledger meant also figuring out what the last entry of a ledger should be, and writing it to ZooKeeper. For a long time this was the only way to read a ledger. If a writer was writing to the ledger, then anything it wrote after this point would be lost (fencing was added later). The open operation was the natural place to put recovery, as at this point it was only possible to read a recovered ledger.
+
+openLedgerNoRecovery was added in 2011. This allowed users to read from a ledger as it was being written to, which opened up a bunch of tailing uses cases. Recovery was still the default, because that was what it had always been, and tailing was still considered a secondary usecase. If the user wanted to skip recovery, they'd have to explicitly call the no recovery method.
+
+Then the new API arrived. In the new API, tailing is the primary read use case, and recovery has been demoted to a boolean flag on a builder for the open operation. The user is for the most part unaware of recovery.
+
+However, recovery is still one of the most important aspects of BookKeeper. It is the mechanism on which our Total Order Atomic Broadcast guarantees are built. It deserves to be a bit more prominent than a boolean flag in a builder. It also doesn't help that the terminology is inconsistent. The flag is called withRecovery, while the to check if recovery is needed, we call isClosed. Closed itself is ambiguous because it may refer to the local handle, or it may refer to the state of the ledger.
+
+As tailing is now the primary usecase, we expect that if a writer fails, then whichever node takes over as writer already has a non-recovered ReadHandle open. It would be nice to be able to continue using this Handle to read to the end of the ledger.
 
 Review comment:
   - not we are not saying streaming is the primary purpose. the points in #774 are open by default should be non invasive. 
   
   - I think we shouldn't have a api that would touch ReadHandle. `bk.forceSealedOp.withLedgerId(..)` works for me.

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