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 2012/02/17 11:01:59 UTC
[jira] [Created] (BOOKKEEPER-175) Bookie code is very coupled
Bookie code is very coupled
---------------------------
Key: BOOKKEEPER-175
URL: https://issues.apache.org/jira/browse/BOOKKEEPER-175
Project: Bookkeeper
Issue Type: Improvement
Reporter: Ivan Kelly
Assignee: Ivan Kelly
Fix For: 4.1.0
Bookie owns EntryLogger, LedgerCache, LedgerDescriptors which all depend on each other in strange ways. Sometimes we access the ledgerCache directly, sometimes through LedgerDescriptors. etc, etc. It's messy and there's no hierarchy.
I propose that we refactor Bookie to only contain the EntryLogger and journalling code (this should be factored at some stage also). The EntryLogger can then own the ledgerCache and the LedgerDescriptors, and then we would how have to have the entanglement as observed on BOOKKEEPER-160.
--
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-175) Bookie code is very coupled
Posted by "Sijie Guo (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/BOOKKEEPER-175?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13233347#comment-13233347 ]
Sijie Guo commented on BOOKKEEPER-175:
--------------------------------------
thanks for Ivan's replies. the patch is good to me. +1.
> Bookie code is very coupled
> ---------------------------
>
> Key: BOOKKEEPER-175
> URL: https://issues.apache.org/jira/browse/BOOKKEEPER-175
> Project: Bookkeeper
> Issue Type: Improvement
> Reporter: Ivan Kelly
> Assignee: Ivan Kelly
> Fix For: 4.1.0
>
> Attachments: BOOKKEEPER-175.diff
>
>
> Bookie owns EntryLogger, LedgerCache, LedgerDescriptors which all depend on each other in strange ways. Sometimes we access the ledgerCache directly, sometimes through LedgerDescriptors. etc, etc. It's messy and there's no hierarchy.
> I propose that we refactor Bookie to only contain the EntryLogger and journalling code (this should be factored at some stage also). The EntryLogger can then own the ledgerCache and the LedgerDescriptors, and then we would how have to have the entanglement as observed on BOOKKEEPER-160.
--
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-175) Bookie code is very coupled
Posted by "Hudson (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/BOOKKEEPER-175?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13233873#comment-13233873 ]
Hudson commented on BOOKKEEPER-175:
-----------------------------------
Integrated in bookkeeper-trunk #419 (See [https://builds.apache.org/job/bookkeeper-trunk/419/])
BOOKKEEPER-175: Bookie code is very coupled (ivank) (Revision 1302870)
Result = ABORTED
ivank :
Files :
* /zookeeper/bookkeeper/trunk/CHANGES.txt
* /zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/Bookie.java
* /zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/HandleFactory.java
* /zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/HandleFactoryImpl.java
* /zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/LedgerDescriptor.java
* /zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/LedgerDescriptorImpl.java
* /zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/LedgerDescriptorReadOnlyImpl.java
> Bookie code is very coupled
> ---------------------------
>
> Key: BOOKKEEPER-175
> URL: https://issues.apache.org/jira/browse/BOOKKEEPER-175
> Project: Bookkeeper
> Issue Type: Improvement
> Reporter: Ivan Kelly
> Assignee: Ivan Kelly
> Fix For: 4.1.0
>
> Attachments: BOOKKEEPER-175.diff
>
>
> Bookie owns EntryLogger, LedgerCache, LedgerDescriptors which all depend on each other in strange ways. Sometimes we access the ledgerCache directly, sometimes through LedgerDescriptors. etc, etc. It's messy and there's no hierarchy.
> I propose that we refactor Bookie to only contain the EntryLogger and journalling code (this should be factored at some stage also). The EntryLogger can then own the ledgerCache and the LedgerDescriptors, and then we would how have to have the entanglement as observed on BOOKKEEPER-160.
--
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-175) Bookie code is very coupled
Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/BOOKKEEPER-175?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13233267#comment-13233267 ]
jiraposter@reviews.apache.org commented on BOOKKEEPER-175:
----------------------------------------------------------
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4405/#review6113
-----------------------------------------------------------
bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/Bookie.java
<https://reviews.apache.org/r/4405/#comment13142>
do you consider moving masterKeyCache into ledgerCache? it seems that would be more clear.
bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/Bookie.java
<https://reviews.apache.org/r/4405/#comment13143>
it seems that there is a reference counting in ledger handle before. but after refactoring, you remove it. so is it OK?
- Sijie
On 2012-03-19 11:43:51, Ivan Kelly wrote:
bq.
bq. -----------------------------------------------------------
bq. This is an automatically generated e-mail. To reply, visit:
bq. https://reviews.apache.org/r/4405/
bq. -----------------------------------------------------------
bq.
bq. (Updated 2012-03-19 11:43:51)
bq.
bq.
bq. Review request for bookkeeper.
bq.
bq.
bq. Summary
bq. -------
bq.
bq. Patch creates LedgerCache interface, and LedgerCacheImpl, which implements the interface. LedgerCacheImpl's contains what LedgerCache did before the change. LedgerDescriptor uses LedgerCacheImpl directly now, which is ugly, but its only temporary as BOOKKEEPER-175 will fix LedgerDescriptors.
bq.
bq.
bq. This addresses bug BOOKKEEPER-175.
bq. https://issues.apache.org/jira/browse/BOOKKEEPER-175
bq.
bq.
bq. Diffs
bq. -----
bq.
bq. bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/Bookie.java 6e47c08
bq. bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/HandleFactory.java PRE-CREATION
bq. bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/HandleFactoryImpl.java PRE-CREATION
bq. bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/LedgerDescriptor.java 87a1e66
bq. bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/LedgerDescriptorImpl.java PRE-CREATION
bq. bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/LedgerDescriptorReadOnlyImpl.java PRE-CREATION
bq.
bq. Diff: https://reviews.apache.org/r/4405/diff
bq.
bq.
bq. Testing
bq. -------
bq.
bq.
bq. Thanks,
bq.
bq. Ivan
bq.
bq.
> Bookie code is very coupled
> ---------------------------
>
> Key: BOOKKEEPER-175
> URL: https://issues.apache.org/jira/browse/BOOKKEEPER-175
> Project: Bookkeeper
> Issue Type: Improvement
> Reporter: Ivan Kelly
> Assignee: Ivan Kelly
> Fix For: 4.1.0
>
> Attachments: BOOKKEEPER-175.diff
>
>
> Bookie owns EntryLogger, LedgerCache, LedgerDescriptors which all depend on each other in strange ways. Sometimes we access the ledgerCache directly, sometimes through LedgerDescriptors. etc, etc. It's messy and there's no hierarchy.
> I propose that we refactor Bookie to only contain the EntryLogger and journalling code (this should be factored at some stage also). The EntryLogger can then own the ledgerCache and the LedgerDescriptors, and then we would how have to have the entanglement as observed on BOOKKEEPER-160.
--
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-175) Bookie code is very coupled
Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/BOOKKEEPER-175?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13232571#comment-13232571 ]
jiraposter@reviews.apache.org commented on BOOKKEEPER-175:
----------------------------------------------------------
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4405/
-----------------------------------------------------------
Review request for bookkeeper.
Summary
-------
Patch creates LedgerCache interface, and LedgerCacheImpl, which implements the interface. LedgerCacheImpl's contains what LedgerCache did before the change. LedgerDescriptor uses LedgerCacheImpl directly now, which is ugly, but its only temporary as BOOKKEEPER-175 will fix LedgerDescriptors.
This addresses bug BOOKKEEPER-175.
https://issues.apache.org/jira/browse/BOOKKEEPER-175
Diffs
-----
bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/Bookie.java 6e47c08
bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/HandleFactory.java PRE-CREATION
bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/HandleFactoryImpl.java PRE-CREATION
bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/LedgerDescriptor.java 87a1e66
bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/LedgerDescriptorImpl.java PRE-CREATION
bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/LedgerDescriptorReadOnlyImpl.java PRE-CREATION
Diff: https://reviews.apache.org/r/4405/diff
Testing
-------
Thanks,
Ivan
> Bookie code is very coupled
> ---------------------------
>
> Key: BOOKKEEPER-175
> URL: https://issues.apache.org/jira/browse/BOOKKEEPER-175
> Project: Bookkeeper
> Issue Type: Improvement
> Reporter: Ivan Kelly
> Assignee: Ivan Kelly
> Fix For: 4.1.0
>
> Attachments: BOOKKEEPER-175.diff
>
>
> Bookie owns EntryLogger, LedgerCache, LedgerDescriptors which all depend on each other in strange ways. Sometimes we access the ledgerCache directly, sometimes through LedgerDescriptors. etc, etc. It's messy and there's no hierarchy.
> I propose that we refactor Bookie to only contain the EntryLogger and journalling code (this should be factored at some stage also). The EntryLogger can then own the ledgerCache and the LedgerDescriptors, and then we would how have to have the entanglement as observed on BOOKKEEPER-160.
--
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-175) Bookie code is very coupled
Posted by "Ivan Kelly (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/BOOKKEEPER-175?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13233314#comment-13233314 ]
Ivan Kelly commented on BOOKKEEPER-175:
---------------------------------------
{quote}
do you consider moving masterKeyCache into ledgerCache? it seems that would be more clear.
{quote}
The masterKeyCache belongs with the journal, because the masterKeyCache is an optimisation for the journal. We only need it because we don't want to store the password with every journal entry. I have been thinking about moving the journal code out of Bookie. If/When I do that, ill move masterKeyCache with it.
{quote}
it seems that there is a reference counting in ledger handle before. but after refactoring, you remove it. so is it OK?
{quote}
It was never used. Removing it makes the code quite a bit cleaner.
> Bookie code is very coupled
> ---------------------------
>
> Key: BOOKKEEPER-175
> URL: https://issues.apache.org/jira/browse/BOOKKEEPER-175
> Project: Bookkeeper
> Issue Type: Improvement
> Reporter: Ivan Kelly
> Assignee: Ivan Kelly
> Fix For: 4.1.0
>
> Attachments: BOOKKEEPER-175.diff
>
>
> Bookie owns EntryLogger, LedgerCache, LedgerDescriptors which all depend on each other in strange ways. Sometimes we access the ledgerCache directly, sometimes through LedgerDescriptors. etc, etc. It's messy and there's no hierarchy.
> I propose that we refactor Bookie to only contain the EntryLogger and journalling code (this should be factored at some stage also). The EntryLogger can then own the ledgerCache and the LedgerDescriptors, and then we would how have to have the entanglement as observed on BOOKKEEPER-160.
--
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-175) Bookie code is very coupled
Posted by "Flavio Junqueira (Updated) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/BOOKKEEPER-175?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Flavio Junqueira updated BOOKKEEPER-175:
----------------------------------------
Fix Version/s: (was: 4.1.0)
4.2.0
Moving it to 4.2.0.
> Bookie code is very coupled
> ---------------------------
>
> Key: BOOKKEEPER-175
> URL: https://issues.apache.org/jira/browse/BOOKKEEPER-175
> Project: Bookkeeper
> Issue Type: Improvement
> Reporter: Ivan Kelly
> Assignee: Ivan Kelly
> Fix For: 4.2.0
>
>
> Bookie owns EntryLogger, LedgerCache, LedgerDescriptors which all depend on each other in strange ways. Sometimes we access the ledgerCache directly, sometimes through LedgerDescriptors. etc, etc. It's messy and there's no hierarchy.
> I propose that we refactor Bookie to only contain the EntryLogger and journalling code (this should be factored at some stage also). The EntryLogger can then own the ledgerCache and the LedgerDescriptors, and then we would how have to have the entanglement as observed on BOOKKEEPER-160.
--
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-175) Bookie code is very coupled
Posted by "Ivan Kelly (Updated) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/BOOKKEEPER-175?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Ivan Kelly updated BOOKKEEPER-175:
----------------------------------
Attachment: BOOKKEEPER-175.diff
Patch moves handle creation out of bookie. There are now two types of handle, LedgerDescriptorImpl and LedgerDescriptorReadOnlyImpl. Any attempt to use the read only version for writing will throw an exception. This should make implementing BOOKKEEPER-135 (check passwd on fencing) trivial.
> Bookie code is very coupled
> ---------------------------
>
> Key: BOOKKEEPER-175
> URL: https://issues.apache.org/jira/browse/BOOKKEEPER-175
> Project: Bookkeeper
> Issue Type: Improvement
> Reporter: Ivan Kelly
> Assignee: Ivan Kelly
> Fix For: 4.1.0
>
> Attachments: BOOKKEEPER-175.diff
>
>
> Bookie owns EntryLogger, LedgerCache, LedgerDescriptors which all depend on each other in strange ways. Sometimes we access the ledgerCache directly, sometimes through LedgerDescriptors. etc, etc. It's messy and there's no hierarchy.
> I propose that we refactor Bookie to only contain the EntryLogger and journalling code (this should be factored at some stage also). The EntryLogger can then own the ledgerCache and the LedgerDescriptors, and then we would how have to have the entanglement as observed on BOOKKEEPER-160.
--
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-175) Bookie code is very coupled
Posted by "Ivan Kelly (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/BOOKKEEPER-175?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13233316#comment-13233316 ]
Ivan Kelly commented on BOOKKEEPER-175:
---------------------------------------
bq. It was never used.
By which i mean, it was incremented and decremented but the resulting value was never used.
> Bookie code is very coupled
> ---------------------------
>
> Key: BOOKKEEPER-175
> URL: https://issues.apache.org/jira/browse/BOOKKEEPER-175
> Project: Bookkeeper
> Issue Type: Improvement
> Reporter: Ivan Kelly
> Assignee: Ivan Kelly
> Fix For: 4.1.0
>
> Attachments: BOOKKEEPER-175.diff
>
>
> Bookie owns EntryLogger, LedgerCache, LedgerDescriptors which all depend on each other in strange ways. Sometimes we access the ledgerCache directly, sometimes through LedgerDescriptors. etc, etc. It's messy and there's no hierarchy.
> I propose that we refactor Bookie to only contain the EntryLogger and journalling code (this should be factored at some stage also). The EntryLogger can then own the ledgerCache and the LedgerDescriptors, and then we would how have to have the entanglement as observed on BOOKKEEPER-160.
--
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