hbase-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Chongxin Li" <lichong...@zju.edu.cn>
Subject Re: Review Request: HBASE-50: Snapshot of table
Date Thu, 12 Aug 2010 07:50:48 GMT


> On 2010-08-11 11:32:27, stack wrote:
> > src/main/java/org/apache/hadoop/hbase/master/SnapshotMonitor.java, line 166
> > <http://review.cloudera.org/r/467/diff/3/?file=6019#file6019line166>
> >
> >     Want to remove this or enable the assertion?  One or the other I'd say rather
than this.

remove it


> On 2010-08-11 11:32:27, stack wrote:
> > src/main/java/org/apache/hadoop/hbase/master/SnapshotTracker.java, line 1
> > <http://review.cloudera.org/r/467/diff/3/?file=6021#file6021line1>
> >
> >     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.

I'll think about it. Any suggestion?


> On 2010-08-11 11:32:27, stack wrote:
> > src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java, line 2288
> > <http://review.cloudera.org/r/467/diff/3/?file=6024#file6024line2288>
> >
> >     And flushing is disabled at this point too, right?  Compactions? (Good).

yes, flushing and compaction are disabled when snapshot.


> On 2010-08-11 11:32:27, stack wrote:
> > src/main/java/org/apache/hadoop/hbase/regionserver/Store.java, line 944
> > <http://review.cloudera.org/r/467/diff/3/?file=6027#file6027line944>
> >
> >     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.

This method is only used to delete old store files after compaction, is it appropriate to
move it to Region?


> On 2010-08-11 11:32:27, stack wrote:
> > src/test/java/org/apache/hadoop/hbase/master/TestSnapshot.java, line 382
> > <http://review.cloudera.org/r/467/diff/3/?file=6037#file6037line382>
> >
> >     What about a test of restore from snapshot?  Is there one?  I dont' see it?

It's already in TestAdmin


> On 2010-08-11 11:32:27, stack wrote:
> > src/main/java/org/apache/hadoop/hbase/util/FSUtils.java, line 713
> > <http://review.cloudera.org/r/467/diff/3/?file=6032#file6032line713>
> >
> >     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?

OK, I'll move it to Reference


> On 2010-08-11 11:32:27, stack wrote:
> > src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.java, line 267
> > <http://review.cloudera.org/r/467/diff/3/?file=6028#file6028line267>
> >
> >     Why you have to pass the reference?  It wasn't needed previously?

Previously there is only one type of reference file, i.e. reference after split. But right
now there are another type of reference file for snapshot. We need to know the reference type
to get the referred to file. 

This is used for table restored from snapshot.


> On 2010-08-11 11:32:27, stack wrote:
> > src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java, line 2355
> > <http://review.cloudera.org/r/467/diff/3/?file=6024#file6024line2355>
> >
> >     If snapshot fails, do we have to do cleanup?

HRegions just quit the snapshot mode if fails. The master would be notified with failure and
do the clean up work for the whole snapshot.


- Chongxin


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://review.cloudera.org/r/467/#review840
-----------------------------------------------------------


On 2010-08-09 03:52:11, Chongxin Li wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://review.cloudera.org/r/467/
> -----------------------------------------------------------
> 
> (Updated 2010-08-09 03:52:11)
> 
> 
> Review request for hbase.
> 
> 
> Summary
> -------
> 
> This patch includes the first three sub-tasks of HBASE-50:
> 1. Start and monitor the creation of snapshot via ZooKeeper
> 2. Create snapshot of an HBase table
> 3. Some existing functions of HBase are modified to support snapshot
> 
> Currently snapshots can be created as expected, but can not be restored or deleted yet
> 
> 
> This addresses bug HBASE-50.
>     http://issues.apache.org/jira/browse/HBASE-50
> 
> 
> Diffs
> -----
> 
>   src/main/java/org/apache/hadoop/hbase/HConstants.java c77ebf5 
>   src/main/java/org/apache/hadoop/hbase/HRegionInfo.java ee94690 
>   src/main/java/org/apache/hadoop/hbase/HSnapshotDescriptor.java PRE-CREATION 
>   src/main/java/org/apache/hadoop/hbase/HTableDescriptor.java 0d57270 
>   src/main/java/org/apache/hadoop/hbase/SnapshotExistsException.java PRE-CREATION 
>   src/main/java/org/apache/hadoop/hbase/TablePartialOpenException.java PRE-CREATION 
>   src/main/java/org/apache/hadoop/hbase/client/HBaseAdmin.java 8b01aa0 
>   src/main/java/org/apache/hadoop/hbase/io/HalfStoreFileReader.java ed12e7a 
>   src/main/java/org/apache/hadoop/hbase/io/HbaseObjectWritable.java 85fde3a 
>   src/main/java/org/apache/hadoop/hbase/io/Reference.java 219203c 
>   src/main/java/org/apache/hadoop/hbase/io/hfile/HFile.java b2de7e4 
>   src/main/java/org/apache/hadoop/hbase/ipc/HBaseRPCProtocolVersion.java d4bcbed 
>   src/main/java/org/apache/hadoop/hbase/ipc/HMasterInterface.java bd48a4b 
>   src/main/java/org/apache/hadoop/hbase/mapreduce/LoadIncrementalHFiles.java 1183584

>   src/main/java/org/apache/hadoop/hbase/master/BaseScanner.java 69eab39 
>   src/main/java/org/apache/hadoop/hbase/master/DeleteSnapshot.java PRE-CREATION 
>   src/main/java/org/apache/hadoop/hbase/master/HMaster.java e4bd30d 
>   src/main/java/org/apache/hadoop/hbase/master/LogsCleaner.java 9d1a8b8 
>   src/main/java/org/apache/hadoop/hbase/master/RestoreSnapshot.java PRE-CREATION 
>   src/main/java/org/apache/hadoop/hbase/master/SnapshotLogCleaner.java PRE-CREATION 
>   src/main/java/org/apache/hadoop/hbase/master/SnapshotMonitor.java PRE-CREATION 
>   src/main/java/org/apache/hadoop/hbase/master/SnapshotOperation.java PRE-CREATION 
>   src/main/java/org/apache/hadoop/hbase/master/SnapshotTracker.java PRE-CREATION 
>   src/main/java/org/apache/hadoop/hbase/master/TableDelete.java 1153e62 
>   src/main/java/org/apache/hadoop/hbase/master/TableSnapshot.java PRE-CREATION 
>   src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java 6dc41a4 
>   src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 6a54736 
>   src/main/java/org/apache/hadoop/hbase/regionserver/SnapshotThread.java PRE-CREATION

>   src/main/java/org/apache/hadoop/hbase/regionserver/Store.java ae9e190 
>   src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.java 757a50c 
>   src/main/java/org/apache/hadoop/hbase/regionserver/ZKSnapshotWatcher.java PRE-CREATION

>   src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java 9593286 
>   src/main/java/org/apache/hadoop/hbase/replication/master/ReplicationLogCleaner.java
4d4b00a 
>   src/main/java/org/apache/hadoop/hbase/util/FSUtils.java 5cf3481 
>   src/main/java/org/apache/hadoop/hbase/zookeeper/ZooKeeperWrapper.java 3827fa5 
>   src/main/resources/hbase-default.xml b73f0ff 
>   src/test/java/org/apache/hadoop/hbase/HBaseTestingUtility.java 4d09fe9 
>   src/test/java/org/apache/hadoop/hbase/client/TestAdmin.java c9b78b9 
>   src/test/java/org/apache/hadoop/hbase/master/TestSnapshot.java PRE-CREATION 
>   src/test/java/org/apache/hadoop/hbase/master/TestSnapshotFailure.java PRE-CREATION

>   src/test/java/org/apache/hadoop/hbase/regionserver/TestCompaction.java 34b8044 
>   src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegion.java 98bd3e5 
>   src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegionSnapshot.java PRE-CREATION

>   src/test/java/org/apache/hadoop/hbase/regionserver/TestStoreFile.java 38ef520 
>   src/test/java/org/apache/hadoop/hbase/regionserver/TestZKSnapshotWatcher.java PRE-CREATION

> 
> Diff: http://review.cloudera.org/r/467/diff
> 
> 
> Testing
> -------
> 
> Unit tests and integration tests with mini cluster passed.
> 
> 
> Thanks,
> 
> Chongxin
> 
>


Mime
View raw message