hadoop-zookeeper-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Flavio Paiva Junqueira (JIRA)" <j...@apache.org>
Subject [jira] Commented: (ZOOKEEPER-288) Cleanup and fixes to BookKeeper
Date Thu, 26 Mar 2009 10:54:34 GMT

    [ https://issues.apache.org/jira/browse/ZOOKEEPER-288?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12689443#action_12689443
] 

Flavio Paiva Junqueira commented on ZOOKEEPER-288:
--------------------------------------------------

Thanks, Ben.

> > 1) you have genSecurePadding() in two different classes. for maintainability, you
should only have it in one place.

Those methods are not necessary anymore, so I have just removed them in the new patch I'm
uploading.

> > 2) in genLedgerKey and genMacKey, why do you allocate a byte array, do the copy
and then do the 
> > digest.update(), rather than calling digest.update() twice? i think it makes is
simpler and more readable.

When profiling BK, I noticed that reducing the number of calls to update increases performance
by a notch, so I've been following this strategy of only calling update once. I don't have
a strong argument for why that happens, so I can go either way.

> > 3) MasterKeys should start with a lower case letter. you never remove from MasterKeys,
so you should probably 
> > make that a WeakHashMap, so that it can get GCed.

Done. Do you think that "ledgers" in Bookie.java should also be a WeakHashMap? 

> > 4) are the masterkeys stored with the ledger fragment? i couldn't find it.

Currently it is not stored with the fragment. Do you think it should?




> Cleanup and fixes to BookKeeper
> -------------------------------
>
>                 Key: ZOOKEEPER-288
>                 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-288
>             Project: Zookeeper
>          Issue Type: Improvement
>          Components: contrib-bookkeeper
>    Affects Versions: 3.0.0
>            Reporter: Flavio Paiva Junqueira
>            Assignee: Flavio Paiva Junqueira
>             Fix For: 3.2.0
>
>         Attachments: ZOOKEEPER-BOOKKEEPER-288.patch, ZOOKEEPER-BOOKKEEPER-288.patch,
ZOOKEEPER-BOOKKEEPER-288.patch, ZOOKEEPER-BOOKKEEPER-288.patch
>
>
> We observed one race condition when multiple threads try to write concurrently. This
patch should fix it.  I will also remove some commented code.

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


Mime
View raw message