You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@bookkeeper.apache.org by "Ivan Kelly (Created) (JIRA)" <ji...@apache.org> on 2011/10/24 11:57:32 UTC

[jira] [Created] (BOOKKEEPER-89) Bookkeeper API changes for initial Bookkeeper release

Bookkeeper API changes for initial Bookkeeper release
-----------------------------------------------------

                 Key: BOOKKEEPER-89
                 URL: https://issues.apache.org/jira/browse/BOOKKEEPER-89
             Project: Bookkeeper
          Issue Type: Improvement
            Reporter: Ivan Kelly
            Assignee: Ivan Kelly


Changes are as follows.

BookKeeper#createLedger, parameter is named passwd, "Key" used in LedgerHandle api
BookKeeper#getBookieClient shouldn't be public
BookKeeper#createComplete shouldn't be public
BookKeeper#openComplete shouldn't be public
BookKeeper#deleteComplete shouldn't be public
BookKeeper#halt could be changed to close(), should throw a BKException

LedgerHandle#getLedgerKey passwd is used in BookKeeper, should possibly be private
LedgerHandle#getLedgerMetadata shouldn't be public
LedgerHandle#getDigestManager shouldn't be public
LedgerHandle#getDistributionSchedule shouldn't be public
LedgerHandle#writeLedgerConfig shouldn't be public
LedgerHandle#addEntry should return void, errors should go in an Exception
LedgerHandle#readComplete should not be public
LedgerHandle#addComplete should not be public
LedgerHandle#readLastConfirmedCompelte should not be public
LedgerHandle#closeComplete should not be public

ASyncCallback#RecoverCallback shouldn't be public


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

        

[jira] [Updated] (BOOKKEEPER-89) Bookkeeper API changes for initial Bookkeeper release

Posted by "Ivan Kelly (Updated) (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/BOOKKEEPER-89?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Ivan Kelly updated BOOKKEEPER-89:
---------------------------------

    Attachment: BOOKKEEPER-89.diff
    
> Bookkeeper API changes for initial Bookkeeper release
> -----------------------------------------------------
>
>                 Key: BOOKKEEPER-89
>                 URL: https://issues.apache.org/jira/browse/BOOKKEEPER-89
>             Project: Bookkeeper
>          Issue Type: Improvement
>            Reporter: Ivan Kelly
>            Assignee: Ivan Kelly
>             Fix For: 4.0.0
>
>         Attachments: BOOKKEEPER-89.diff, BOOKKEEPER-89.diff, BOOKKEEPER-89.diff
>
>
> Changes are as follows.
> BookKeeper#createLedger, parameter is named passwd, "Key" used in LedgerHandle api
> BookKeeper#getBookieClient shouldn't be public
> BookKeeper#createComplete shouldn't be public
> BookKeeper#openComplete shouldn't be public
> BookKeeper#deleteComplete shouldn't be public
> BookKeeper#halt could be changed to close(), should throw a BKException
> LedgerHandle#getLedgerKey passwd is used in BookKeeper, should possibly be private
> LedgerHandle#getLedgerMetadata shouldn't be public
> LedgerHandle#getDigestManager shouldn't be public
> LedgerHandle#getDistributionSchedule shouldn't be public
> LedgerHandle#writeLedgerConfig shouldn't be public
> LedgerHandle#addEntry should return void, errors should go in an Exception
> LedgerHandle#readComplete should not be public
> LedgerHandle#addComplete should not be public
> LedgerHandle#readLastConfirmedCompelte should not be public
> LedgerHandle#closeComplete should not be public
> ASyncCallback#RecoverCallback shouldn't be public

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

        

[jira] [Commented] (BOOKKEEPER-89) Bookkeeper API changes for initial Bookkeeper release

Posted by "Ivan Kelly (Commented) (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/BOOKKEEPER-89?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13135952#comment-13135952 ] 

Ivan Kelly commented on BOOKKEEPER-89:
--------------------------------------

Actually, Sijie had already spotted this problem. BOOKKEEPER-94.
                
> Bookkeeper API changes for initial Bookkeeper release
> -----------------------------------------------------
>
>                 Key: BOOKKEEPER-89
>                 URL: https://issues.apache.org/jira/browse/BOOKKEEPER-89
>             Project: Bookkeeper
>          Issue Type: Improvement
>            Reporter: Ivan Kelly
>            Assignee: Ivan Kelly
>             Fix For: 4.0.0
>
>         Attachments: BOOKKEEPER-89.diff, BOOKKEEPER-89.diff, BOOKKEEPER-89.diff, BOOKKEEPER-89.diff, TEST-org.apache.bookkeeper.test.BookieReadWriteTest.xml
>
>
> Changes are as follows.
> BookKeeper#createLedger, parameter is named passwd, "Key" used in LedgerHandle api
> BookKeeper#getBookieClient shouldn't be public
> BookKeeper#createComplete shouldn't be public
> BookKeeper#openComplete shouldn't be public
> BookKeeper#deleteComplete shouldn't be public
> BookKeeper#halt could be changed to close(), should throw a BKException
> LedgerHandle#getLedgerKey passwd is used in BookKeeper, should possibly be private
> LedgerHandle#getLedgerMetadata shouldn't be public
> LedgerHandle#getDigestManager shouldn't be public
> LedgerHandle#getDistributionSchedule shouldn't be public
> LedgerHandle#writeLedgerConfig shouldn't be public
> LedgerHandle#addEntry should return void, errors should go in an Exception
> LedgerHandle#readComplete should not be public
> LedgerHandle#addComplete should not be public
> LedgerHandle#readLastConfirmedCompelte should not be public
> LedgerHandle#closeComplete should not be public
> ASyncCallback#RecoverCallback shouldn't be public

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

        

[jira] [Commented] (BOOKKEEPER-89) Bookkeeper API changes for initial Bookkeeper release

Posted by "Flavio Junqueira (Commented) (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/BOOKKEEPER-89?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13136354#comment-13136354 ] 

Flavio Junqueira commented on BOOKKEEPER-89:
--------------------------------------------

+1, looks great!
                
> Bookkeeper API changes for initial Bookkeeper release
> -----------------------------------------------------
>
>                 Key: BOOKKEEPER-89
>                 URL: https://issues.apache.org/jira/browse/BOOKKEEPER-89
>             Project: Bookkeeper
>          Issue Type: Improvement
>            Reporter: Ivan Kelly
>            Assignee: Ivan Kelly
>             Fix For: 4.0.0
>
>         Attachments: BOOKKEEPER-89.diff, BOOKKEEPER-89.diff, BOOKKEEPER-89.diff, BOOKKEEPER-89.diff, BOOKKEEPER-89.diff, TEST-org.apache.bookkeeper.test.BookieReadWriteTest.xml
>
>
> Changes are as follows.
> BookKeeper#createLedger, parameter is named passwd, "Key" used in LedgerHandle api
> BookKeeper#getBookieClient shouldn't be public
> BookKeeper#createComplete shouldn't be public
> BookKeeper#openComplete shouldn't be public
> BookKeeper#deleteComplete shouldn't be public
> BookKeeper#halt could be changed to close(), should throw a BKException
> LedgerHandle#getLedgerKey passwd is used in BookKeeper, should possibly be private
> LedgerHandle#getLedgerMetadata shouldn't be public
> LedgerHandle#getDigestManager shouldn't be public
> LedgerHandle#getDistributionSchedule shouldn't be public
> LedgerHandle#writeLedgerConfig shouldn't be public
> LedgerHandle#addEntry should return void, errors should go in an Exception
> LedgerHandle#readComplete should not be public
> LedgerHandle#addComplete should not be public
> LedgerHandle#readLastConfirmedCompelte should not be public
> LedgerHandle#closeComplete should not be public
> ASyncCallback#RecoverCallback shouldn't be public

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

        

[jira] [Commented] (BOOKKEEPER-89) Bookkeeper API changes for initial Bookkeeper release

Posted by "Benjamin Reed (Commented) (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/BOOKKEEPER-89?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13137211#comment-13137211 ] 

Benjamin Reed commented on BOOKKEEPER-89:
-----------------------------------------

+1 yes, i think you are right ivan. i don't feel strongly about it.
                
> Bookkeeper API changes for initial Bookkeeper release
> -----------------------------------------------------
>
>                 Key: BOOKKEEPER-89
>                 URL: https://issues.apache.org/jira/browse/BOOKKEEPER-89
>             Project: Bookkeeper
>          Issue Type: Improvement
>            Reporter: Ivan Kelly
>            Assignee: Ivan Kelly
>             Fix For: 4.0.0
>
>         Attachments: BOOKKEEPER-89.diff, BOOKKEEPER-89.diff, BOOKKEEPER-89.diff, BOOKKEEPER-89.diff, BOOKKEEPER-89.diff, TEST-org.apache.bookkeeper.test.BookieReadWriteTest.xml
>
>
> Changes are as follows.
> BookKeeper#createLedger, parameter is named passwd, "Key" used in LedgerHandle api
> BookKeeper#getBookieClient shouldn't be public
> BookKeeper#createComplete shouldn't be public
> BookKeeper#openComplete shouldn't be public
> BookKeeper#deleteComplete shouldn't be public
> BookKeeper#halt could be changed to close(), should throw a BKException
> LedgerHandle#getLedgerKey passwd is used in BookKeeper, should possibly be private
> LedgerHandle#getLedgerMetadata shouldn't be public
> LedgerHandle#getDigestManager shouldn't be public
> LedgerHandle#getDistributionSchedule shouldn't be public
> LedgerHandle#writeLedgerConfig shouldn't be public
> LedgerHandle#addEntry should return void, errors should go in an Exception
> LedgerHandle#readComplete should not be public
> LedgerHandle#addComplete should not be public
> LedgerHandle#readLastConfirmedCompelte should not be public
> LedgerHandle#closeComplete should not be public
> ASyncCallback#RecoverCallback shouldn't be public

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

        

[jira] [Updated] (BOOKKEEPER-89) Bookkeeper API changes for initial Bookkeeper release

Posted by "Flavio Junqueira (Updated) (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/BOOKKEEPER-89?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Flavio Junqueira updated BOOKKEEPER-89:
---------------------------------------

    Attachment: TEST-org.apache.bookkeeper.test.BookieReadWriteTest.xml
    
> Bookkeeper API changes for initial Bookkeeper release
> -----------------------------------------------------
>
>                 Key: BOOKKEEPER-89
>                 URL: https://issues.apache.org/jira/browse/BOOKKEEPER-89
>             Project: Bookkeeper
>          Issue Type: Improvement
>            Reporter: Ivan Kelly
>            Assignee: Ivan Kelly
>             Fix For: 4.0.0
>
>         Attachments: BOOKKEEPER-89.diff, BOOKKEEPER-89.diff, BOOKKEEPER-89.diff, BOOKKEEPER-89.diff, TEST-org.apache.bookkeeper.test.BookieReadWriteTest.xml
>
>
> Changes are as follows.
> BookKeeper#createLedger, parameter is named passwd, "Key" used in LedgerHandle api
> BookKeeper#getBookieClient shouldn't be public
> BookKeeper#createComplete shouldn't be public
> BookKeeper#openComplete shouldn't be public
> BookKeeper#deleteComplete shouldn't be public
> BookKeeper#halt could be changed to close(), should throw a BKException
> LedgerHandle#getLedgerKey passwd is used in BookKeeper, should possibly be private
> LedgerHandle#getLedgerMetadata shouldn't be public
> LedgerHandle#getDigestManager shouldn't be public
> LedgerHandle#getDistributionSchedule shouldn't be public
> LedgerHandle#writeLedgerConfig shouldn't be public
> LedgerHandle#addEntry should return void, errors should go in an Exception
> LedgerHandle#readComplete should not be public
> LedgerHandle#addComplete should not be public
> LedgerHandle#readLastConfirmedCompelte should not be public
> LedgerHandle#closeComplete should not be public
> ASyncCallback#RecoverCallback shouldn't be public

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

        

[jira] [Commented] (BOOKKEEPER-89) Bookkeeper API changes for initial Bookkeeper release

Posted by "Ivan Kelly (Commented) (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/BOOKKEEPER-89?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13137248#comment-13137248 ] 

Ivan Kelly commented on BOOKKEEPER-89:
--------------------------------------

Committed as r1189842. Thanks for reviewing, guys.
                
> Bookkeeper API changes for initial Bookkeeper release
> -----------------------------------------------------
>
>                 Key: BOOKKEEPER-89
>                 URL: https://issues.apache.org/jira/browse/BOOKKEEPER-89
>             Project: Bookkeeper
>          Issue Type: Improvement
>            Reporter: Ivan Kelly
>            Assignee: Ivan Kelly
>             Fix For: 4.0.0
>
>         Attachments: BOOKKEEPER-89.diff, BOOKKEEPER-89.diff, BOOKKEEPER-89.diff, BOOKKEEPER-89.diff, BOOKKEEPER-89.diff, TEST-org.apache.bookkeeper.test.BookieReadWriteTest.xml
>
>
> Changes are as follows.
> BookKeeper#createLedger, parameter is named passwd, "Key" used in LedgerHandle api
> BookKeeper#getBookieClient shouldn't be public
> BookKeeper#createComplete shouldn't be public
> BookKeeper#openComplete shouldn't be public
> BookKeeper#deleteComplete shouldn't be public
> BookKeeper#halt could be changed to close(), should throw a BKException
> LedgerHandle#getLedgerKey passwd is used in BookKeeper, should possibly be private
> LedgerHandle#getLedgerMetadata shouldn't be public
> LedgerHandle#getDigestManager shouldn't be public
> LedgerHandle#getDistributionSchedule shouldn't be public
> LedgerHandle#writeLedgerConfig shouldn't be public
> LedgerHandle#addEntry should return void, errors should go in an Exception
> LedgerHandle#readComplete should not be public
> LedgerHandle#addComplete should not be public
> LedgerHandle#readLastConfirmedCompelte should not be public
> LedgerHandle#closeComplete should not be public
> ASyncCallback#RecoverCallback shouldn't be public

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

        

[jira] [Updated] (BOOKKEEPER-89) Bookkeeper API changes for initial Bookkeeper release

Posted by "Ivan Kelly (Updated) (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/BOOKKEEPER-89?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Ivan Kelly updated BOOKKEEPER-89:
---------------------------------

    Attachment: BOOKKEEPER-89.diff

Implemented changes as in description. RecoveryCallback is still public as BookKeeperTools needs it. BookKeeperTools has been moved in to the client package.
                
> Bookkeeper API changes for initial Bookkeeper release
> -----------------------------------------------------
>
>                 Key: BOOKKEEPER-89
>                 URL: https://issues.apache.org/jira/browse/BOOKKEEPER-89
>             Project: Bookkeeper
>          Issue Type: Improvement
>            Reporter: Ivan Kelly
>            Assignee: Ivan Kelly
>         Attachments: BOOKKEEPER-89.diff
>
>
> Changes are as follows.
> BookKeeper#createLedger, parameter is named passwd, "Key" used in LedgerHandle api
> BookKeeper#getBookieClient shouldn't be public
> BookKeeper#createComplete shouldn't be public
> BookKeeper#openComplete shouldn't be public
> BookKeeper#deleteComplete shouldn't be public
> BookKeeper#halt could be changed to close(), should throw a BKException
> LedgerHandle#getLedgerKey passwd is used in BookKeeper, should possibly be private
> LedgerHandle#getLedgerMetadata shouldn't be public
> LedgerHandle#getDigestManager shouldn't be public
> LedgerHandle#getDistributionSchedule shouldn't be public
> LedgerHandle#writeLedgerConfig shouldn't be public
> LedgerHandle#addEntry should return void, errors should go in an Exception
> LedgerHandle#readComplete should not be public
> LedgerHandle#addComplete should not be public
> LedgerHandle#readLastConfirmedCompelte should not be public
> LedgerHandle#closeComplete should not be public
> ASyncCallback#RecoverCallback shouldn't be public

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

        

[jira] [Commented] (BOOKKEEPER-89) Bookkeeper API changes for initial Bookkeeper release

Posted by "Ivan Kelly (Commented) (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/BOOKKEEPER-89?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13135933#comment-13135933 ] 

Ivan Kelly commented on BOOKKEEPER-89:
--------------------------------------

Failure is unrelated. Is related to BOOKKEEPER-92, which I think has exposed a bug. I'll try and fix now and submit a new JIRA for it. Hopefully Jenkins will catch this sort of thing once ZK 3.4 is out.


                
> Bookkeeper API changes for initial Bookkeeper release
> -----------------------------------------------------
>
>                 Key: BOOKKEEPER-89
>                 URL: https://issues.apache.org/jira/browse/BOOKKEEPER-89
>             Project: Bookkeeper
>          Issue Type: Improvement
>            Reporter: Ivan Kelly
>            Assignee: Ivan Kelly
>             Fix For: 4.0.0
>
>         Attachments: BOOKKEEPER-89.diff, BOOKKEEPER-89.diff, BOOKKEEPER-89.diff, BOOKKEEPER-89.diff, TEST-org.apache.bookkeeper.test.BookieReadWriteTest.xml
>
>
> Changes are as follows.
> BookKeeper#createLedger, parameter is named passwd, "Key" used in LedgerHandle api
> BookKeeper#getBookieClient shouldn't be public
> BookKeeper#createComplete shouldn't be public
> BookKeeper#openComplete shouldn't be public
> BookKeeper#deleteComplete shouldn't be public
> BookKeeper#halt could be changed to close(), should throw a BKException
> LedgerHandle#getLedgerKey passwd is used in BookKeeper, should possibly be private
> LedgerHandle#getLedgerMetadata shouldn't be public
> LedgerHandle#getDigestManager shouldn't be public
> LedgerHandle#getDistributionSchedule shouldn't be public
> LedgerHandle#writeLedgerConfig shouldn't be public
> LedgerHandle#addEntry should return void, errors should go in an Exception
> LedgerHandle#readComplete should not be public
> LedgerHandle#addComplete should not be public
> LedgerHandle#readLastConfirmedCompelte should not be public
> LedgerHandle#closeComplete should not be public
> ASyncCallback#RecoverCallback shouldn't be public

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

        

[jira] [Commented] (BOOKKEEPER-89) Bookkeeper API changes for initial Bookkeeper release

Posted by "Benjamin Reed (Commented) (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/BOOKKEEPER-89?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13136010#comment-13136010 ] 

Benjamin Reed commented on BOOKKEEPER-89:
-----------------------------------------

looks great. i like how you moved the callbacks from the class to inner classes! my one comment is that i think we should keep the accessor for the ZooKeeper public. if you are using bookkeeper and you want to do some zookeeper operations, usually you want to use the same zookeeper object to keep any session events in sync.
                
> Bookkeeper API changes for initial Bookkeeper release
> -----------------------------------------------------
>
>                 Key: BOOKKEEPER-89
>                 URL: https://issues.apache.org/jira/browse/BOOKKEEPER-89
>             Project: Bookkeeper
>          Issue Type: Improvement
>            Reporter: Ivan Kelly
>            Assignee: Ivan Kelly
>             Fix For: 4.0.0
>
>         Attachments: BOOKKEEPER-89.diff, BOOKKEEPER-89.diff, BOOKKEEPER-89.diff, BOOKKEEPER-89.diff, BOOKKEEPER-89.diff, TEST-org.apache.bookkeeper.test.BookieReadWriteTest.xml
>
>
> Changes are as follows.
> BookKeeper#createLedger, parameter is named passwd, "Key" used in LedgerHandle api
> BookKeeper#getBookieClient shouldn't be public
> BookKeeper#createComplete shouldn't be public
> BookKeeper#openComplete shouldn't be public
> BookKeeper#deleteComplete shouldn't be public
> BookKeeper#halt could be changed to close(), should throw a BKException
> LedgerHandle#getLedgerKey passwd is used in BookKeeper, should possibly be private
> LedgerHandle#getLedgerMetadata shouldn't be public
> LedgerHandle#getDigestManager shouldn't be public
> LedgerHandle#getDistributionSchedule shouldn't be public
> LedgerHandle#writeLedgerConfig shouldn't be public
> LedgerHandle#addEntry should return void, errors should go in an Exception
> LedgerHandle#readComplete should not be public
> LedgerHandle#addComplete should not be public
> LedgerHandle#readLastConfirmedCompelte should not be public
> LedgerHandle#closeComplete should not be public
> ASyncCallback#RecoverCallback shouldn't be public

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

        

[jira] [Updated] (BOOKKEEPER-89) Bookkeeper API changes for initial Bookkeeper release

Posted by "Ivan Kelly (Updated) (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/BOOKKEEPER-89?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Ivan Kelly updated BOOKKEEPER-89:
---------------------------------

    Attachment: BOOKKEEPER-89.diff

One final change. I had forgotten to change addEntry to return an exception instead of a return code. 
                
> Bookkeeper API changes for initial Bookkeeper release
> -----------------------------------------------------
>
>                 Key: BOOKKEEPER-89
>                 URL: https://issues.apache.org/jira/browse/BOOKKEEPER-89
>             Project: Bookkeeper
>          Issue Type: Improvement
>            Reporter: Ivan Kelly
>            Assignee: Ivan Kelly
>             Fix For: 4.0.0
>
>         Attachments: BOOKKEEPER-89.diff, BOOKKEEPER-89.diff, BOOKKEEPER-89.diff, BOOKKEEPER-89.diff, BOOKKEEPER-89.diff, TEST-org.apache.bookkeeper.test.BookieReadWriteTest.xml
>
>
> Changes are as follows.
> BookKeeper#createLedger, parameter is named passwd, "Key" used in LedgerHandle api
> BookKeeper#getBookieClient shouldn't be public
> BookKeeper#createComplete shouldn't be public
> BookKeeper#openComplete shouldn't be public
> BookKeeper#deleteComplete shouldn't be public
> BookKeeper#halt could be changed to close(), should throw a BKException
> LedgerHandle#getLedgerKey passwd is used in BookKeeper, should possibly be private
> LedgerHandle#getLedgerMetadata shouldn't be public
> LedgerHandle#getDigestManager shouldn't be public
> LedgerHandle#getDistributionSchedule shouldn't be public
> LedgerHandle#writeLedgerConfig shouldn't be public
> LedgerHandle#addEntry should return void, errors should go in an Exception
> LedgerHandle#readComplete should not be public
> LedgerHandle#addComplete should not be public
> LedgerHandle#readLastConfirmedCompelte should not be public
> LedgerHandle#closeComplete should not be public
> ASyncCallback#RecoverCallback shouldn't be public

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

        

[jira] [Commented] (BOOKKEEPER-89) Bookkeeper API changes for initial Bookkeeper release

Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/BOOKKEEPER-89?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13135805#comment-13135805 ] 

jiraposter@reviews.apache.org commented on BOOKKEEPER-89:
---------------------------------------------------------



bq.  On 2011-10-26 00:03:20, fpj wrote:
bq.  > bookkeeper-server/src/main/java/org/apache/bookkeeper/client/BookKeeper.java, line 187
bq.  > <https://reviews.apache.org/r/2543/diff/2/?file=52724#file52724line187>
bq.  >
bq.  >     We differentiate between password and ledger key in BookKeeper. The password is the string that the application passes to bookkeeper when creating a ledger. The password is used to generate the ledger key (check the constructor of ledger handle) and the mac key when MAC is being used.

Ah, I'll change this back then and update the javadoc for LedgerHandle#getLedgerKey


bq.  On 2011-10-26 00:03:20, fpj wrote:
bq.  > bookkeeper-server/src/main/java/org/apache/bookkeeper/client/BookKeeperTools.java, line 547
bq.  > <https://reviews.apache.org/r/2543/diff/2/?file=52725#file52725line547>
bq.  >
bq.  >     Is this a tab?

Nope, it's line which contains only whitespace, in this case spaces. Its considered bad style, so thats probably why it's on show here.


bq.  On 2011-10-26 00:03:20, fpj wrote:
bq.  > bookkeeper-server/src/test/java/org/apache/bookkeeper/client/LedgerCacheTest.java, line 22
bq.  > <https://reviews.apache.org/r/2543/diff/2/?file=52732#file52732line22>
bq.  >
bq.  >     Why are we moving this test to a different package?

It accesses MacDigestManager, which is now package private. In general I think it's better to have tests in the same package as the code they're testing, so that they can poke at internals without having to open them to the world.


bq.  On 2011-10-26 00:03:20, fpj wrote:
bq.  > bookkeeper-server/src/main/java/org/apache/bookkeeper/client/BookKeeperTools.java, line 1
bq.  > <https://reviews.apache.org/r/2543/diff/2/?file=52725#file52725line1>
bq.  >
bq.  >     I understand that these tools need visibility to some internal methods of the client package, but this class is not really part of the bookkeeper client. It is instead a tool for things like bookie recovery.
bq.  >     
bq.  >     I was wondering if we should at least have it in a separate folder to avoid confusion.

It's a tricky one. While it is toollike, it reaches into the internals of the client code. Really a tool should just use the public api itself. How about this? I'll move the BookKeeperTools back to tools, and put the main() in it, but then I'll create a class BookKeeperAdmin which acts as an admin client and implements the code that reaches into the internals. Then BookKeeperTools can use the public methods exposed by BookKeeperAdmin.


- Ivan


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/2543/#review2843
-----------------------------------------------------------


On 2011-10-24 14:53:25, Ivan Kelly wrote:
bq.  
bq.  -----------------------------------------------------------
bq.  This is an automatically generated e-mail. To reply, visit:
bq.  https://reviews.apache.org/r/2543/
bq.  -----------------------------------------------------------
bq.  
bq.  (Updated 2011-10-24 14:53:25)
bq.  
bq.  
bq.  Review request for bookkeeper.
bq.  
bq.  
bq.  Summary
bq.  -------
bq.  
bq.  Changes are as follows.
bq.  
bq.  BookKeeper#createLedger, parameter is named passwd, "Key" used in LedgerHandle api
bq.  BookKeeper#getBookieClient shouldn't be public
bq.  BookKeeper#createComplete shouldn't be public
bq.  BookKeeper#openComplete shouldn't be public
bq.  BookKeeper#deleteComplete shouldn't be public
bq.  BookKeeper#halt could be changed to close(), should throw a BKException
bq.  
bq.  LedgerHandle#getLedgerKey passwd is used in BookKeeper, should possibly be private
bq.  LedgerHandle#getLedgerMetadata shouldn't be public
bq.  LedgerHandle#getDigestManager shouldn't be public
bq.  LedgerHandle#getDistributionSchedule shouldn't be public
bq.  LedgerHandle#writeLedgerConfig shouldn't be public
bq.  LedgerHandle#addEntry should return void, errors should go in an Exception
bq.  LedgerHandle#readComplete should not be public
bq.  LedgerHandle#addComplete should not be public
bq.  LedgerHandle#readLastConfirmedCompelte should not be public
bq.  LedgerHandle#closeComplete should not be public
bq.  
bq.  ASyncCallback#RecoverCallback shouldn't be public
bq.  
bq.  
bq.  This addresses bug BOOKKEEPER-89.
bq.      https://issues.apache.org/jira/browse/BOOKKEEPER-89
bq.  
bq.  
bq.  Diffs
bq.  -----
bq.  
bq.    bookkeeper-server/src/main/java/org/apache/bookkeeper/client/AsyncCallback.java 6421460 
bq.    bookkeeper-server/src/main/java/org/apache/bookkeeper/client/BookKeeper.java 6af43ae 
bq.    bookkeeper-server/src/main/java/org/apache/bookkeeper/client/BookKeeperTools.java PRE-CREATION 
bq.    bookkeeper-server/src/main/java/org/apache/bookkeeper/client/DigestManager.java d4af3fa 
bq.    bookkeeper-server/src/main/java/org/apache/bookkeeper/client/DistributionSchedule.java 78aaa15 
bq.    bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerHandle.java 959df73 
bq.    bookkeeper-server/src/main/java/org/apache/bookkeeper/client/MacDigestManager.java 1131652 
bq.    bookkeeper-server/src/main/java/org/apache/bookkeeper/tools/BookKeeperTools.java 94e444c 
bq.    bookkeeper-server/src/test/java/org/apache/bookkeeper/client/BookKeeperTestClient.java dfc63d7 
bq.    bookkeeper-server/src/test/java/org/apache/bookkeeper/client/LedgerCacheTest.java PRE-CREATION 
bq.    bookkeeper-server/src/test/java/org/apache/bookkeeper/test/BaseTestCase.java 224c796 
bq.    bookkeeper-server/src/test/java/org/apache/bookkeeper/test/BookieFailureTest.java 82483f3 
bq.    bookkeeper-server/src/test/java/org/apache/bookkeeper/test/BookieReadWriteTest.java 56331ef 
bq.    bookkeeper-server/src/test/java/org/apache/bookkeeper/test/BookieRecoveryTest.java f933ba1 
bq.    bookkeeper-server/src/test/java/org/apache/bookkeeper/test/LedgerCacheTest.java 3a78507 
bq.    hedwig-server/src/main/java/org/apache/hedwig/server/benchmark/BookkeeperBenchmark.java a934985 
bq.    hedwig-server/src/main/java/org/apache/hedwig/server/netty/PubSubServer.java 726341d 
bq.    hedwig-server/src/test/java/org/apache/hedwig/server/persistence/BookKeeperTestBase.java b918d97 
bq.    pom.xml 2392db5 
bq.  
bq.  Diff: https://reviews.apache.org/r/2543/diff
bq.  
bq.  
bq.  Testing
bq.  -------
bq.  
bq.  
bq.  Thanks,
bq.  
bq.  Ivan
bq.  
bq.


                
> Bookkeeper API changes for initial Bookkeeper release
> -----------------------------------------------------
>
>                 Key: BOOKKEEPER-89
>                 URL: https://issues.apache.org/jira/browse/BOOKKEEPER-89
>             Project: Bookkeeper
>          Issue Type: Improvement
>            Reporter: Ivan Kelly
>            Assignee: Ivan Kelly
>             Fix For: 4.0.0
>
>         Attachments: BOOKKEEPER-89.diff, BOOKKEEPER-89.diff
>
>
> Changes are as follows.
> BookKeeper#createLedger, parameter is named passwd, "Key" used in LedgerHandle api
> BookKeeper#getBookieClient shouldn't be public
> BookKeeper#createComplete shouldn't be public
> BookKeeper#openComplete shouldn't be public
> BookKeeper#deleteComplete shouldn't be public
> BookKeeper#halt could be changed to close(), should throw a BKException
> LedgerHandle#getLedgerKey passwd is used in BookKeeper, should possibly be private
> LedgerHandle#getLedgerMetadata shouldn't be public
> LedgerHandle#getDigestManager shouldn't be public
> LedgerHandle#getDistributionSchedule shouldn't be public
> LedgerHandle#writeLedgerConfig shouldn't be public
> LedgerHandle#addEntry should return void, errors should go in an Exception
> LedgerHandle#readComplete should not be public
> LedgerHandle#addComplete should not be public
> LedgerHandle#readLastConfirmedCompelte should not be public
> LedgerHandle#closeComplete should not be public
> ASyncCallback#RecoverCallback shouldn't be public

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

        

[jira] [Commented] (BOOKKEEPER-89) Bookkeeper API changes for initial Bookkeeper release

Posted by "Flavio Junqueira (Commented) (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/BOOKKEEPER-89?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13135875#comment-13135875 ] 

Flavio Junqueira commented on BOOKKEEPER-89:
--------------------------------------------

BookieReadWriteTest is failing for me. I'm attaching the surefire report.
                
> Bookkeeper API changes for initial Bookkeeper release
> -----------------------------------------------------
>
>                 Key: BOOKKEEPER-89
>                 URL: https://issues.apache.org/jira/browse/BOOKKEEPER-89
>             Project: Bookkeeper
>          Issue Type: Improvement
>            Reporter: Ivan Kelly
>            Assignee: Ivan Kelly
>             Fix For: 4.0.0
>
>         Attachments: BOOKKEEPER-89.diff, BOOKKEEPER-89.diff, BOOKKEEPER-89.diff, BOOKKEEPER-89.diff
>
>
> Changes are as follows.
> BookKeeper#createLedger, parameter is named passwd, "Key" used in LedgerHandle api
> BookKeeper#getBookieClient shouldn't be public
> BookKeeper#createComplete shouldn't be public
> BookKeeper#openComplete shouldn't be public
> BookKeeper#deleteComplete shouldn't be public
> BookKeeper#halt could be changed to close(), should throw a BKException
> LedgerHandle#getLedgerKey passwd is used in BookKeeper, should possibly be private
> LedgerHandle#getLedgerMetadata shouldn't be public
> LedgerHandle#getDigestManager shouldn't be public
> LedgerHandle#getDistributionSchedule shouldn't be public
> LedgerHandle#writeLedgerConfig shouldn't be public
> LedgerHandle#addEntry should return void, errors should go in an Exception
> LedgerHandle#readComplete should not be public
> LedgerHandle#addComplete should not be public
> LedgerHandle#readLastConfirmedCompelte should not be public
> LedgerHandle#closeComplete should not be public
> ASyncCallback#RecoverCallback shouldn't be public

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

        

[jira] [Commented] (BOOKKEEPER-89) Bookkeeper API changes for initial Bookkeeper release

Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/BOOKKEEPER-89?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13134118#comment-13134118 ] 

jiraposter@reviews.apache.org commented on BOOKKEEPER-89:
---------------------------------------------------------


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/2543/
-----------------------------------------------------------

(Updated 2011-10-24 14:53:25.740238)


Review request for bookkeeper.


Summary
-------

Changes are as follows.

BookKeeper#createLedger, parameter is named passwd, "Key" used in LedgerHandle api
BookKeeper#getBookieClient shouldn't be public
BookKeeper#createComplete shouldn't be public
BookKeeper#openComplete shouldn't be public
BookKeeper#deleteComplete shouldn't be public
BookKeeper#halt could be changed to close(), should throw a BKException

LedgerHandle#getLedgerKey passwd is used in BookKeeper, should possibly be private
LedgerHandle#getLedgerMetadata shouldn't be public
LedgerHandle#getDigestManager shouldn't be public
LedgerHandle#getDistributionSchedule shouldn't be public
LedgerHandle#writeLedgerConfig shouldn't be public
LedgerHandle#addEntry should return void, errors should go in an Exception
LedgerHandle#readComplete should not be public
LedgerHandle#addComplete should not be public
LedgerHandle#readLastConfirmedCompelte should not be public
LedgerHandle#closeComplete should not be public

ASyncCallback#RecoverCallback shouldn't be public


This addresses bug BOOKKEEPER-89.
    https://issues.apache.org/jira/browse/BOOKKEEPER-89


Diffs (updated)
-----

  bookkeeper-server/src/main/java/org/apache/bookkeeper/client/AsyncCallback.java 6421460 
  bookkeeper-server/src/main/java/org/apache/bookkeeper/client/BookKeeper.java 6af43ae 
  bookkeeper-server/src/main/java/org/apache/bookkeeper/client/BookKeeperTools.java PRE-CREATION 
  bookkeeper-server/src/main/java/org/apache/bookkeeper/client/DigestManager.java d4af3fa 
  bookkeeper-server/src/main/java/org/apache/bookkeeper/client/DistributionSchedule.java 78aaa15 
  bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerHandle.java 959df73 
  bookkeeper-server/src/main/java/org/apache/bookkeeper/client/MacDigestManager.java 1131652 
  bookkeeper-server/src/main/java/org/apache/bookkeeper/tools/BookKeeperTools.java 94e444c 
  bookkeeper-server/src/test/java/org/apache/bookkeeper/client/BookKeeperTestClient.java dfc63d7 
  bookkeeper-server/src/test/java/org/apache/bookkeeper/client/LedgerCacheTest.java PRE-CREATION 
  bookkeeper-server/src/test/java/org/apache/bookkeeper/test/BaseTestCase.java 224c796 
  bookkeeper-server/src/test/java/org/apache/bookkeeper/test/BookieFailureTest.java 82483f3 
  bookkeeper-server/src/test/java/org/apache/bookkeeper/test/BookieReadWriteTest.java 56331ef 
  bookkeeper-server/src/test/java/org/apache/bookkeeper/test/BookieRecoveryTest.java f933ba1 
  bookkeeper-server/src/test/java/org/apache/bookkeeper/test/LedgerCacheTest.java 3a78507 
  hedwig-server/src/main/java/org/apache/hedwig/server/benchmark/BookkeeperBenchmark.java a934985 
  hedwig-server/src/main/java/org/apache/hedwig/server/netty/PubSubServer.java 726341d 
  hedwig-server/src/test/java/org/apache/hedwig/server/persistence/BookKeeperTestBase.java b918d97 
  pom.xml 2392db5 

Diff: https://reviews.apache.org/r/2543/diff


Testing
-------


Thanks,

Ivan


                
> Bookkeeper API changes for initial Bookkeeper release
> -----------------------------------------------------
>
>                 Key: BOOKKEEPER-89
>                 URL: https://issues.apache.org/jira/browse/BOOKKEEPER-89
>             Project: Bookkeeper
>          Issue Type: Improvement
>            Reporter: Ivan Kelly
>            Assignee: Ivan Kelly
>             Fix For: 4.0.0
>
>         Attachments: BOOKKEEPER-89.diff, BOOKKEEPER-89.diff
>
>
> Changes are as follows.
> BookKeeper#createLedger, parameter is named passwd, "Key" used in LedgerHandle api
> BookKeeper#getBookieClient shouldn't be public
> BookKeeper#createComplete shouldn't be public
> BookKeeper#openComplete shouldn't be public
> BookKeeper#deleteComplete shouldn't be public
> BookKeeper#halt could be changed to close(), should throw a BKException
> LedgerHandle#getLedgerKey passwd is used in BookKeeper, should possibly be private
> LedgerHandle#getLedgerMetadata shouldn't be public
> LedgerHandle#getDigestManager shouldn't be public
> LedgerHandle#getDistributionSchedule shouldn't be public
> LedgerHandle#writeLedgerConfig shouldn't be public
> LedgerHandle#addEntry should return void, errors should go in an Exception
> LedgerHandle#readComplete should not be public
> LedgerHandle#addComplete should not be public
> LedgerHandle#readLastConfirmedCompelte should not be public
> LedgerHandle#closeComplete should not be public
> ASyncCallback#RecoverCallback shouldn't be public

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

        

[jira] [Commented] (BOOKKEEPER-89) Bookkeeper API changes for initial Bookkeeper release

Posted by "Ivan Kelly (Commented) (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/BOOKKEEPER-89?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13136019#comment-13136019 ] 

Ivan Kelly commented on BOOKKEEPER-89:
--------------------------------------

In that case I would suggest passing your own ZK instance into the constructor. Do you think more is needed than this?
                
> Bookkeeper API changes for initial Bookkeeper release
> -----------------------------------------------------
>
>                 Key: BOOKKEEPER-89
>                 URL: https://issues.apache.org/jira/browse/BOOKKEEPER-89
>             Project: Bookkeeper
>          Issue Type: Improvement
>            Reporter: Ivan Kelly
>            Assignee: Ivan Kelly
>             Fix For: 4.0.0
>
>         Attachments: BOOKKEEPER-89.diff, BOOKKEEPER-89.diff, BOOKKEEPER-89.diff, BOOKKEEPER-89.diff, BOOKKEEPER-89.diff, TEST-org.apache.bookkeeper.test.BookieReadWriteTest.xml
>
>
> Changes are as follows.
> BookKeeper#createLedger, parameter is named passwd, "Key" used in LedgerHandle api
> BookKeeper#getBookieClient shouldn't be public
> BookKeeper#createComplete shouldn't be public
> BookKeeper#openComplete shouldn't be public
> BookKeeper#deleteComplete shouldn't be public
> BookKeeper#halt could be changed to close(), should throw a BKException
> LedgerHandle#getLedgerKey passwd is used in BookKeeper, should possibly be private
> LedgerHandle#getLedgerMetadata shouldn't be public
> LedgerHandle#getDigestManager shouldn't be public
> LedgerHandle#getDistributionSchedule shouldn't be public
> LedgerHandle#writeLedgerConfig shouldn't be public
> LedgerHandle#addEntry should return void, errors should go in an Exception
> LedgerHandle#readComplete should not be public
> LedgerHandle#addComplete should not be public
> LedgerHandle#readLastConfirmedCompelte should not be public
> LedgerHandle#closeComplete should not be public
> ASyncCallback#RecoverCallback shouldn't be public

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

        

[jira] [Updated] (BOOKKEEPER-89) Bookkeeper API changes for initial Bookkeeper release

Posted by "Ivan Kelly (Updated) (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/BOOKKEEPER-89?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Ivan Kelly updated BOOKKEEPER-89:
---------------------------------

    Attachment: BOOKKEEPER-89.diff

Fixed the hedwig server compile. I hadn't cleaned, before testing last time.
                
> Bookkeeper API changes for initial Bookkeeper release
> -----------------------------------------------------
>
>                 Key: BOOKKEEPER-89
>                 URL: https://issues.apache.org/jira/browse/BOOKKEEPER-89
>             Project: Bookkeeper
>          Issue Type: Improvement
>            Reporter: Ivan Kelly
>            Assignee: Ivan Kelly
>             Fix For: 4.0.0
>
>         Attachments: BOOKKEEPER-89.diff, BOOKKEEPER-89.diff
>
>
> Changes are as follows.
> BookKeeper#createLedger, parameter is named passwd, "Key" used in LedgerHandle api
> BookKeeper#getBookieClient shouldn't be public
> BookKeeper#createComplete shouldn't be public
> BookKeeper#openComplete shouldn't be public
> BookKeeper#deleteComplete shouldn't be public
> BookKeeper#halt could be changed to close(), should throw a BKException
> LedgerHandle#getLedgerKey passwd is used in BookKeeper, should possibly be private
> LedgerHandle#getLedgerMetadata shouldn't be public
> LedgerHandle#getDigestManager shouldn't be public
> LedgerHandle#getDistributionSchedule shouldn't be public
> LedgerHandle#writeLedgerConfig shouldn't be public
> LedgerHandle#addEntry should return void, errors should go in an Exception
> LedgerHandle#readComplete should not be public
> LedgerHandle#addComplete should not be public
> LedgerHandle#readLastConfirmedCompelte should not be public
> LedgerHandle#closeComplete should not be public
> ASyncCallback#RecoverCallback shouldn't be public

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

        

[jira] [Commented] (BOOKKEEPER-89) Bookkeeper API changes for initial Bookkeeper release

Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/BOOKKEEPER-89?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13135574#comment-13135574 ] 

jiraposter@reviews.apache.org commented on BOOKKEEPER-89:
---------------------------------------------------------


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/2543/#review2843
-----------------------------------------------------------


Thanks a lot for working on this patch, Ivan. It is mostly good for me, but I have a few comments if you don't mind.


bookkeeper-server/src/main/java/org/apache/bookkeeper/client/BookKeeper.java
<https://reviews.apache.org/r/2543/#comment6370>

    We differentiate between password and ledger key in BookKeeper. The password is the string that the application passes to bookkeeper when creating a ledger. The password is used to generate the ledger key (check the constructor of ledger handle) and the mac key when MAC is being used.



bookkeeper-server/src/main/java/org/apache/bookkeeper/client/BookKeeperTools.java
<https://reviews.apache.org/r/2543/#comment6376>

    I understand that these tools need visibility to some internal methods of the client package, but this class is not really part of the bookkeeper client. It is instead a tool for things like bookie recovery.
    
    I was wondering if we should at least have it in a separate folder to avoid confusion.



bookkeeper-server/src/main/java/org/apache/bookkeeper/client/BookKeeperTools.java
<https://reviews.apache.org/r/2543/#comment6371>

    Is this a tab?



bookkeeper-server/src/test/java/org/apache/bookkeeper/client/LedgerCacheTest.java
<https://reviews.apache.org/r/2543/#comment6373>

    Why are we moving this test to a different package?


- fpj


On 2011-10-24 14:53:25, Ivan Kelly wrote:
bq.  
bq.  -----------------------------------------------------------
bq.  This is an automatically generated e-mail. To reply, visit:
bq.  https://reviews.apache.org/r/2543/
bq.  -----------------------------------------------------------
bq.  
bq.  (Updated 2011-10-24 14:53:25)
bq.  
bq.  
bq.  Review request for bookkeeper.
bq.  
bq.  
bq.  Summary
bq.  -------
bq.  
bq.  Changes are as follows.
bq.  
bq.  BookKeeper#createLedger, parameter is named passwd, "Key" used in LedgerHandle api
bq.  BookKeeper#getBookieClient shouldn't be public
bq.  BookKeeper#createComplete shouldn't be public
bq.  BookKeeper#openComplete shouldn't be public
bq.  BookKeeper#deleteComplete shouldn't be public
bq.  BookKeeper#halt could be changed to close(), should throw a BKException
bq.  
bq.  LedgerHandle#getLedgerKey passwd is used in BookKeeper, should possibly be private
bq.  LedgerHandle#getLedgerMetadata shouldn't be public
bq.  LedgerHandle#getDigestManager shouldn't be public
bq.  LedgerHandle#getDistributionSchedule shouldn't be public
bq.  LedgerHandle#writeLedgerConfig shouldn't be public
bq.  LedgerHandle#addEntry should return void, errors should go in an Exception
bq.  LedgerHandle#readComplete should not be public
bq.  LedgerHandle#addComplete should not be public
bq.  LedgerHandle#readLastConfirmedCompelte should not be public
bq.  LedgerHandle#closeComplete should not be public
bq.  
bq.  ASyncCallback#RecoverCallback shouldn't be public
bq.  
bq.  
bq.  This addresses bug BOOKKEEPER-89.
bq.      https://issues.apache.org/jira/browse/BOOKKEEPER-89
bq.  
bq.  
bq.  Diffs
bq.  -----
bq.  
bq.    bookkeeper-server/src/main/java/org/apache/bookkeeper/client/AsyncCallback.java 6421460 
bq.    bookkeeper-server/src/main/java/org/apache/bookkeeper/client/BookKeeper.java 6af43ae 
bq.    bookkeeper-server/src/main/java/org/apache/bookkeeper/client/BookKeeperTools.java PRE-CREATION 
bq.    bookkeeper-server/src/main/java/org/apache/bookkeeper/client/DigestManager.java d4af3fa 
bq.    bookkeeper-server/src/main/java/org/apache/bookkeeper/client/DistributionSchedule.java 78aaa15 
bq.    bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerHandle.java 959df73 
bq.    bookkeeper-server/src/main/java/org/apache/bookkeeper/client/MacDigestManager.java 1131652 
bq.    bookkeeper-server/src/main/java/org/apache/bookkeeper/tools/BookKeeperTools.java 94e444c 
bq.    bookkeeper-server/src/test/java/org/apache/bookkeeper/client/BookKeeperTestClient.java dfc63d7 
bq.    bookkeeper-server/src/test/java/org/apache/bookkeeper/client/LedgerCacheTest.java PRE-CREATION 
bq.    bookkeeper-server/src/test/java/org/apache/bookkeeper/test/BaseTestCase.java 224c796 
bq.    bookkeeper-server/src/test/java/org/apache/bookkeeper/test/BookieFailureTest.java 82483f3 
bq.    bookkeeper-server/src/test/java/org/apache/bookkeeper/test/BookieReadWriteTest.java 56331ef 
bq.    bookkeeper-server/src/test/java/org/apache/bookkeeper/test/BookieRecoveryTest.java f933ba1 
bq.    bookkeeper-server/src/test/java/org/apache/bookkeeper/test/LedgerCacheTest.java 3a78507 
bq.    hedwig-server/src/main/java/org/apache/hedwig/server/benchmark/BookkeeperBenchmark.java a934985 
bq.    hedwig-server/src/main/java/org/apache/hedwig/server/netty/PubSubServer.java 726341d 
bq.    hedwig-server/src/test/java/org/apache/hedwig/server/persistence/BookKeeperTestBase.java b918d97 
bq.    pom.xml 2392db5 
bq.  
bq.  Diff: https://reviews.apache.org/r/2543/diff
bq.  
bq.  
bq.  Testing
bq.  -------
bq.  
bq.  
bq.  Thanks,
bq.  
bq.  Ivan
bq.  
bq.


                
> Bookkeeper API changes for initial Bookkeeper release
> -----------------------------------------------------
>
>                 Key: BOOKKEEPER-89
>                 URL: https://issues.apache.org/jira/browse/BOOKKEEPER-89
>             Project: Bookkeeper
>          Issue Type: Improvement
>            Reporter: Ivan Kelly
>            Assignee: Ivan Kelly
>             Fix For: 4.0.0
>
>         Attachments: BOOKKEEPER-89.diff, BOOKKEEPER-89.diff
>
>
> Changes are as follows.
> BookKeeper#createLedger, parameter is named passwd, "Key" used in LedgerHandle api
> BookKeeper#getBookieClient shouldn't be public
> BookKeeper#createComplete shouldn't be public
> BookKeeper#openComplete shouldn't be public
> BookKeeper#deleteComplete shouldn't be public
> BookKeeper#halt could be changed to close(), should throw a BKException
> LedgerHandle#getLedgerKey passwd is used in BookKeeper, should possibly be private
> LedgerHandle#getLedgerMetadata shouldn't be public
> LedgerHandle#getDigestManager shouldn't be public
> LedgerHandle#getDistributionSchedule shouldn't be public
> LedgerHandle#writeLedgerConfig shouldn't be public
> LedgerHandle#addEntry should return void, errors should go in an Exception
> LedgerHandle#readComplete should not be public
> LedgerHandle#addComplete should not be public
> LedgerHandle#readLastConfirmedCompelte should not be public
> LedgerHandle#closeComplete should not be public
> ASyncCallback#RecoverCallback shouldn't be public

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

        

[jira] [Updated] (BOOKKEEPER-89) Bookkeeper API changes for initial Bookkeeper release

Posted by "Ivan Kelly (Updated) (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/BOOKKEEPER-89?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Ivan Kelly updated BOOKKEEPER-89:
---------------------------------

    Attachment: BOOKKEEPER-89.diff

patch generated with --no-prefix
                
> Bookkeeper API changes for initial Bookkeeper release
> -----------------------------------------------------
>
>                 Key: BOOKKEEPER-89
>                 URL: https://issues.apache.org/jira/browse/BOOKKEEPER-89
>             Project: Bookkeeper
>          Issue Type: Improvement
>            Reporter: Ivan Kelly
>            Assignee: Ivan Kelly
>             Fix For: 4.0.0
>
>         Attachments: BOOKKEEPER-89.diff, BOOKKEEPER-89.diff, BOOKKEEPER-89.diff, BOOKKEEPER-89.diff
>
>
> Changes are as follows.
> BookKeeper#createLedger, parameter is named passwd, "Key" used in LedgerHandle api
> BookKeeper#getBookieClient shouldn't be public
> BookKeeper#createComplete shouldn't be public
> BookKeeper#openComplete shouldn't be public
> BookKeeper#deleteComplete shouldn't be public
> BookKeeper#halt could be changed to close(), should throw a BKException
> LedgerHandle#getLedgerKey passwd is used in BookKeeper, should possibly be private
> LedgerHandle#getLedgerMetadata shouldn't be public
> LedgerHandle#getDigestManager shouldn't be public
> LedgerHandle#getDistributionSchedule shouldn't be public
> LedgerHandle#writeLedgerConfig shouldn't be public
> LedgerHandle#addEntry should return void, errors should go in an Exception
> LedgerHandle#readComplete should not be public
> LedgerHandle#addComplete should not be public
> LedgerHandle#readLastConfirmedCompelte should not be public
> LedgerHandle#closeComplete should not be public
> ASyncCallback#RecoverCallback shouldn't be public

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

        

[jira] [Commented] (BOOKKEEPER-89) Bookkeeper API changes for initial Bookkeeper release

Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/BOOKKEEPER-89?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13135835#comment-13135835 ] 

jiraposter@reviews.apache.org commented on BOOKKEEPER-89:
---------------------------------------------------------


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/2543/
-----------------------------------------------------------

(Updated 2011-10-26 09:49:18.492814)


Review request for bookkeeper.


Changes
-------

Addressed comments. Removed passwd->key change. Added BookKeeperAdmin and moved BookKeeperTools back to tools package.


Summary
-------

Changes are as follows.

BookKeeper#createLedger, parameter is named passwd, "Key" used in LedgerHandle api
BookKeeper#getBookieClient shouldn't be public
BookKeeper#createComplete shouldn't be public
BookKeeper#openComplete shouldn't be public
BookKeeper#deleteComplete shouldn't be public
BookKeeper#halt could be changed to close(), should throw a BKException

LedgerHandle#getLedgerKey passwd is used in BookKeeper, should possibly be private
LedgerHandle#getLedgerMetadata shouldn't be public
LedgerHandle#getDigestManager shouldn't be public
LedgerHandle#getDistributionSchedule shouldn't be public
LedgerHandle#writeLedgerConfig shouldn't be public
LedgerHandle#addEntry should return void, errors should go in an Exception
LedgerHandle#readComplete should not be public
LedgerHandle#addComplete should not be public
LedgerHandle#readLastConfirmedCompelte should not be public
LedgerHandle#closeComplete should not be public

ASyncCallback#RecoverCallback shouldn't be public


This addresses bug BOOKKEEPER-89.
    https://issues.apache.org/jira/browse/BOOKKEEPER-89


Diffs (updated)
-----

  bookkeeper-server/src/main/java/org/apache/bookkeeper/client/AsyncCallback.java 6421460 
  bookkeeper-server/src/main/java/org/apache/bookkeeper/client/BookKeeper.java 6af43ae 
  bookkeeper-server/src/main/java/org/apache/bookkeeper/client/BookKeeperAdmin.java PRE-CREATION 
  bookkeeper-server/src/main/java/org/apache/bookkeeper/client/DigestManager.java d4af3fa 
  bookkeeper-server/src/main/java/org/apache/bookkeeper/client/DistributionSchedule.java 78aaa15 
  bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerHandle.java 959df73 
  bookkeeper-server/src/main/java/org/apache/bookkeeper/client/MacDigestManager.java 1131652 
  bookkeeper-server/src/main/java/org/apache/bookkeeper/tools/BookKeeperTools.java 94e444c 
  bookkeeper-server/src/test/java/org/apache/bookkeeper/client/BookKeeperTestClient.java dfc63d7 
  bookkeeper-server/src/test/java/org/apache/bookkeeper/client/LedgerCacheTest.java PRE-CREATION 
  bookkeeper-server/src/test/java/org/apache/bookkeeper/test/BaseTestCase.java 224c796 
  bookkeeper-server/src/test/java/org/apache/bookkeeper/test/BookieFailureTest.java 82483f3 
  bookkeeper-server/src/test/java/org/apache/bookkeeper/test/BookieReadWriteTest.java 56331ef 
  bookkeeper-server/src/test/java/org/apache/bookkeeper/test/BookieRecoveryTest.java f933ba1 
  bookkeeper-server/src/test/java/org/apache/bookkeeper/test/LedgerCacheTest.java 3a78507 
  hedwig-server/src/main/java/org/apache/hedwig/server/benchmark/BookkeeperBenchmark.java a934985 
  hedwig-server/src/main/java/org/apache/hedwig/server/netty/PubSubServer.java 726341d 
  hedwig-server/src/test/java/org/apache/hedwig/server/persistence/BookKeeperTestBase.java b918d97 
  pom.xml 2392db5 

Diff: https://reviews.apache.org/r/2543/diff


Testing
-------


Thanks,

Ivan


                
> Bookkeeper API changes for initial Bookkeeper release
> -----------------------------------------------------
>
>                 Key: BOOKKEEPER-89
>                 URL: https://issues.apache.org/jira/browse/BOOKKEEPER-89
>             Project: Bookkeeper
>          Issue Type: Improvement
>            Reporter: Ivan Kelly
>            Assignee: Ivan Kelly
>             Fix For: 4.0.0
>
>         Attachments: BOOKKEEPER-89.diff, BOOKKEEPER-89.diff, BOOKKEEPER-89.diff
>
>
> Changes are as follows.
> BookKeeper#createLedger, parameter is named passwd, "Key" used in LedgerHandle api
> BookKeeper#getBookieClient shouldn't be public
> BookKeeper#createComplete shouldn't be public
> BookKeeper#openComplete shouldn't be public
> BookKeeper#deleteComplete shouldn't be public
> BookKeeper#halt could be changed to close(), should throw a BKException
> LedgerHandle#getLedgerKey passwd is used in BookKeeper, should possibly be private
> LedgerHandle#getLedgerMetadata shouldn't be public
> LedgerHandle#getDigestManager shouldn't be public
> LedgerHandle#getDistributionSchedule shouldn't be public
> LedgerHandle#writeLedgerConfig shouldn't be public
> LedgerHandle#addEntry should return void, errors should go in an Exception
> LedgerHandle#readComplete should not be public
> LedgerHandle#addComplete should not be public
> LedgerHandle#readLastConfirmedCompelte should not be public
> LedgerHandle#closeComplete should not be public
> ASyncCallback#RecoverCallback shouldn't be public

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

        

[jira] [Commented] (BOOKKEEPER-89) Bookkeeper API changes for initial Bookkeeper release

Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/BOOKKEEPER-89?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13134043#comment-13134043 ] 

jiraposter@reviews.apache.org commented on BOOKKEEPER-89:
---------------------------------------------------------


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/2543/
-----------------------------------------------------------

Review request for bookkeeper.


Summary
-------

Changes are as follows.

BookKeeper#createLedger, parameter is named passwd, "Key" used in LedgerHandle api
BookKeeper#getBookieClient shouldn't be public
BookKeeper#createComplete shouldn't be public
BookKeeper#openComplete shouldn't be public
BookKeeper#deleteComplete shouldn't be public
BookKeeper#halt could be changed to close(), should throw a BKException

LedgerHandle#getLedgerKey passwd is used in BookKeeper, should possibly be private
LedgerHandle#getLedgerMetadata shouldn't be public
LedgerHandle#getDigestManager shouldn't be public
LedgerHandle#getDistributionSchedule shouldn't be public
LedgerHandle#writeLedgerConfig shouldn't be public
LedgerHandle#addEntry should return void, errors should go in an Exception
LedgerHandle#readComplete should not be public
LedgerHandle#addComplete should not be public
LedgerHandle#readLastConfirmedCompelte should not be public
LedgerHandle#closeComplete should not be public

ASyncCallback#RecoverCallback shouldn't be public


This addresses bug BOOKKEEPER-89.
    https://issues.apache.org/jira/browse/BOOKKEEPER-89


Diffs
-----

  bookkeeper-server/src/main/java/org/apache/bookkeeper/client/AsyncCallback.java 6421460 
  bookkeeper-server/src/main/java/org/apache/bookkeeper/client/BookKeeper.java 6af43ae 
  bookkeeper-server/src/main/java/org/apache/bookkeeper/client/BookKeeperTools.java PRE-CREATION 
  bookkeeper-server/src/main/java/org/apache/bookkeeper/client/DigestManager.java d4af3fa 
  bookkeeper-server/src/main/java/org/apache/bookkeeper/client/DistributionSchedule.java 78aaa15 
  bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerHandle.java 959df73 
  bookkeeper-server/src/main/java/org/apache/bookkeeper/client/MacDigestManager.java 1131652 
  bookkeeper-server/src/main/java/org/apache/bookkeeper/tools/BookKeeperTools.java 94e444c 
  bookkeeper-server/src/test/java/org/apache/bookkeeper/client/BookKeeperTestClient.java dfc63d7 
  bookkeeper-server/src/test/java/org/apache/bookkeeper/test/BaseTestCase.java 224c796 
  bookkeeper-server/src/test/java/org/apache/bookkeeper/test/BookieFailureTest.java 82483f3 
  bookkeeper-server/src/test/java/org/apache/bookkeeper/test/BookieReadWriteTest.java 56331ef 
  bookkeeper-server/src/test/java/org/apache/bookkeeper/test/BookieRecoveryTest.java f933ba1 
  pom.xml 2392db5 

Diff: https://reviews.apache.org/r/2543/diff


Testing
-------


Thanks,

Ivan


                
> Bookkeeper API changes for initial Bookkeeper release
> -----------------------------------------------------
>
>                 Key: BOOKKEEPER-89
>                 URL: https://issues.apache.org/jira/browse/BOOKKEEPER-89
>             Project: Bookkeeper
>          Issue Type: Improvement
>            Reporter: Ivan Kelly
>            Assignee: Ivan Kelly
>             Fix For: 4.0.0
>
>         Attachments: BOOKKEEPER-89.diff
>
>
> Changes are as follows.
> BookKeeper#createLedger, parameter is named passwd, "Key" used in LedgerHandle api
> BookKeeper#getBookieClient shouldn't be public
> BookKeeper#createComplete shouldn't be public
> BookKeeper#openComplete shouldn't be public
> BookKeeper#deleteComplete shouldn't be public
> BookKeeper#halt could be changed to close(), should throw a BKException
> LedgerHandle#getLedgerKey passwd is used in BookKeeper, should possibly be private
> LedgerHandle#getLedgerMetadata shouldn't be public
> LedgerHandle#getDigestManager shouldn't be public
> LedgerHandle#getDistributionSchedule shouldn't be public
> LedgerHandle#writeLedgerConfig shouldn't be public
> LedgerHandle#addEntry should return void, errors should go in an Exception
> LedgerHandle#readComplete should not be public
> LedgerHandle#addComplete should not be public
> LedgerHandle#readLastConfirmedCompelte should not be public
> LedgerHandle#closeComplete should not be public
> ASyncCallback#RecoverCallback shouldn't be public

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