Return-Path: Delivered-To: apmail-hbase-issues-archive@www.apache.org Received: (qmail 87095 invoked from network); 11 Aug 2010 18:49:54 -0000 Received: from unknown (HELO mail.apache.org) (140.211.11.3) by 140.211.11.9 with SMTP; 11 Aug 2010 18:49:54 -0000 Received: (qmail 72640 invoked by uid 500); 11 Aug 2010 18:49:54 -0000 Delivered-To: apmail-hbase-issues-archive@hbase.apache.org Received: (qmail 72564 invoked by uid 500); 11 Aug 2010 18:49:54 -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 72511 invoked by uid 99); 11 Aug 2010 18:49:53 -0000 Received: from nike.apache.org (HELO nike.apache.org) (192.87.106.230) by apache.org (qpsmtpd/0.29) with ESMTP; Wed, 11 Aug 2010 18:49:53 +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; Wed, 11 Aug 2010 18:49:51 +0000 Received: from thor (localhost [127.0.0.1]) by thor.apache.org (8.13.8+Sun/8.13.8) with ESMTP id o7BInTID014485 for ; Wed, 11 Aug 2010 18:49:29 GMT Message-ID: <28739120.283221281552569233.JavaMail.jira@thor> Date: Wed, 11 Aug 2010 14:49:29 -0400 (EDT) From: "HBase Review Board (JIRA)" To: issues@hbase.apache.org Subject: [jira] Commented: (HBASE-50) Snapshot of table 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-50?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12897382#action_12897382 ] HBase Review Board commented on HBASE-50: ----------------------------------------- Message from: stack@duboce.net ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://review.cloudera.org/r/467/#review840 ----------------------------------------------------------- This is looking really great Li. If you can address the comments below in a new version, lets get this patch committed. Unfortunately, the master rewrite is going to change a bunch of the stuff that this patch depends on. For example, BaseScanner is going away. But, thats not your issue. What about scaling? The only problematic issue I saw was copy of storefiles on restore. We should file an issue to do that via a MR job. src/main/java/org/apache/hadoop/hbase/master/SnapshotMonitor.java final src/main/java/org/apache/hadoop/hbase/master/SnapshotMonitor.java Why let this out? src/main/java/org/apache/hadoop/hbase/master/SnapshotMonitor.java Want to remove this or enable the assertion? One or the other I'd say rather than this. src/main/java/org/apache/hadoop/hbase/master/SnapshotOperation.java This class looks good. src/main/java/org/apache/hadoop/hbase/master/SnapshotTracker.java Its a pity this class is named so. We're about to bring in a new patch that redoes the zk stuff -- breaks it up into pieces each with a singular purpose; e.g. tracking root location or tracking meta region server -- and unfortunately the pattern is to name these purposed classes *Tracker. There'll be a clash of this kinda Tracker and the new zk Trackers. Not important, just saying in case you have another name in mind for this class. src/main/java/org/apache/hadoop/hbase/master/SnapshotTracker.java Can you make this a configuration? int maxRetries = Configuration.getInt("hbase.snapshot.retries", 3); or something? It doesn't have to go into hbase-default.xml src/main/java/org/apache/hadoop/hbase/master/SnapshotTracker.java Yeah, can the max retries be made into a data member rather than hardcoded everywhere in this class? src/main/java/org/apache/hadoop/hbase/master/TableSnapshot.java Looks like you need more explaination here? This is a special case right where the table is offline and we're asked to make a snapshot of it? src/main/java/org/apache/hadoop/hbase/master/TableSnapshot.java Its not a backup, its creating references, right? src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java This looks like an improvement, making it static so can be used more generally. src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java And flushing is disabled at this point too, right? Compactions? (Good). src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java If snapshot fails, do we have to do cleanup? src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java Good src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java This looks good. src/main/java/org/apache/hadoop/hbase/regionserver/SnapshotThread.java Call this Snapshotter instead? src/main/java/org/apache/hadoop/hbase/regionserver/Store.java Do we have to do this down at the Store level? Coud we move it up to Region or up to the RegionServer itself? It already has an HTable instance. src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.java Why you have to pass the reference? It wasn't needed previously? src/main/java/org/apache/hadoop/hbase/util/FSUtils.java Does this stuff belong in here in this general utility class? Should it be polluted with References? Should this stuff be over in io package where the Reference is or static methods on Reference? src/test/java/org/apache/hadoop/hbase/master/TestSnapshot.java What about a test of restore from snapshot? Is there one? I dont' see it? src/test/java/org/apache/hadoop/hbase/master/TestSnapshotFailure.java Good test src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegionSnapshot.java There is getTestDir(final String subdirName) FYI src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegionSnapshot.java Looks good. src/test/java/org/apache/hadoop/hbase/regionserver/TestZKSnapshotWatcher.java Great - stack > Snapshot of table > ----------------- > > Key: HBASE-50 > URL: https://issues.apache.org/jira/browse/HBASE-50 > Project: HBase > Issue Type: New Feature > Reporter: Billy Pearson > Assignee: Li Chongxin > Priority: Minor > Attachments: HBase Snapshot Design Report V2.pdf, HBase Snapshot Design Report V3.pdf, HBase Snapshot Implementation Plan.pdf, Snapshot Class Diagram.png > > > Havening an option to take a snapshot of a table would be vary useful in production. > What I would like to see this option do is do a merge of all the data into one or more files stored in the same folder on the dfs. This way we could save data in case of a software bug in hadoop or user code. > The other advantage would be to be able to export a table to multi locations. Say I had a read_only table that must be online. I could take a snapshot of it when needed and export it to a separate data center and have it loaded there and then i would have it online at multi data centers for load balancing and failover. > I understand that hadoop takes the need out of havening backup to protect from failed servers, but this does not protect use from software bugs that might delete or alter data in ways we did not plan. We should have a way we can roll back a dataset. -- This message is automatically generated by JIRA. - You can reply to this email to add a comment to the issue online.