[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-464?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Flavio Paiva Junqueira updated ZOOKEEPER-464:
---------------------------------------------

    Status: Open  (was: Patch Available)

Hi Erwin, It looks mostly fine. Here are a few suggestions and requests:

* When you upload a new patch, please don't delete the previous one. Also, I 
can't remember if we cancelled and re-summitted, but every time we submit a new 
version of the patch, we need to cancel the previous patch in the workflow, 
upload the new patch, and then submit again. This is to make sure that Hudson 
tests the patch.

* For documentation, the source files are under 
src/docs/src/documentation/content/xdocs. They are xml files that we compile 
with forrest to generate the documentation we have on the Web. If you can't 
figure out how to change the doc files and compile, then please open a jira and 
list the necessary modifications. It took me a while to get used to forrest, so 
don't worry too much if you can get it to work.
 
* In LedgerDeleteTest, I would suggest to replace this excerpt of code:

{noformat}
if (f.isFile() && f.getName().equals("0.log")) {
    LOG.error("Found the entry log file (0.log) that should have been deleted 
in ledgerDirectory: "
                            + ledgerDirectory);
    // This test has failed since we have found the entry Log
    // file that should have been deleted.
    assertTrue(false);
}
{noformat}

with:

{noformat}
assertFalse(f.isFile() && f.getName().equals("0.log"), "Found the entry log 
file (0.log) that should have been deleted in ledgerDirectory: " + 
ledgerDirectory);
{noformat}

I believe it is more compact and does the same, doesn't it?

* I'd like to ask you to add javadocs to the new methods. I've seen several new 
methods that have comments right before the method declaration using "//". 
Also, please review the text of the methods as you change the syntax to javadoc 
as some of them do not parse correctly.  
* Doesn't access to the following data structures in LedgerCache.deleteLedger() 
need to be synchronized:
{noformat}      
fileInfoCache
cleanLedgers
dirtyLedgers
openLedgers
{noformat}

> Need procedure to garbage collect ledgers
> -----------------------------------------
>
>                 Key: ZOOKEEPER-464
>                 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-464
>             Project: Zookeeper
>          Issue Type: New Feature
>          Components: contrib-bookkeeper
>            Reporter: Flavio Paiva Junqueira
>            Assignee: Erwin Tam
>             Fix For: 3.4.0
>
>         Attachments: ZOOKEEPER-464.patch
>
>
> An application using BookKeeper is likely to use a large number of ledgers 
> over time. Such an application might not need all ledgers created over time 
> and might want to delete some of these ledgers to free up some space on 
> bookies. The idea of this jira is to implement a procedure that enables an 
> application to garbage-collect unwanted ledgers.
> To garbage-collect a ledger, we need to delete the ledger metadata on 
> ZooKeeper, and delete the ledger data on corresponding bookies. 

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

Reply via email to