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-50) Snapshot of table
Date Wed, 11 Aug 2010 18:49:29 GMT

    [ 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
<http://review.cloudera.org/r/467/#comment2820>

    final



src/main/java/org/apache/hadoop/hbase/master/SnapshotMonitor.java
<http://review.cloudera.org/r/467/#comment2821>

    Why let this out?



src/main/java/org/apache/hadoop/hbase/master/SnapshotMonitor.java
<http://review.cloudera.org/r/467/#comment2822>

    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
<http://review.cloudera.org/r/467/#comment2823>

    This class looks good.



src/main/java/org/apache/hadoop/hbase/master/SnapshotTracker.java
<http://review.cloudera.org/r/467/#comment2824>

    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
<http://review.cloudera.org/r/467/#comment2825>

    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
<http://review.cloudera.org/r/467/#comment2826>

    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
<http://review.cloudera.org/r/467/#comment2827>

    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
<http://review.cloudera.org/r/467/#comment2828>

    Its not a backup, its creating references, right?



src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
<http://review.cloudera.org/r/467/#comment2829>

    This looks like an improvement, making it static so can be used more generally.



src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
<http://review.cloudera.org/r/467/#comment2830>

    And flushing is disabled at this point too, right?  Compactions? (Good).



src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
<http://review.cloudera.org/r/467/#comment2831>

    If snapshot fails, do we have to do cleanup?



src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java
<http://review.cloudera.org/r/467/#comment2832>

    Good



src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java
<http://review.cloudera.org/r/467/#comment2833>

    This looks good.



src/main/java/org/apache/hadoop/hbase/regionserver/SnapshotThread.java
<http://review.cloudera.org/r/467/#comment2834>

    Call this Snapshotter instead?



src/main/java/org/apache/hadoop/hbase/regionserver/Store.java
<http://review.cloudera.org/r/467/#comment2835>

    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
<http://review.cloudera.org/r/467/#comment2836>

    Why you have to pass the reference?  It wasn't needed previously?



src/main/java/org/apache/hadoop/hbase/util/FSUtils.java
<http://review.cloudera.org/r/467/#comment2837>

    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
<http://review.cloudera.org/r/467/#comment2838>

    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
<http://review.cloudera.org/r/467/#comment2839>

    Good test



src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegionSnapshot.java
<http://review.cloudera.org/r/467/#comment2840>

    There is getTestDir(final String subdirName) FYI



src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegionSnapshot.java
<http://review.cloudera.org/r/467/#comment2841>

    Looks good.



src/test/java/org/apache/hadoop/hbase/regionserver/TestZKSnapshotWatcher.java
<http://review.cloudera.org/r/467/#comment2842>

    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.


Mime
View raw message