[
https://issues.apache.org/jira/browse/BOOKKEEPER-220?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13293311#comment-13293311
]
Matteo Merli commented on BOOKKEEPER-220:
-----------------------------------------
@Ivan
bq. I've just taken a look a the code. I like it a lot. It's very nice, neat
and readable. I like the direction it's going. I have a few concerns though.
Thank you for reviewing it.
bq. My biggest concern is that the implementation doesn't take precautions so
that split brain does not occur. If a managed ledger is being used as a write
ahead log for node A, and node B falsely suspects that A is down, it can start
writing to the the write ahead log for A and there's nothing to stop them. This
isn't very difficult to prevent, but it needs to be done in your code. You need
to use versioned zookeeper writes in the metastore, and ensure that when you
create a ledger, all previous ledgers are closed.
Yes, there is actually no enforcement that makes sure the a managed ledger is
running only on one box and it would be definitely better to have some kind of
protection. I wasn't sure on how to implement it (my first thought
was to have a lock on zk), and the other reason is that from our side we are
already protecting this using a zk lock (a topic maps to one managed ledger and
one topic is served by one broker).
Regarding versioned writes, what should be the behavior when a versioned write
fails (meaning someone else is messing up with the managed ledger) ? Versioned
writes would make the state consistent but there would still be problems in
running the managed ledgers from different machines (consumer not getting
messages,
And, how can I verify that a ledger is closed for writing (I think I'd need to
check only the last ledger in the list) ?
bq. Another potential issue in the future is that the async and sync versions
of addEntry do not share a code path. This could lead to bugs in the future
which get fixed on one and not on the other. It would be better to have a
completely async version, and then implement the sync version by calling the
async and waiting on a countdown latch. Also, I think it would be nicer to have
asyncAddEntry using the async calls and callbacks rather than a worker thread
OR if using the worker threads, using the actor pattern. I prefer the former,
as it will perform better as more requests can be outstanding at a time, but
the latter works too.
I'm all for making (at least) asyncAddEntry truly async and unifying the
sync/async path. My only concern is how to deal when having to call more than
one
BK async method. For example, asyncAddEntry can require to create a new ledger
if the current one is full. That would mean to call bk.asyncCreateLedger() and
in the callback use the newly created ledger with ledger.asyncAddEntry(). Is
this feasable? I think (maybe I'm wrong) that it's not safe to call BK api
methods from inside a BK callback.
Current implementation is cumbersome, but it's truly async in the most common
case of addEntry, when not crossing ledger boundaries.
bq. Instead of having managed cursor return a list of entries, how about having
managed cursor itself present a iterator type interface where you can keep
calling next() to get the next entry?
Yes, I initially made the readEntries() that returns a list because I thought
it was easier to get and easier to use for batch sending and let the client to
pick up the batch size, but I'm open to change it. What I would'n want is to
have both readEntries and the iterator style because that would be confusing.
bq. One last small thing, is that it would be better to use a custom exception,
rather than java.lang.Exception.
Ok, I agree with that. What would be more appropriate, to use BKException or
some other one (MLException, ...) ?
> Managed Ledger proposal
> -----------------------
>
> Key: BOOKKEEPER-220
> URL: https://issues.apache.org/jira/browse/BOOKKEEPER-220
> Project: Bookkeeper
> Issue Type: New Feature
> Components: bookkeeper-client
> Reporter: Matteo Merli
> Assignee: Matteo Merli
> Fix For: 4.2.0
>
> Attachments: 0001-BOOKKEEPER-220-Managed-Ledger-proposal.patch,
> 0001-BOOKKEEPER-220-Managed-Ledger-proposal.patch
>
>
> The ManagedLedger design is based on our need to manage a set of ledgers,
> with a single writer (at any point in time) and a set on consumers that read
> entries from it.
> The ManagedLedger also takes care of periodically closing ledgers to have a
> "reasonable" sized sets of ledgers that can individually deleted when no more
> needed.
> I've put on github the interface proposal (along with an early WIP
> implementation)
> http://github.com/merlimat/managed-ledger
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators:
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
