Return-Path: Delivered-To: apmail-hbase-issues-archive@www.apache.org Received: (qmail 86603 invoked from network); 23 Jul 2010 19:17:29 -0000 Received: from unknown (HELO mail.apache.org) (140.211.11.3) by 140.211.11.9 with SMTP; 23 Jul 2010 19:17:29 -0000 Received: (qmail 47735 invoked by uid 500); 23 Jul 2010 19:17:29 -0000 Delivered-To: apmail-hbase-issues-archive@hbase.apache.org Received: (qmail 47676 invoked by uid 500); 23 Jul 2010 19:17:28 -0000 Mailing-List: contact issues-help@hbase.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Delivered-To: mailing list issues@hbase.apache.org Received: (qmail 47668 invoked by uid 99); 23 Jul 2010 19:17:28 -0000 Received: from nike.apache.org (HELO nike.apache.org) (192.87.106.230) by apache.org (qpsmtpd/0.29) with ESMTP; Fri, 23 Jul 2010 19:17:28 +0000 X-ASF-Spam-Status: No, hits=-2000.0 required=10.0 tests=ALL_TRUSTED X-Spam-Check-By: apache.org Received: from [140.211.11.22] (HELO thor.apache.org) (140.211.11.22) by apache.org (qpsmtpd/0.29) with ESMTP; Fri, 23 Jul 2010 19:17:25 +0000 Received: from thor (localhost [127.0.0.1]) by thor.apache.org (8.13.8+Sun/8.13.8) with ESMTP id o6NJH3Tq020015 for ; Fri, 23 Jul 2010 19:17:04 GMT Message-ID: <7648710.554591279912623815.JavaMail.jira@thor> Date: Fri, 23 Jul 2010 15:17:03 -0400 (EDT) From: "HBase Review Board (JIRA)" To: issues@hbase.apache.org Subject: [jira] Commented: (HBASE-1364) [performance] Distributed splitting of regionserver commit logs MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-JIRA-FingerPrint: 30527f35849b9dde25b450d4833f0394 X-Virus-Checked: Checked by ClamAV on apache.org [ 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" ----------------------------------------------------------- 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 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 Fix those long lines. src/main/java/org/apache/hadoop/hbase/regionserver/LogSplitter.java rogue "q" src/main/java/org/apache/hadoop/hbase/regionserver/LogSplitter.java Why are those static? src/main/java/org/apache/hadoop/hbase/regionserver/LogSplitter.java 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 both process and run call this method, can there be a race? src/main/java/org/apache/hadoop/hbase/regionserver/LogSplitter.java don't need to declare this here src/main/java/org/apache/hadoop/hbase/regionserver/LogSplitter.java What does that mean? src/main/java/org/apache/hadoop/hbase/regionserver/LogSplitter.java why two lines? src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java rogue "b" src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java ? src/main/java/org/apache/hadoop/hbase/zookeeper/ZooKeeperWrapper.java confusing name when looking at what's returned, fix that src/main/java/org/apache/hadoop/hbase/zookeeper/ZooKeeperWrapper.java 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 most of that stuff can be removed and put into the src/main/java/org/apache/hadoop/hbase/zookeeper/ZooKeeperWrapper.java so you create a lock with data=null? src/main/java/org/apache/hadoop/hbase/zookeeper/ZooKeeperWrapper.java Or you were just disconnected, could mean a lot of things right? src/main/java/org/apache/hadoop/hbase/zookeeper/ZooKeeperWrapper.java JavaBean convention, don't start parameters' name with upper case src/main/java/org/apache/hadoop/hbase/zookeeper/ZooKeeperWrapper.java So we log here and we log in LogSplitter, remove one of them? src/main/java/org/apache/hadoop/hbase/zookeeper/ZooKeeperWrapper.java again, name confusing WRT returned type src/main/java/org/apache/hadoop/hbase/zookeeper/ZooKeeperWrapper.java same comment src/main/java/org/apache/hadoop/hbase/zookeeper/ZooKeeperWrapper.java don't start with upper case src/main/java/org/apache/hadoop/hbase/zookeeper/ZooKeeperWrapper.java Usually ppl check that the other way around src/main/java/org/apache/hadoop/hbase/zookeeper/ZooKeeperWrapper.java use HConstants.EMPTY_BYTE_ARRAY src/main/java/org/apache/hadoop/hbase/zookeeper/ZooKeeperWrapper.java third ERROR line if splitPath is null, keep only one src/main/java/org/apache/hadoop/hbase/zookeeper/ZooKeeperWrapper.java pull the next lines on this one with a tertiary operator src/main/java/org/apache/hadoop/hbase/zookeeper/ZooKeeperWrapper.java EMPTY_BYTE_ARRAY src/main/java/org/apache/hadoop/hbase/zookeeper/ZooKeeperWrapper.java Use Bytes.toBytes src/main/java/org/apache/hadoop/hbase/zookeeper/ZooKeeperWrapper.java Use Bytes.toString src/test/java/org/apache/hadoop/hbase/regionserver/wal/DistributedTestHLog.java copy pasta, we're in 2010 now! :P src/test/java/org/apache/hadoop/hbase/regionserver/wal/DistributedTestHLog.java clean src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestHLogSplit.java white spaces... src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestLogRolling.java 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.