[
https://issues.apache.org/jira/browse/BOOKKEEPER-463?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13535847#comment-13535847
]
Flavio Junqueira commented on BOOKKEEPER-463:
---------------------------------------------
Most of my comments were just fixing the wording of comments in the patch and I
can actually take care of them if you'd like. There are three other comments
that are not about wording. Sijie has responded to one of them, the one about
the packages and it sounds like we could consider it moving forward (should we
create a jira for it?). The other two are the following:
bq. ScanAndCompareGarbageCollector.java. Is it possible that
ledgerManager.getLedgerRanges(); returns an empty iterator in the case of an
error? I'm worried that this block "if (!ledgerRangeIterator.hasNext())..."
could end up deleting all active ledgers by mistake".
I had a look at the code and it sounds like in the case that we have an error
in the ledger manager, we can have an NPE, but not delete active ledgers
accidentally. In particular, looking at FlatLedgerManager#getLedgerRanges, I
observed the following.Say that getChildrenInSingleNode returns a null pointer
for the children list. I believe ledgerListToSet will throw an exception (NPE)
when trying to execute the for loop.
Perhaps what we should do is check if ledgerNodes is null in ledgerListToSet
and return null in this case. If zkActiveLedgers is null, then we throw an
IOException. In general, the problem I see with this approach is that it is
hard to guarantee that all implementations of LedgerManager#getLedgerRanges
will do the right thing.
bq. removeLedgerMetadata. There are two cases in which we return
BKException.Code.MetadataVersionException and we don't give any more
information. It would be good to give a better hint on what the actual error
was.
The two solutions I see here are to have different error codes or to log
different messages. Logging different messages seems simpler.
> Refactor garbage collection code for ease to plugin different GC algorithm.
> ---------------------------------------------------------------------------
>
> Key: BOOKKEEPER-463
> URL: https://issues.apache.org/jira/browse/BOOKKEEPER-463
> Project: Bookkeeper
> Issue Type: Sub-task
> Components: bookkeeper-server
> Reporter: Sijie Guo
> Assignee: Fangmin Lv
> Fix For: 4.2.0
>
> Attachments:
> 0001-BOOKKEEPER-463-Refactor-garbage-collection-code-for-.patch,
> BOOKKEEPER-463.patch, BOOKKEEPER-463.patch, BOOKKEEPER-463.patch,
> BOOKKEEPER-463.patch, BOOKKEEPER-463.patch, BOOKKEEPER-463.patch,
> BOOKKEEPER-463.patch, BOOKKEEPER-463.patch, BOOKKEEPER-463.patch
>
>
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
