[
https://issues.apache.org/jira/browse/BOOKKEEPER-220?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13286373#comment-13286373
]
Sijie Guo commented on BOOKKEEPER-220:
--------------------------------------
@Matteo, thanks for great work on managed ledger.
Some comments as below:
1. ManagedLedgerFactoryImpl with zookeeper quorum string created its owned
zookeeper handle and bookkeeper handle but doesn't close them.
2. You did a lot of work in the constructor of ManagedLedgerImpl, which is not
a good practice.
{code}
@Override
public ManagedLedger open(String name, ManagedLedgerConfig config) throws
Exception {
ManagedLedger ledger = ledgers.get(name);
if (ledger != null) {
log.info("Reusing opened ManagedLedger: {}", name);
return ledger;
} else {
ledger = new ManagedLedgerImpl(this, bookKeeper, store, config,
executor, name);
ManagedLedger oldValue = ledgers.putIfAbsent(name, ledger);
if (oldValue != null)
return oldValue;
else
return ledger;
}
}
{code}
If two thread open the same managed ledger concurrently, it would construct two
different managedledgerimpl object. during these two objects construction, they
would try to update same metadata. How to resolve such confliction? since you
metastore api doesn't provide some conditional write interface to ensure
consistency. I would suggest adding conditional write interface in you
metastore. You could refer BOOKKEEPER-250 (the MetadataManager interface I
added for Hedwig to update its persistence info).
3. async & sync operations. seems that you using executor to change sync
operation to async. why not leverage the async interface provided by bookkeeper
directly? And implement the sync operation based async operation.
4. in line 183 of ManagedLedgerImpl, you use lastLedger.getLastAddConfirmed()
as entry id to return the position. If concurrent addEntry operations arrives,
you would read wrong entry id. why not use the entryid returned by
lastLedger#addEntry. Besides that, I think getLastAddConfirmed doesn't equal to
entryid you added for a writable ledger.
5. for asyncAddEntry, addEntry in ManagedLedgerImpl, you don't change ledger
when addEntry failed. The ledger would be closed when adding entry failed, you
could not add more entries into it even the system goes back. You could refer
BOOKKEEPER-74.
6. trimConsumedLedgers would be triggered immediately if necessary when
updating cursors. It might take time to deleteLedger & updateLedgersId. Is is
possible to trim the ledgers lazily, e.g. running in background?
> 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
>
>
> 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
