[jira] Issue Comment Edited: (HBASE-532) Odd interaction between HRegion.get, HRegion.deleteAll and compactions

["Bryan Duxbury (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/HBASE-532?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12589712#action_12589712 ] 

bryanduxbury edited comment on HBASE-532 at 4/16/08 12:53 PM:
---------------------------------------------------------------

Memcache.java
 * What's with the weird "/* ... \n*/" comments over the variable declarations at the top? if you're not going to use 3 lines use 1.
 * createSortedMap should be createSyncSortedMap? 
 * I think this comment should be something like "Must be followed by a call to clearSnapshot". Leave off the bit about if it's not empty, etc. Let clearSnapshot figure out what to do if it's empty.
{code}
 +   * Must be answered by a call to {@link #clearSnapshot(SortedMap)} if
 +   * returned snapshot is not empty.
{code}
 * Rename "ss" parameter in clearSnapshot to something like oldSnapshot? Would be a little clearer to read at a glance.
 * Should get() create a list and pass it into both calls of internalGet? Would save us an object creation.
 * The private version of getNextRow should probably be called internalGetNextRow to follow the pattern of the rest of the methods in Memcache.
 * Private version of getNextRow should have a comment on the inner loop that indicates it's iterating because it needs to skip other cells of the same row. 
 * In MemcacheScanner#next, what's the purpose of the "rr" variable? Should have a name that actually describes its purpose.
 

      was (Author: bryanduxbury):
    Memcache.java
 * What's with the weird "/* ... \n*/" comments over the variable declarations at the top? if you're not going to use 3 lines use 1.
 * createSortedMap should be createSyncSortedMap? 
 * I think this comment should be something like "Must be followed by a call to clearSnapshot". Leave off the bit about if it's not empty, etc. Let clearSnapshot figure out what to do if it's empty.
{{{code}}}
 +   * Must be answered by a call to {@link #clearSnapshot(SortedMap)} if
 +   * returned snapshot is not empty.
{{{code}}}
 * Rename "ss" parameter in clearSnapshot to something like oldSnapshot? Would be a little clearer to read at a glance.
 * Should get() create a list and pass it into both calls of internalGet? Would save us an object creation.
 * The private version of getNextRow should probably be called internalGetNextRow to follow the pattern of the rest of the methods in Memcache.
 * Private version of getNextRow should have a comment on the inner loop that indicates it's iterating because it needs to skip other cells of the same row. 
 * In MemcacheScanner#next, what's the purpose of the "rr" variable? Should have a name that actually describes its purpose.
 
  
> Odd interaction between HRegion.get, HRegion.deleteAll and compactions
> ----------------------------------------------------------------------
>
>                 Key: HBASE-532
>                 URL: https://issues.apache.org/jira/browse/HBASE-532
>             Project: Hadoop HBase
>          Issue Type: Bug
>    Affects Versions: 0.2.0, 0.1.1
>            Reporter: Jim Kellerman
>            Assignee: stack
>            Priority: Blocker
>             Fix For: 0.2.0, 0.1.2
>
>         Attachments: 532.patch
>
>
> If you apply the patch for HBASE-483 to the 0.1 branch and comment out lines 309 and 315 of MetaUtils.java (which force compactions of the root and meta regions respectively), TestMergeTool fails. Why forcing compactions makes the test succeed is a mystery to me.

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