hbase-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "HBase Review Board (JIRA)" <j...@apache.org>
Subject [jira] Commented: (HBASE-1364) [performance] Distributed splitting of regionserver commit logs
Date Fri, 23 Jul 2010 19:17:03 GMT

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

HBase Review Board commented on HBASE-1364:
-------------------------------------------

Message from: "Jean-Daniel Cryans" <jdcryans@apache.org>

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://review.hbase.org/r/370/#review464
-----------------------------------------------------------


First pass on this patch. Lots of cleanup that needs to be done, and it's a bit hard to follow
the flow of events without any clear documentation that gives an overview of distributed splitting.
Nothing big, just some use cases that could be put in the class javadoc of LogSplitter?


src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java
<http://review.hbase.org/r/370/#comment1903>

    I'm sure you have a good reason of putting that there, but at least one issue I'm seeing
is that this code is also in init() (which will be run just after that) and it's almost the
same thing.
    
    Also, fs.automatic.close is handled by the ShutdownHook class, you shouldn't be setting
it.



src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java
<http://review.hbase.org/r/370/#comment1904>

    Fix those long lines.



src/main/java/org/apache/hadoop/hbase/regionserver/LogSplitter.java
<http://review.hbase.org/r/370/#comment1905>

    rogue "q"



src/main/java/org/apache/hadoop/hbase/regionserver/LogSplitter.java
<http://review.hbase.org/r/370/#comment1914>

    Why are those static?



src/main/java/org/apache/hadoop/hbase/regionserver/LogSplitter.java
<http://review.hbase.org/r/370/#comment1906>

    remove that white space and all the others in that class at the same place



src/main/java/org/apache/hadoop/hbase/regionserver/LogSplitter.java
<http://review.hbase.org/r/370/#comment1922>

    both process and run call this method, can there be a race?



src/main/java/org/apache/hadoop/hbase/regionserver/LogSplitter.java
<http://review.hbase.org/r/370/#comment1923>

    don't need to declare this here



src/main/java/org/apache/hadoop/hbase/regionserver/LogSplitter.java
<http://review.hbase.org/r/370/#comment1907>

    What does that mean?



src/main/java/org/apache/hadoop/hbase/regionserver/LogSplitter.java
<http://review.hbase.org/r/370/#comment1918>

    why two lines?



src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java
<http://review.hbase.org/r/370/#comment1908>

    rogue "b"



src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java
<http://review.hbase.org/r/370/#comment1909>

    ?



src/main/java/org/apache/hadoop/hbase/zookeeper/ZooKeeperWrapper.java
<http://review.hbase.org/r/370/#comment1917>

    confusing name when looking at what's returned, fix that



src/main/java/org/apache/hadoop/hbase/zookeeper/ZooKeeperWrapper.java
<http://review.hbase.org/r/370/#comment1921>

    Why two lines for nodes? Also, if nodes is null for any reason, won't that throw an NPE?



src/main/java/org/apache/hadoop/hbase/zookeeper/ZooKeeperWrapper.java
<http://review.hbase.org/r/370/#comment1924>

     most of that stuff can be removed and put into the 



src/main/java/org/apache/hadoop/hbase/zookeeper/ZooKeeperWrapper.java
<http://review.hbase.org/r/370/#comment1928>

    so you create a lock with data=null?



src/main/java/org/apache/hadoop/hbase/zookeeper/ZooKeeperWrapper.java
<http://review.hbase.org/r/370/#comment1930>

    Or you were just disconnected, could mean a lot of things right?



src/main/java/org/apache/hadoop/hbase/zookeeper/ZooKeeperWrapper.java
<http://review.hbase.org/r/370/#comment1915>

    JavaBean convention, don't start parameters' name with upper case



src/main/java/org/apache/hadoop/hbase/zookeeper/ZooKeeperWrapper.java
<http://review.hbase.org/r/370/#comment1916>

    So we log here and we log in LogSplitter, remove one of them?



src/main/java/org/apache/hadoop/hbase/zookeeper/ZooKeeperWrapper.java
<http://review.hbase.org/r/370/#comment1933>

    again, name confusing WRT returned type



src/main/java/org/apache/hadoop/hbase/zookeeper/ZooKeeperWrapper.java
<http://review.hbase.org/r/370/#comment1931>

    same comment
    



src/main/java/org/apache/hadoop/hbase/zookeeper/ZooKeeperWrapper.java
<http://review.hbase.org/r/370/#comment1934>

    don't start with upper case



src/main/java/org/apache/hadoop/hbase/zookeeper/ZooKeeperWrapper.java
<http://review.hbase.org/r/370/#comment1938>

    Usually ppl check that the other way around



src/main/java/org/apache/hadoop/hbase/zookeeper/ZooKeeperWrapper.java
<http://review.hbase.org/r/370/#comment1936>

    use HConstants.EMPTY_BYTE_ARRAY



src/main/java/org/apache/hadoop/hbase/zookeeper/ZooKeeperWrapper.java
<http://review.hbase.org/r/370/#comment1940>

    third ERROR line if splitPath is null, keep only one



src/main/java/org/apache/hadoop/hbase/zookeeper/ZooKeeperWrapper.java
<http://review.hbase.org/r/370/#comment1943>

    pull the next lines on this one with a tertiary operator



src/main/java/org/apache/hadoop/hbase/zookeeper/ZooKeeperWrapper.java
<http://review.hbase.org/r/370/#comment1941>

    EMPTY_BYTE_ARRAY



src/main/java/org/apache/hadoop/hbase/zookeeper/ZooKeeperWrapper.java
<http://review.hbase.org/r/370/#comment1942>

    Use Bytes.toBytes



src/main/java/org/apache/hadoop/hbase/zookeeper/ZooKeeperWrapper.java
<http://review.hbase.org/r/370/#comment1944>

    Use Bytes.toString



src/test/java/org/apache/hadoop/hbase/regionserver/wal/DistributedTestHLog.java
<http://review.hbase.org/r/370/#comment1911>

    copy pasta, we're in 2010 now! :P



src/test/java/org/apache/hadoop/hbase/regionserver/wal/DistributedTestHLog.java
<http://review.hbase.org/r/370/#comment1910>

    clean



src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestHLogSplit.java
<http://review.hbase.org/r/370/#comment1912>

    white spaces...



src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestLogRolling.java
<http://review.hbase.org/r/370/#comment1913>

    clean


- Jean-Daniel





> [performance] Distributed splitting of regionserver commit logs
> ---------------------------------------------------------------
>
>                 Key: HBASE-1364
>                 URL: https://issues.apache.org/jira/browse/HBASE-1364
>             Project: HBase
>          Issue Type: Improvement
>            Reporter: stack
>            Assignee: Alex Newman
>            Priority: Critical
>             Fix For: 0.92.0
>
>         Attachments: 1 (3), 1364-v2.patch, 1364.patch
>
>          Time Spent: 8h
>  Remaining Estimate: 0h
>
> HBASE-1008 has some improvements to our log splitting on regionserver crash; but it needs
to run even faster.
> (Below is from HBASE-1008)
> In bigtable paper, the split is distributed. If we're going to have 1000 logs, we need
to distribute or at least multithread the splitting.
> 1. As is, regions starting up expect to find one reconstruction log only. Need to make
it so pick up a bunch of edit logs and it should be fine that logs are elsewhere in hdfs in
an output directory written by all split participants whether multithreaded or a mapreduce-like
distributed process (Lets write our distributed sort first as a MR so we learn whats involved;
distributed sort, as much as possible should use MR framework pieces). On startup, regions
go to this directory and pick up the files written by split participants deleting and clearing
the dir when all have been read in. Making it so can take multiple logs for input, can also
make the split process more robust rather than current tenuous process which loses all edits
if it doesn't make it to the end without error.
> 2. Each column family rereads the reconstruction log to find its edits. Need to fix that.
Split can sort the edits by column family so store only reads its edits.

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