You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hbase.apache.org by Chongxin Li <li...@zju.edu.cn> on 2010/08/01 12:51:44 UTC

Review Request: HBASE-50: Snapshot of table

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

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/TablePartialOpenException.java PRE-CREATION 
  src/main/java/org/apache/hadoop/hbase/client/HBaseAdmin.java 8b01aa0 
  src/main/java/org/apache/hadoop/hbase/io/Reference.java 219203c 
  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/master/BaseScanner.java 69eab39 
  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/SnapshotLogCleaner.java PRE-CREATION 
  src/main/java/org/apache/hadoop/hbase/master/SnapshotMonitor.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/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/TestZKSnapshotWatcher.java PRE-CREATION 

Diff: http://review.cloudera.org/r/467/diff


Testing
-------

Unit tests and integration tests with mini cluster passed.


Thanks,

Chongxin


Re: Review Request: HBASE-50: Snapshot of table

Posted by Chongxin Li <li...@zju.edu.cn>.

> On 2010-08-10 21:34:40, stack wrote:
> > src/main/java/org/apache/hadoop/hbase/HTableDescriptor.java, line 673
> > <http://review.cloudera.org/r/467/diff/3/?file=6002#file6002line673>
> >
> >     This is fine for an hbase that is a fresh install but what about case where the data has been migrated from an older hbase version; it won't have this column family in .META.  We should make a little migration script that adds it or on start of new version, check for it and if not present, create it.
> 
> Chongxin Li wrote:
>     That's right. But AddColumn operation requires the table disabled to proceed, ROOT table can not be disabled once the system is started. Then how could we execute the migration script or check and create it on start of new version?

This can be done with a script when HBase is shutdown. The script scans the root region with MetaUtils and add the column family SNAPSHOT to .META. table?


- Chongxin


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


On 2010-08-12 02:43:42, Chongxin Li wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://review.cloudera.org/r/467/
> -----------------------------------------------------------
> 
> (Updated 2010-08-12 02:43:42)
> 
> 
> 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/HTableDescriptor.java 0d57270 
>   src/main/java/org/apache/hadoop/hbase/SnapshotDescriptor.java PRE-CREATION 
>   src/main/java/org/apache/hadoop/hbase/SnapshotExistsException.java PRE-CREATION 
>   src/main/java/org/apache/hadoop/hbase/TablePartiallyOpenException.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/Snapshotter.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/TestLogsCleaner.java 8b7f60f 
>   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
> 
>


Re: Review Request: HBASE-50: Snapshot of table

Posted by Chongxin Li <li...@zju.edu.cn>.

> On 2010-08-10 21:34:40, stack wrote:
> > src/main/java/org/apache/hadoop/hbase/HTableDescriptor.java, line 673
> > <http://review.cloudera.org/r/467/diff/3/?file=6002#file6002line673>
> >
> >     This is fine for an hbase that is a fresh install but what about case where the data has been migrated from an older hbase version; it won't have this column family in .META.  We should make a little migration script that adds it or on start of new version, check for it and if not present, create it.

That's right. But AddColumn operation requires the table disabled to proceed, ROOT table can not be disabled once the system is started. Then how could we execute the migration script or check and create it on start of new version?


> On 2010-08-10 21:34:40, stack wrote:
> > src/main/java/org/apache/hadoop/hbase/client/HBaseAdmin.java, line 899
> > <http://review.cloudera.org/r/467/diff/3/?file=6005#file6005line899>
> >
> >     Can the snapshot name be empty and then we'll make one up?

a default snapshot name? or a auto-generated snapshot name, such as creation time?


> On 2010-08-10 21:34:40, stack wrote:
> > src/main/java/org/apache/hadoop/hbase/client/HBaseAdmin.java, line 951
> > <http://review.cloudera.org/r/467/diff/3/?file=6005#file6005line951>
> >
> >     For restore of the snapshot, do you use loadtable.rb or Todd's new bulkloading scripts?

Currently, NO...
Snapshot is composed of a list of log files and a bunch of reference files for HFiles of the table. These reference files have the same hierarchy as the original table and the name is in the format of "1239384747630.tablename", where the front is the file name of the referred HFile and the latter is table name for snapshot. Thus to restore a snapshot, just copy reference files (which are just a few bytes) to the table dir, update the META and split the logs. When this table is enabled, the system know how to replay the commit edits and read such a reference file. Methods getReferredToFile, open in StoreFile are updated to deal with this kind of reference files for snapshots.

At present, snapshot can only be restored to the table whose name is the same as the one for which the snapshot is created. That the old table with the same name must be deleted before restore a snapshot. That's what I do in unit test TestAdmin. Restoring snapshot to a different table name has a low priority. It has not been implemented yet.


> On 2010-08-10 21:34:40, stack wrote:
> > src/main/java/org/apache/hadoop/hbase/io/Reference.java, line 50
> > <http://review.cloudera.org/r/467/diff/3/?file=6008#file6008line50>
> >
> >     Whats this?  A different kind of reference?

Yes.. This is the reference file in snapshot. It references an HFile of the original table.


> On 2010-08-10 21:34:40, stack wrote:
> > src/main/java/org/apache/hadoop/hbase/master/SnapshotLogCleaner.java, line 115
> > <http://review.cloudera.org/r/467/diff/3/?file=6018#file6018line115>
> >
> >     This looks like a class that you could write a unit test for?

Sure, I'll add another case in TestLogsCleaner.


> On 2010-08-10 21:34:40, stack wrote:
> > src/main/java/org/apache/hadoop/hbase/master/RestoreSnapshot.java, line 130
> > <http://review.cloudera.org/r/467/diff/3/?file=6017#file6017line130>
> >
> >     If table were big, this could be prohibitively expensive?  A single-threaded copy of all of a tables data?  We could compliment this with MR-base restore, something that did the copy using MR?

This method is only used in RestoreSnapshot, where reference files of snapshot are copied to the table dir. These reference files just contains a few bytes instead of the table's data. Snapshots share the table data with the original table and other snapshots. Do we still need a MR job?


> On 2010-08-10 21:34:40, stack wrote:
> > src/main/java/org/apache/hadoop/hbase/master/BaseScanner.java, line 212
> > <http://review.cloudera.org/r/467/diff/3/?file=6013#file6013line212>
> >
> >     Why Random negative number?  Why not just leave it blank?

If a blank value is used as the key, there would be only one item at last if it is the first few times to scan the regions. Using random negative number indicates all these regions have not been scanned before. If it has been scanned, there would be a last checking time for it instead.


> On 2010-08-10 21:34:40, stack wrote:
> > src/main/java/org/apache/hadoop/hbase/mapreduce/LoadIncrementalHFiles.java, line 251
> > <http://review.cloudera.org/r/467/diff/3/?file=6012#file6012line251>
> >
> >     Is this comment right?

I just renamed the Ranges to caps, comment was not changed.


> On 2010-08-10 21:34:40, stack wrote:
> > src/main/java/org/apache/hadoop/hbase/io/Reference.java, line 149
> > <http://review.cloudera.org/r/467/diff/3/?file=6008#file6008line149>
> >
> >     Hmm... is this good?  You are dropping some the region name when you toString.  Do we have to?

This has not been changed. Just rename field "region" to "range"


> On 2010-08-10 21:34:40, stack wrote:
> > src/main/java/org/apache/hadoop/hbase/io/Reference.java, line 156
> > <http://review.cloudera.org/r/467/diff/3/?file=6008#file6008line156>
> >
> >     This could be a problem when fellas migrate old data to use this new hbase.  If there are References in the old data, then this deserialization will fail?  I'm fine w/ you creating a new issue named something like "Migration from 0.20.x hbase to 0.90" and adding a note in there that we need to consider this little issue.  Better though would be if the data was able to migrate itself at runtime; i.e. recognize a boolean on the stream and then deserialize the old style into the new, etc.

Actually I think it is fine to migrate old data to new hbase. Old references are serialized by DataOutput.writeBoolean(boolean), where value (byte)1 is written if the argument is true and value (byte)0 is written if argument is false. 

See (from Ted's review):
http://download.oracle.com/javase/1.4.2/docs/api/java/io/DataOutput.html#writeBoolean%28boolean%29

Thus value (byte)1 was written if it is the top file region (isTopFileRegion is true). That is exactly the same as current value of TOP. For the same reason, this deserialization would work for the references in the old data, right? 

That's why we can not use ordinal of Enum and serialize the int value. The serialization size of this field would be different for the new data and old data if int value is used.


- Chongxin


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


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


Re: Review Request: HBASE-50: Snapshot of table

Posted by Chongxin Li <li...@zju.edu.cn>.

> On 2010-08-10 21:34:40, stack wrote:
> > src/main/java/org/apache/hadoop/hbase/master/HMaster.java, line 234
> > <http://review.cloudera.org/r/467/diff/3/?file=6015#file6015line234>
> >
> >     You might want to check the returns from these methods.

Snapshot root dir might already exist, e.g. created in previous start up, then mkdirs would return false. But this is normal.

Here are previous comments from Todd:
you can just call mkdirs, I think, and it won't fail if it already exists


- Chongxin


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


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


Re: Review Request: HBASE-50: Snapshot of table

Posted by st...@duboce.net.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://review.cloudera.org/r/467/#review823
-----------------------------------------------------------


I reviewed the first page of diffs.  Will do second page tomorrow (This is a lovely big patch Li -- so far, so good... keep up the good work).


src/main/java/org/apache/hadoop/hbase/HTableDescriptor.java
<http://review.cloudera.org/r/467/#comment2750>

    This is fine for an hbase that is a fresh install but what about case where the data has been migrated from an older hbase version; it won't have this column family in .META.  We should make a little migration script that adds it or on start of new version, check for it and if not present, create it.



src/main/java/org/apache/hadoop/hbase/HTableDescriptor.java
<http://review.cloudera.org/r/467/#comment2751>

    ditto
    



src/main/java/org/apache/hadoop/hbase/TablePartialOpenException.java
<http://review.cloudera.org/r/467/#comment2752>

    Maybe exception should be called TablePartiallyOpenException (Not important.  Minor).



src/main/java/org/apache/hadoop/hbase/client/HBaseAdmin.java
<http://review.cloudera.org/r/467/#comment2753>

    Can the snapshot name be empty and then we'll make one up?



src/main/java/org/apache/hadoop/hbase/client/HBaseAdmin.java
<http://review.cloudera.org/r/467/#comment2754>

    Sure, why not. Its stored in the filesystem?  If so, for sure use same rule.



src/main/java/org/apache/hadoop/hbase/client/HBaseAdmin.java
<http://review.cloudera.org/r/467/#comment2755>

    For restore of the snapshot, do you use loadtable.rb or Todd's new bulkloading scripts?



src/main/java/org/apache/hadoop/hbase/io/HalfStoreFileReader.java
<http://review.cloudera.org/r/467/#comment2756>

    This looks like an improvement.



src/main/java/org/apache/hadoop/hbase/io/Reference.java
<http://review.cloudera.org/r/467/#comment2757>

    Whats this?  A different kind of reference?



src/main/java/org/apache/hadoop/hbase/io/Reference.java
<http://review.cloudera.org/r/467/#comment2758>

    Do you need to specify the ordinals?  Wont they be these numbers anyways? Or is it in case someone adds a new type of reference?



src/main/java/org/apache/hadoop/hbase/io/Reference.java
<http://review.cloudera.org/r/467/#comment2759>

    Why not just use the ordinal?  http://download-llnw.oracle.com/javase/1.5.0/docs/api/java/lang/Enum.html#ordinal()  And serialize the int?



src/main/java/org/apache/hadoop/hbase/io/Reference.java
<http://review.cloudera.org/r/467/#comment2760>

    Hmm... is this good?  You are dropping some the region name when you toString.  Do we have to?



src/main/java/org/apache/hadoop/hbase/io/Reference.java
<http://review.cloudera.org/r/467/#comment2765>

    This could be a problem when fellas migrate old data to use this new hbase.  If there are References in the old data, then this deserialization will fail?  I'm fine w/ you creating a new issue named something like "Migration from 0.20.x hbase to 0.90" and adding a note in there that we need to consider this little issue.  Better though would be if the data was able to migrate itself at runtime; i.e. recognize a boolean on the stream and then deserialize the old style into the new, etc.



src/main/java/org/apache/hadoop/hbase/ipc/HBaseRPCProtocolVersion.java
<http://review.cloudera.org/r/467/#comment2767>

    Good.



src/main/java/org/apache/hadoop/hbase/mapreduce/LoadIncrementalHFiles.java
<http://review.cloudera.org/r/467/#comment2768>

    Is this comment right?



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

    Why Random negative number?  Why not just leave it blank?



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

    It'd be cool breaking up these methods so they were static if possible so you could unit test them to ensure they do as advertised.



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

    Check the result.  It may not have worked.  If it didn't deserves a WARN at least.



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

    You might want to check the returns from these methods.



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

    Don't bother warning I'd say.. just throw



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

    Yeah, just throw... that'll show in logs anyway (I believe)



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

    I'm with Ted on this one. Need to do something about it else it'll annoy till the end of time.



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

    This looks like it should be a general utility method (Not important)



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

    If table were big, this could be prohibitively expensive?  A single-threaded copy of all of a tables data?  We could compliment this with MR-base restore, something that did the copy using MR?



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

    This looks like a class that you could write a unit test for?


- stack


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


Re: Review Request: HBASE-50: Snapshot of table

Posted by Ted Yu <te...@yahoo.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://review.cloudera.org/r/467/#review803
-----------------------------------------------------------



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

    IOException should be handled so that synchronization of reference counts isn't interrupted.


- Ted


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


Re: Review Request: HBASE-50: Snapshot of table

Posted by Chongxin Li <li...@zju.edu.cn>.

> On 2010-08-10 22:20:23, Ted Yu wrote:
> > src/main/java/org/apache/hadoop/hbase/io/Reference.java, line 156
> > <http://review.cloudera.org/r/467/diff/3/?file=6008#file6008line156>
> >
> >     I think the current code is backward compatible. Boolean value of true is interpreted as TOP, value of false is BOTTOM.
> >     Since ENTIRE is introduced, this code is not backward compatible.
> >     
> >     See:
> >     http://download.oracle.com/javase/1.4.2/docs/api/java/io/DataOutput.html#writeBoolean%28boolean%29

Why it is not backward compatible when ENTIRE is introduces? The value for ENTIRE is 2, different from the old written value of boolean.


- Chongxin


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


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


Re: Review Request: HBASE-50: Snapshot of table

Posted by Ted Yu <te...@yahoo.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://review.cloudera.org/r/467/#review829
-----------------------------------------------------------



src/main/java/org/apache/hadoop/hbase/io/Reference.java
<http://review.cloudera.org/r/467/#comment2793>

    I think the current code is backward compatible. Boolean value of true is interpreted as TOP, value of false is BOTTOM.
    Since ENTIRE is introduced, this code is not backward compatible.
    
    See:
    http://download.oracle.com/javase/1.4.2/docs/api/java/io/DataOutput.html#writeBoolean%28boolean%29


- Ted


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


Re: Review Request: HBASE-50: Snapshot of table

Posted by Ted Yu <te...@yahoo.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://review.cloudera.org/r/467/#review846
-----------------------------------------------------------



src/main/java/org/apache/hadoop/hbase/io/Reference.java
<http://review.cloudera.org/r/467/#comment2846>

    I meant value of 2 cannot be correctly interpreted as boolean.
    



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

    I think we need to limit the space consumed by failed snapshots.
    This issue can be addressed by a future JIRA.


- Ted


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


Re: Review Request: HBASE-50: Snapshot of table

Posted by Chongxin Li <li...@zju.edu.cn>.

> On 2010-08-10 10:49:06, stack wrote:
> > src/main/java/org/apache/hadoop/hbase/HSnapshotDescriptor.java, line 36
> > <http://review.cloudera.org/r/467/diff/3/?file=6001#file6001line36>
> >
> >     Drop the H.  Call it SnapshotDescriptor

Alright


> On 2010-08-10 10:49:06, stack wrote:
> > src/main/java/org/apache/hadoop/hbase/HSnapshotDescriptor.java, line 41
> > <http://review.cloudera.org/r/467/diff/3/?file=6001#file6001line41>
> >
> >     If it is in under the snapshot directory maybe just call this file snapshotinfo? Drop the '.' prefix.  The '.' prefix is usually to demark 'special' files we don't want to consider as part of normal operation.  In this case, we are under a snapshot directory, already outside of 'normal' operation.

This is named following .regioninfo


> On 2010-08-10 10:49:06, stack wrote:
> > src/main/java/org/apache/hadoop/hbase/HRegionInfo.java, line 373
> > <http://review.cloudera.org/r/467/diff/3/?file=6000#file6000line373>
> >
> >     How often is this called?  If it happens alot, it could add up -- be expensive.

Not too much actually. This method is only called in BaseScanner when reference rows in META are checked and synchronized with the reference files. And right now there would be at most five rows to be checked in one scan of META.
There is no region info saved in each reference row. Thus reference row which is a combination of SNAPSHOT_PREFIX and region name is parsed to obtain the region name. That's why we need this method.


- Chongxin


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


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


Re: Review Request: HBASE-50: Snapshot of table

Posted by st...@duboce.net.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://review.cloudera.org/r/467/#review800
-----------------------------------------------------------


Made a start. More to follow.


src/main/java/org/apache/hadoop/hbase/HConstants.java
<http://review.cloudera.org/r/467/#comment2706>

    Is this what I think it is?  We are keeping reference counts on a region up in .META.?  What about the question I had a while back on what happens when this row is deleted because the region has split and daughters no longer have reference to this parent?  Maybe this is something else.  I'll keep reading.



src/main/java/org/apache/hadoop/hbase/HConstants.java
<http://review.cloudera.org/r/467/#comment2708>

    ok.. I think I see whats going to happen (perhaps ignore previous comment)



src/main/java/org/apache/hadoop/hbase/HRegionInfo.java
<http://review.cloudera.org/r/467/#comment2709>

    How often is this called?  If it happens alot, it could add up -- be expensive.



src/main/java/org/apache/hadoop/hbase/HRegionInfo.java
<http://review.cloudera.org/r/467/#comment2710>

    Ok.  Good.



src/main/java/org/apache/hadoop/hbase/HSnapshotDescriptor.java
<http://review.cloudera.org/r/467/#comment2711>

    Drop the H.  Call it SnapshotDescriptor



src/main/java/org/apache/hadoop/hbase/HSnapshotDescriptor.java
<http://review.cloudera.org/r/467/#comment2712>

    If it is in under the snapshot directory maybe just call this file snapshotinfo? Drop the '.' prefix.  The '.' prefix is usually to demark 'special' files we don't want to consider as part of normal operation.  In this case, we are under a snapshot directory, already outside of 'normal' operation.


- stack


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


Re: Review Request: HBASE-50: Snapshot of table

Posted by Chongxin Li <li...@zju.edu.cn>.

> On 2010-08-10 10:04:44, Ted Yu wrote:
> > src/main/java/org/apache/hadoop/hbase/master/HMaster.java, line 962
> > <http://review.cloudera.org/r/467/diff/3/?file=6015#file6015line962>
> >
> >     It would be better to move crashed snapshots into a separate directory under snapshot rootDir.

If so, probably we need the above method.
But why move crashed snapshots into a separate directory? It would be pretty hard to recover a crashed snapshot.


> On 2010-08-10 10:04:44, Ted Yu wrote:
> > src/main/java/org/apache/hadoop/hbase/master/HMaster.java, line 945
> > <http://review.cloudera.org/r/467/diff/3/?file=6015#file6015line945>
> >
> >     If you create directory for failed snapshots, you can also add listFailedSnapshots() method.

Currently there is no directory for failed snapshots. If snapshot fails, it is cleaned up and exception is thrown to notify the user.


> On 2010-08-10 10:04:44, Ted Yu wrote:
> > src/main/java/org/apache/hadoop/hbase/master/HMaster.java, line 930
> > <http://review.cloudera.org/r/467/diff/3/?file=6015#file6015line930>
> >
> >     Do we need to abort TableSnapshot processing in case of exception ?

For snapshot which is created by TableSnapshot, the table must be offline and snapshot is totally driven by the master. Region servers have no awareness of such a snapshot. So in case of exception, just clean up the failed snapshot. There is no need to abort the snapshot across the cluster.

Regarding SnapshotMonitor, it only monitors the snapshots which are created across the region servers.


- Chongxin


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


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


Re: Review Request: HBASE-50: Snapshot of table

Posted by Ted Yu <te...@yahoo.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://review.cloudera.org/r/467/#review799
-----------------------------------------------------------



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

    Do we need to abort TableSnapshot processing in case of exception ?



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

    If you create directory for failed snapshots, you can also add listFailedSnapshots() method.



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

    It would be better to move crashed snapshots into a separate directory under snapshot rootDir.


- Ted


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


Re: Review Request: HBASE-50: Snapshot of table

Posted by Ted Yu <te...@yahoo.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://review.cloudera.org/r/467/#review897
-----------------------------------------------------------



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

    We should log if we fail to delete.



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

    Yes.


- Ted


On 2010-08-12 02:43:42, Chongxin Li wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://review.cloudera.org/r/467/
> -----------------------------------------------------------
> 
> (Updated 2010-08-12 02:43:42)
> 
> 
> 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/HTableDescriptor.java 0d57270 
>   src/main/java/org/apache/hadoop/hbase/SnapshotDescriptor.java PRE-CREATION 
>   src/main/java/org/apache/hadoop/hbase/SnapshotExistsException.java PRE-CREATION 
>   src/main/java/org/apache/hadoop/hbase/TablePartiallyOpenException.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/Snapshotter.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/TestLogsCleaner.java 8b7f60f 
>   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
> 
>


Re: Review Request: HBASE-50: Snapshot of table

Posted by Ted Yu <yu...@gmail.com>.
Here are other choices for SnapshotTracker (see
http://freethesaurus.net/s.php?q=watcher):
SnapshotObserver, SnapshotSentinel

I prefer the second one.

For HBase root directory:
        HBaseConfiguration conf = new HBaseConfiguration();
        String rootDir = conf.get("hbase.rootdir");

On Thu, Aug 12, 2010 at 5:53 AM, Chongxin Li <li...@zju.edu.cn> wrote:

>
>
> > On 2010-08-12 02:53:06, Ted Yu wrote:
> > > src/main/java/org/apache/hadoop/hbase/master/SnapshotTracker.java, line
> 1
> > > <http://review.cloudera.org/r/467/diff/3/?file=6021#file6021line1>
> > >
> > >     How about SnapshotWatcher ?
>
> Will it sound like this class implement the Watcher interface of ZK?
>
>
> > On 2010-08-12 02:53:06, Ted Yu wrote:
> > > src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.java, line
> 283
> > > <http://review.cloudera.org/r/467/diff/3/?file=6028#file6028line283>
> > >
> > >     Can we get to hbase root directly ?
>
> Since this method is static, we probably need another parameter for root
> directory?
>
>
> - Chongxin
>
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://review.cloudera.org/r/467/#review869
> -----------------------------------------------------------
>
>
> On 2010-08-12 02:43:42, Chongxin Li wrote:
> >
> > -----------------------------------------------------------
> > This is an automatically generated e-mail. To reply, visit:
> > http://review.cloudera.org/r/467/
> > -----------------------------------------------------------
> >
> > (Updated 2010-08-12 02:43:42)
> >
> >
> > 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/HTableDescriptor.java 0d57270
> >   src/main/java/org/apache/hadoop/hbase/SnapshotDescriptor.java
> PRE-CREATION
> >   src/main/java/org/apache/hadoop/hbase/SnapshotExistsException.java
> PRE-CREATION
> >   src/main/java/org/apache/hadoop/hbase/TablePartiallyOpenException.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/Snapshotter.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/TestLogsCleaner.java
> 8b7f60f
> >   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
> >
> >
>
>

Re: Review Request: HBASE-50: Snapshot of table

Posted by Chongxin Li <li...@zju.edu.cn>.

> On 2010-08-12 02:53:06, Ted Yu wrote:
> > src/main/java/org/apache/hadoop/hbase/master/SnapshotTracker.java, line 1
> > <http://review.cloudera.org/r/467/diff/3/?file=6021#file6021line1>
> >
> >     How about SnapshotWatcher ?

Will it sound like this class implement the Watcher interface of ZK?


> On 2010-08-12 02:53:06, Ted Yu wrote:
> > src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.java, line 283
> > <http://review.cloudera.org/r/467/diff/3/?file=6028#file6028line283>
> >
> >     Can we get to hbase root directly ?

Since this method is static, we probably need another parameter for root directory?


- Chongxin


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


On 2010-08-12 02:43:42, Chongxin Li wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://review.cloudera.org/r/467/
> -----------------------------------------------------------
> 
> (Updated 2010-08-12 02:43:42)
> 
> 
> 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/HTableDescriptor.java 0d57270 
>   src/main/java/org/apache/hadoop/hbase/SnapshotDescriptor.java PRE-CREATION 
>   src/main/java/org/apache/hadoop/hbase/SnapshotExistsException.java PRE-CREATION 
>   src/main/java/org/apache/hadoop/hbase/TablePartiallyOpenException.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/Snapshotter.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/TestLogsCleaner.java 8b7f60f 
>   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
> 
>


Re: Review Request: HBASE-50: Snapshot of table

Posted by Ted Yu <te...@yahoo.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://review.cloudera.org/r/467/#review869
-----------------------------------------------------------



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

    How about SnapshotWatcher ?



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

    I think putting this in Region is good.



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

    Can we get to hbase root directly ?


- Ted


On 2010-08-12 02:43:42, Chongxin Li wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://review.cloudera.org/r/467/
> -----------------------------------------------------------
> 
> (Updated 2010-08-12 02:43:42)
> 
> 
> 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/HTableDescriptor.java 0d57270 
>   src/main/java/org/apache/hadoop/hbase/SnapshotDescriptor.java PRE-CREATION 
>   src/main/java/org/apache/hadoop/hbase/SnapshotExistsException.java PRE-CREATION 
>   src/main/java/org/apache/hadoop/hbase/TablePartiallyOpenException.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/Snapshotter.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/TestLogsCleaner.java 8b7f60f 
>   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
> 
>


Re: Review Request: HBASE-50: Snapshot of table

Posted by Chongxin Li <li...@zju.edu.cn>.

> On 2010-08-12 10:33:25, Ted Yu wrote:
> > src/main/java/org/apache/hadoop/hbase/master/DeleteSnapshot.java, line 98
> > <http://review.cloudera.org/r/467/diff/4/?file=6589#file6589line98>
> >
> >     Is there more to be done here ?

Deleting the region dir?


> On 2010-08-12 10:33:25, Ted Yu wrote:
> > src/main/java/org/apache/hadoop/hbase/master/DeleteSnapshot.java, line 94
> > <http://review.cloudera.org/r/467/diff/4/?file=6589#file6589line94>
> >
> >     Should return value be checked ?

Deleting the snapshot directory at last would delete all snapshot files anyway. Do we still have to check the return value? What if the return value if false, just log it?


- Chongxin


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


On 2010-08-12 02:43:42, Chongxin Li wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://review.cloudera.org/r/467/
> -----------------------------------------------------------
> 
> (Updated 2010-08-12 02:43:42)
> 
> 
> 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/HTableDescriptor.java 0d57270 
>   src/main/java/org/apache/hadoop/hbase/SnapshotDescriptor.java PRE-CREATION 
>   src/main/java/org/apache/hadoop/hbase/SnapshotExistsException.java PRE-CREATION 
>   src/main/java/org/apache/hadoop/hbase/TablePartiallyOpenException.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/Snapshotter.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/TestLogsCleaner.java 8b7f60f 
>   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
> 
>


Re: Review Request: HBASE-50: Snapshot of table

Posted by Ted Yu <te...@yahoo.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://review.cloudera.org/r/467/#review874
-----------------------------------------------------------



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

    Check return value.



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

    Should return value be checked ?



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

    Is there more to be done here ?


- Ted


On 2010-08-12 02:43:42, Chongxin Li wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://review.cloudera.org/r/467/
> -----------------------------------------------------------
> 
> (Updated 2010-08-12 02:43:42)
> 
> 
> 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/HTableDescriptor.java 0d57270 
>   src/main/java/org/apache/hadoop/hbase/SnapshotDescriptor.java PRE-CREATION 
>   src/main/java/org/apache/hadoop/hbase/SnapshotExistsException.java PRE-CREATION 
>   src/main/java/org/apache/hadoop/hbase/TablePartiallyOpenException.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/Snapshotter.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/TestLogsCleaner.java 8b7f60f 
>   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
> 
>


Re: Review Request: HBASE-50: Snapshot of table

Posted by Ted Yu <te...@yahoo.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://review.cloudera.org/r/467/#review927
-----------------------------------------------------------



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

    Write a script that calls this method.


- Ted


On 2010-08-14 01:30:00, Chongxin Li wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://review.cloudera.org/r/467/
> -----------------------------------------------------------
> 
> (Updated 2010-08-14 01:30:00)
> 
> 
> 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/HTableDescriptor.java 0d57270 
>   src/main/java/org/apache/hadoop/hbase/SnapshotDescriptor.java PRE-CREATION 
>   src/main/java/org/apache/hadoop/hbase/SnapshotExistsException.java PRE-CREATION 
>   src/main/java/org/apache/hadoop/hbase/TablePartiallyOpenException.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/Snapshotter.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/util/MetaUtils.java 4481b12 
>   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/TestLogsCleaner.java 8b7f60f 
>   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
> 
>


Re: Review Request: HBASE-50: Snapshot of table

Posted by Chongxin Li <li...@zju.edu.cn>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://review.cloudera.org/r/467/#review904
-----------------------------------------------------------



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

    Why set retries number to 1 here?
    Since I use MetaUtils in HMaster to scan root region when system is started, this would have a impact for other parts of the system. Is it OK to remove this?


- Chongxin


On 2010-08-14 01:30:00, Chongxin Li wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://review.cloudera.org/r/467/
> -----------------------------------------------------------
> 
> (Updated 2010-08-14 01:30:00)
> 
> 
> 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/HTableDescriptor.java 0d57270 
>   src/main/java/org/apache/hadoop/hbase/SnapshotDescriptor.java PRE-CREATION 
>   src/main/java/org/apache/hadoop/hbase/SnapshotExistsException.java PRE-CREATION 
>   src/main/java/org/apache/hadoop/hbase/TablePartiallyOpenException.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/Snapshotter.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/util/MetaUtils.java 4481b12 
>   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/TestLogsCleaner.java 8b7f60f 
>   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
> 
>


Re: Review Request: HBASE-50: Snapshot of table

Posted by Ted Yu <te...@yahoo.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://review.cloudera.org/r/467/#review995
-----------------------------------------------------------



bin/add_snapshot_family.rb
<http://review.cloudera.org/r/467/#comment3204>

    Please remove this comment.



src/main/java/org/apache/hadoop/hbase/HConstants.java
<http://review.cloudera.org/r/467/#comment3203>

    Should the archive directory be named ".archive" ?


- Ted


On 2010-08-19 08:35:37, Chongxin Li wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://review.cloudera.org/r/467/
> -----------------------------------------------------------
> 
> (Updated 2010-08-19 08:35:37)
> 
> 
> 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
> -----
> 
>   bin/add_snapshot_family.rb PRE-CREATION 
>   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/HTableDescriptor.java 0d57270 
>   src/main/java/org/apache/hadoop/hbase/SnapshotDescriptor.java PRE-CREATION 
>   src/main/java/org/apache/hadoop/hbase/SnapshotExistsException.java PRE-CREATION 
>   src/main/java/org/apache/hadoop/hbase/TablePartiallyOpenException.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/SnapshotSentinel.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/Snapshotter.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/TestLogsCleaner.java 8b7f60f 
>   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
> 
>


Re: Review Request: HBASE-50: Snapshot of table

Posted by Chongxin Li <li...@zju.edu.cn>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://review.cloudera.org/r/467/
-----------------------------------------------------------

(Updated 2010-09-06 04:34:53.459404)


Review request for hbase.


Changes
-------

Add Mapreduce based export (ExportSnapshot) and import (ImportSnapshot) for snapshot, so that snapshot of an hbase table could be exported and imported to other data centers. Unit test (TestSnapshotExport) has passed.


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 (updated)
-----

  bin/add_snapshot_family.rb PRE-CREATION 
  src/main/java/org/apache/hadoop/hbase/HConstants.java bfaa4a1 
  src/main/java/org/apache/hadoop/hbase/HRegionInfo.java ee94690 
  src/main/java/org/apache/hadoop/hbase/HTableDescriptor.java 0d57270 
  src/main/java/org/apache/hadoop/hbase/SnapshotDescriptor.java PRE-CREATION 
  src/main/java/org/apache/hadoop/hbase/SnapshotExistsException.java PRE-CREATION 
  src/main/java/org/apache/hadoop/hbase/TablePartiallyOpenException.java PRE-CREATION 
  src/main/java/org/apache/hadoop/hbase/client/HBaseAdmin.java 8b01aa0 
  src/main/java/org/apache/hadoop/hbase/io/HalfStoreFileReader.java d35a28a 
  src/main/java/org/apache/hadoop/hbase/io/HbaseObjectWritable.java 20860d6 
  src/main/java/org/apache/hadoop/hbase/io/Reference.java 219203c 
  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/ExportSnapshot.java PRE-CREATION 
  src/main/java/org/apache/hadoop/hbase/mapreduce/ImportSnapshot.java PRE-CREATION 
  src/main/java/org/apache/hadoop/hbase/mapreduce/LoadIncrementalHFiles.java 1183584 
  src/main/java/org/apache/hadoop/hbase/master/BaseScanner.java 2deea4a 
  src/main/java/org/apache/hadoop/hbase/master/DeleteSnapshot.java PRE-CREATION 
  src/main/java/org/apache/hadoop/hbase/master/HMaster.java 4735304 
  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/SnapshotSentinel.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 9fdd86d 
  src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 8356d64 
  src/main/java/org/apache/hadoop/hbase/regionserver/Snapshotter.java PRE-CREATION 
  src/main/java/org/apache/hadoop/hbase/regionserver/SplitTransaction.java f1d52b7 
  src/main/java/org/apache/hadoop/hbase/regionserver/Store.java ae9e190 
  src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.java 107d641 
  src/main/java/org/apache/hadoop/hbase/regionserver/ZKSnapshotWatcher.java PRE-CREATION 
  src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java 20a535c 
  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 3256ac9 
  src/main/resources/hbase-default.xml 419bc6d 
  src/test/java/org/apache/hadoop/hbase/HBaseTestingUtility.java fadee21 
  src/test/java/org/apache/hadoop/hbase/client/TestAdmin.java c9b78b9 
  src/test/java/org/apache/hadoop/hbase/mapreduce/TestSnapshotExport.java PRE-CREATION 
  src/test/java/org/apache/hadoop/hbase/master/TestLogsCleaner.java 8b7f60f 
  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 c425953 
  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


Re: Review Request: HBASE-50: Snapshot of table

Posted by Ted Yu <yu...@gmail.com>.
Chongxin:
2. Write a script (add_snapshot_family.rb) to add snapshot family for META
and remove method HMaster.addSnapshotFamily. The script is not tested yet
(how?)

Can user use this script on previous HBase release, then upgrade to new
release which has snapshot feature ?

Re: Review Request: HBASE-50: Snapshot of table

Posted by Chongxin Li <li...@zju.edu.cn>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://review.cloudera.org/r/467/#review957
-----------------------------------------------------------


1. Rename SnapshotTracker to SnapshotSentinel
2. Write a script (add_snapshot_family.rb) to add snapshot family for META and remove method HMaster.addSnapshotFamily. The script is not tested yet (how?)

- Chongxin


On 2010-08-19 08:35:37, Chongxin Li wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://review.cloudera.org/r/467/
> -----------------------------------------------------------
> 
> (Updated 2010-08-19 08:35:37)
> 
> 
> 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
> -----
> 
>   bin/add_snapshot_family.rb PRE-CREATION 
>   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/HTableDescriptor.java 0d57270 
>   src/main/java/org/apache/hadoop/hbase/SnapshotDescriptor.java PRE-CREATION 
>   src/main/java/org/apache/hadoop/hbase/SnapshotExistsException.java PRE-CREATION 
>   src/main/java/org/apache/hadoop/hbase/TablePartiallyOpenException.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/SnapshotSentinel.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/Snapshotter.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/TestLogsCleaner.java 8b7f60f 
>   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
> 
>


Re: Review Request: HBASE-50: Snapshot of table

Posted by Chongxin Li <li...@zju.edu.cn>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://review.cloudera.org/r/467/
-----------------------------------------------------------

(Updated 2010-08-19 08:35:37.043957)


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 (updated)
-----

  bin/add_snapshot_family.rb PRE-CREATION 
  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/HTableDescriptor.java 0d57270 
  src/main/java/org/apache/hadoop/hbase/SnapshotDescriptor.java PRE-CREATION 
  src/main/java/org/apache/hadoop/hbase/SnapshotExistsException.java PRE-CREATION 
  src/main/java/org/apache/hadoop/hbase/TablePartiallyOpenException.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/SnapshotSentinel.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/Snapshotter.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/TestLogsCleaner.java 8b7f60f 
  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


Re: Review Request: HBASE-50: Snapshot of table

Posted by Chongxin Li <li...@zju.edu.cn>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://review.cloudera.org/r/467/
-----------------------------------------------------------

(Updated 2010-08-14 01:30:00.941083)


Review request for hbase.


Changes
-------

1. Update HMaster to check and add the snapshot family for META (with MetaUtils) if it does not exist when the cluster is started. This is used for the data which has been migrated from an older hbase version.
2. Check the return value of delete


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 (updated)
-----

  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/HTableDescriptor.java 0d57270 
  src/main/java/org/apache/hadoop/hbase/SnapshotDescriptor.java PRE-CREATION 
  src/main/java/org/apache/hadoop/hbase/SnapshotExistsException.java PRE-CREATION 
  src/main/java/org/apache/hadoop/hbase/TablePartiallyOpenException.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/Snapshotter.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/util/MetaUtils.java 4481b12 
  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/TestLogsCleaner.java 8b7f60f 
  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


Re: Review Request: HBASE-50: Snapshot of table

Posted by Chongxin Li <li...@zju.edu.cn>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://review.cloudera.org/r/467/
-----------------------------------------------------------

(Updated 2010-08-12 02:43:42.872855)


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 (updated)
-----

  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/HTableDescriptor.java 0d57270 
  src/main/java/org/apache/hadoop/hbase/SnapshotDescriptor.java PRE-CREATION 
  src/main/java/org/apache/hadoop/hbase/SnapshotExistsException.java PRE-CREATION 
  src/main/java/org/apache/hadoop/hbase/TablePartiallyOpenException.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/Snapshotter.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/TestLogsCleaner.java 8b7f60f 
  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


Re: Review Request: HBASE-50: Snapshot of table

Posted by Chongxin Li <li...@zju.edu.cn>.

> On 2010-08-10 22:40:31, Ted Yu wrote:
> > src/main/java/org/apache/hadoop/hbase/master/HMaster.java, line 962
> > <http://review.cloudera.org/r/467/diff/3/?file=6015#file6015line962>
> >
> >     Moving crashed snapshots has two benefits:
> >     1. future call to listSnapshots() wouldn't encounter IOException.
> >     2. it's easy for user to get statistics on failed snapshots and analyze them
> >     
> >     Or, if you log enough information when cleaning up the failed snapshot.
> >

What about snapshot fails when it is being created? Currently it is cleaned up if exception occurs in HMaster.snapshot. Should we also move it to this directory? Then for reference information sync, should we also take the reference files of these failed snapshots into account?


- Chongxin


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


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


Re: Review Request: HBASE-50: Snapshot of table

Posted by Ted Yu <te...@yahoo.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://review.cloudera.org/r/467/#review830
-----------------------------------------------------------



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

    Moving crashed snapshots has two benefits:
    1. future call to listSnapshots() wouldn't encounter IOException.
    2. it's easy for user to get statistics on failed snapshots and analyze them
    
    Or, if you log enough information when cleaning up the failed snapshot.
    


- Ted


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


Re: Review Request: HBASE-50: Snapshot of table

Posted by Chongxin Li <li...@zju.edu.cn>.

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


Re: Review Request: HBASE-50: Snapshot of table

Posted by st...@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


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


Re: Review Request: HBASE-50: Snapshot of table

Posted by Chongxin Li <li...@zju.edu.cn>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://review.cloudera.org/r/467/
-----------------------------------------------------------

(Updated 2010-08-09 03:52:11.875655)


Review request for hbase.


Changes
-------

Quite a lot of changes have been made according Todd's review, here are some major ones:

1. Refactor SnapshotMonitor into one part that is master-global and another part that is created once per-snapshot (SnapshotTracker). 
2. Catch exceptions in HMaster.snapshot and clean up the snapshot if exceptions occur.
3. Always quit snapshot mode for regions no matter whether the snapshot is created successfully on RS.
4. Add a mechanism to check and synchronize the reference count in META with the number of reference files in BaseScanner.
5. Add snapshot operations: DeleteSnapshot, RestoreSnapshot and corresponding tests (in TestAdmin).


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 (updated)
-----

  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


Re: Review Request: HBASE-50: Snapshot of table

Posted by Chongxin Li <li...@zju.edu.cn>.

> On 2010-08-02 13:41:35, Todd Lipcon wrote:
> > src/main/java/org/apache/hadoop/hbase/master/SnapshotLogCleaner.java, line 22
> > <http://review.cloudera.org/r/467/diff/2/?file=4140#file4140line22>
> >
> >     worth noting that this class is not thread-safe? I don't know if these classes need to be thread safe, but you're using an unsynchronized hashset. Also, since refreshHLogsAndSearch clears hlogs before re-adding stuff, it needs to be synchronized more than just using a synchronized collection.

This class is only instantiated once by LogsCleaner so it can be seen as a singleton per master.


> On 2010-08-02 13:41:35, Todd Lipcon wrote:
> > src/main/java/org/apache/hadoop/hbase/master/SnapshotMonitor.java, line 116
> > <http://review.cloudera.org/r/467/diff/2/?file=4141#file4141line116>
> >
> >     does ZKW automatically re-watch the nodes for you, here?
> >     
> >     Also, how does this interact with region server failure? We just assume that the snapshot will timeout and abort, instead of proactively detecting?

Yes, the ZKW automatically re-watch the nodes.

For snapshot abort, if any region server fails to perform the snapshot, it will remove corresponding ready and finished nodes under snapshot directory. This would notify the master snapshot failure and further abort snapshot on all region servers via ZK

For snapshot timeout, it is not detected here. In method waitToFinish, the snapshot status is checked at a regular time (3 seconds here). If this method timeout, exception would be thrown and master will abort the snapshot over the cluster.


> On 2010-08-02 13:41:35, Todd Lipcon wrote:
> > src/main/java/org/apache/hadoop/hbase/master/TableSnapshot.java, line 132
> > <http://review.cloudera.org/r/467/diff/2/?file=4143#file4143line132>
> >
> >     is there a process that scans for cases where the reference count has gotten out of sync?
> >     I'm worried about a case where a snapshot is half-done, and then it fails, so the snapshot is considered aborted, but we never clean up the references because META has been incremented.

This is added in META scanner. Since scanning reference files is expensive, only a few regions are checked and synchronized in one scan. A last checking time is added so that all reference regions are guaranteed to be checked eventually


> On 2010-08-02 13:41:35, Todd Lipcon wrote:
> > src/main/java/org/apache/hadoop/hbase/zookeeper/ZooKeeperWrapper.java, line 1403
> > <http://review.cloudera.org/r/467/diff/2/?file=4153#file4153line1403>
> >
> >     these checks are inherently racy

Then remove it?


> On 2010-08-02 13:41:35, Todd Lipcon wrote:
> > src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.java, line 585
> > <http://review.cloudera.org/r/467/diff/2/?file=4148#file4148line585>
> >
> >     this seems prone to collision if it's multithreaded, since the exists check and the use of the filename aren't atomic

Then how to guarantee atomicity? This unique file name should be unique respecting existing files and files which are already compacted and deleted. Otherwise there might be a name collision in archive directory for HFiles


> On 2010-08-02 13:41:35, Todd Lipcon wrote:
> > src/main/java/org/apache/hadoop/hbase/HSnapshotDescriptor.java, line 132
> > <http://review.cloudera.org/r/467/diff/2/?file=4130#file4130line132>
> >
> >     since we're using the snapshot name as a directory name in HDFS, it has to be a UTF8 string, so why not just keep it as a String above too?

I implemented this class following HTableDescriptor. And even for table name, it is usually used as a byte array instead of String


> On 2010-08-02 13:41:35, Todd Lipcon wrote:
> > src/main/java/org/apache/hadoop/hbase/io/Reference.java, line 61
> > <http://review.cloudera.org/r/467/diff/2/?file=4134#file4134line61>
> >
> >     to keep compatibility with current storefiles, "entire" should be value 2, and bottom should be 0
> >     
> >     while we're at it, maybe rename these to be all caps - Range.TOP, Range.BOTTOM, etc

Have been renamed in the latest revision


> On 2010-08-02 13:41:35, Todd Lipcon wrote:
> > src/main/java/org/apache/hadoop/hbase/master/HMaster.java, lines 918-919
> > <http://review.cloudera.org/r/467/diff/2/?file=4138#file4138line918>
> >
> >     should this be an exception, rather than a result code? ie is it normal to fail?

Currently all results except ALl_FINISH would throw an exception.


> On 2010-08-02 13:41:35, Todd Lipcon wrote:
> > src/main/java/org/apache/hadoop/hbase/master/HMaster.java, line 925
> > <http://review.cloudera.org/r/467/diff/2/?file=4138#file4138line925>
> >
> >     do we have a race here? what if the table gets enabled while the snapshot is being processed? it seems we need some locking here around table status and snapshot modification

Why do we have a race here? I think we can't call HMaster.enableTable during the execution of this method, right? TableDelete is implemented in the same way, the table would not get enabled when it is being deleted, isn't it?


> On 2010-08-02 13:41:35, Todd Lipcon wrote:
> > src/main/java/org/apache/hadoop/hbase/master/SnapshotLogCleaner.java, line 47
> > <http://review.cloudera.org/r/467/diff/2/?file=4140#file4140line47>
> >
> >     do we have a race here between the listStatus and creating a snapshot?

I've done this modification in the latest revision. SnapshotLogCleaner's cache is refreshed after listStatus of LogsCleaner and LogCleaner only cleans the archived logs, creating a new snapshot would not use the archived logs.


> On 2010-08-02 13:41:35, Todd Lipcon wrote:
> > src/main/java/org/apache/hadoop/hbase/master/SnapshotLogCleaner.java, line 115
> > <http://review.cloudera.org/r/467/diff/2/?file=4140#file4140line115>
> >
> >     do we really want to swallow this IOE?

This implements a method from the interface. The method declaration does not throw any exceptions.


> On 2010-08-02 13:41:35, Todd Lipcon wrote:
> > src/main/java/org/apache/hadoop/hbase/master/SnapshotMonitor.java, line 40
> > <http://review.cloudera.org/r/467/diff/2/?file=4141#file4141line40>
> >
> >     this is basically a singleton per-master, right? not per-snapshot? ie this is created at master startup and destroyed at master shutdown, and can coordinate multiple snapshots over its lifetime? would be good to document how this fits into the overall design, and perhaps refactor into one part that is master-global and another part that is created once per-snapshot.

This class has been refactored into two parts


> On 2010-08-02 13:41:35, Todd Lipcon wrote:
> > src/main/java/org/apache/hadoop/hbase/master/SnapshotMonitor.java, line 169
> > <http://review.cloudera.org/r/467/diff/2/?file=4141#file4141line169>
> >
> >     assert that we're in ALLREADY state here?

right


- Chongxin


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


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


Re: Review Request: HBASE-50: Snapshot of table

Posted by Todd Lipcon <to...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://review.cloudera.org/r/467/#review618
-----------------------------------------------------------


looks pretty good! I didn't et a chance to look through the test cases in detail, I'll try to look them over some more later this week.


src/main/java/org/apache/hadoop/hbase/HConstants.java
<http://review.cloudera.org/r/467/#comment2293>

    since we also have a "log archive dir" somewhere, should specify this a little more - this is archived HFiles that are still referenced by snapshots?



src/main/java/org/apache/hadoop/hbase/HSnapshotDescriptor.java
<http://review.cloudera.org/r/467/#comment2294>

    license



src/main/java/org/apache/hadoop/hbase/HSnapshotDescriptor.java
<http://review.cloudera.org/r/467/#comment2295>

    no need for @param javadoc if there is no actual description attached. same thing below in a few places



src/main/java/org/apache/hadoop/hbase/HSnapshotDescriptor.java
<http://review.cloudera.org/r/467/#comment2296>

    why not System.currentTimeMillis?



src/main/java/org/apache/hadoop/hbase/HSnapshotDescriptor.java
<http://review.cloudera.org/r/467/#comment2297>

    empty @return



src/main/java/org/apache/hadoop/hbase/HSnapshotDescriptor.java
<http://review.cloudera.org/r/467/#comment2298>

    since we're using the snapshot name as a directory name in HDFS, it has to be a UTF8 string, so why not just keep it as a String above too?



src/main/java/org/apache/hadoop/hbase/TablePartialOpenException.java
<http://review.cloudera.org/r/467/#comment2299>

    no need for this javadoc (it's obvious)



src/main/java/org/apache/hadoop/hbase/TablePartialOpenException.java
<http://review.cloudera.org/r/467/#comment2300>

    same with this one



src/main/java/org/apache/hadoop/hbase/client/HBaseAdmin.java
<http://review.cloudera.org/r/467/#comment2301>

    add "TODO" to this comment



src/main/java/org/apache/hadoop/hbase/io/Reference.java
<http://review.cloudera.org/r/467/#comment2302>

    to keep compatibility with current storefiles, "entire" should be value 2, and bottom should be 0
    
    while we're at it, maybe rename these to be all caps - Range.TOP, Range.BOTTOM, etc



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

    no need to check size() - iterating the empty array should be fine



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

    if we crash between step 1 and 2, we orphan the archived file. Instead, we can do the delete first (ignoring failure if it doesn't exist) and then update META.



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

    you can just call mkdirs, I think, and it won't fail if it already exists



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

    should this be an exception, rather than a result code? ie is it normal to fail?



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

    do we have a race here? what if the table gets enabled while the snapshot is being processed? it seems we need some locking here around table status and snapshot modification



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

    shouldn't we rethrow in this error case? and in the above error case? ie these should be clauses like:
    
    boolean success=false;
    try {
      ... make snapshot ...
      success = true;
    } finally {
      if (!success) {
        deleteSnapshot();
      }
    }
    



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

    would it be problematic to create a partially written snapshotinfo file? or would it get cleaned up at a higher layer?
    
    (perhaps worth creating snapshotinfo.tmp, then atomically rename it to snapshotinfo if it writes correctly)



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

    license



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

    worth noting that this class is not thread-safe? I don't know if these classes need to be thread safe, but you're using an unsynchronized hashset. Also, since refreshHLogsAndSearch clears hlogs before re-adding stuff, it needs to be synchronized more than just using a synchronized collection.



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

    do we have a race here between the listStatus and creating a snapshot?



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

    document that it may be null, and what null means? in fact, do we ever call this with null? it doesn't look like it.



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

    do we really want to swallow this IOE?



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

    this is basically a singleton per-master, right? not per-snapshot? ie this is created at master startup and destroyed at master shutdown, and can coordinate multiple snapshots over its lifetime? would be good to document how this fits into the overall design, and perhaps refactor into one part that is master-global and another part that is created once per-snapshot.



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

    needs to be volatile - waitForFinish accesses this outside of synchronization



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

    what is this mutex? better to name it based on what objects it synchronizes, and also use new Object() instead of new Integer(0)



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

    are you sure this is the way we name HMaster's ZKW instance? can we just grab the existing zkWrapper instance out of the master object?



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

    what's the synchronization story here? who calls this method?



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

    useless doc



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

    better to just do something like:
    
    if (!mkdirs(...)) {
      throw IOE("could not create")
    }



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

    include path or snapshot name in exception msg



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

    does ZKW automatically re-watch the nodes for you, here?
    
    Also, how does this interact with region server failure? We just assume that the snapshot will timeout and abort, instead of proactively detecting?



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

    add a LOG.debug perhaps for this case - it seems rare that we'd have the correct count of servers but be missing one



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

    assert that we're in ALLREADY state here?



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

    log at least?



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

    this should probably be rethrown?



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

    consider rename to M_ALL_RS_READY and M_ALL_RS_FINISHED for clarity
    
    also, what is M?



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

    rename to M_RS_READY and M_RS_FINISHED?
    Should these RS-specific ones be in a separate enum? GlobalSnapshotStatus vs RSSnapshotStatus?



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

    check return value of mkdirs instead



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

    info level instead, perhaps? seems like a common issue given we're very flaky about region enable status currently.



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

    is there a process that scans for cases where the reference count has gotten out of sync?
    I'm worried about a case where a snapshot is half-done, and then it fails, so the snapshot is considered aborted, but we never clean up the references because META has been incremented.



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

    update message to include snapshot case



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

    rather than returning booleans, it might be better to throw back an exception to indicate error - this way the snapshot coordinator can actually show the reason for the failed snapshot, instead of forcing the user to grep all of the RS logs.



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

    what happens if the snapshot coordinator dies before completing the snapshot? the region is left permanently in snapshot mode?



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

    useless doc



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

    check return value of mkdirs instead of separately calling exists



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

    see above - just call fs.mkdirs



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

    this code is duplicated from the master-driven snapshot - perhaps it can be factored out somewhere



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

    this code is also duplicated



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

    in this case, though, we've already called startSnapshot on some other regions - is this problematic?



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

    missing " " before "on RS"



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

    at this point don't we have to wait for the snapshot coordinator to "commit" the snapshot?



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

    !regionsToBackup.isEmpty()



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

    "on RS" -> " on RS" (space)



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

    perhaps write to a tmp file then move into place, so it's atomic



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

    this seems prone to collision if it's multithreaded, since the exists check and the use of the filename aren't atomic



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

    since createZNodeIfNotExists already does existance check, you don't need the .exists above



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

    does ZKWatcher automatically re-watch for you?



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

    should actually wait for the snapshotThread to exit here - otherwise maybe an aborting one is still doing some work, which might overlap with the next one



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

    more examples of checking mkdirs



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

    useless javadoc



src/main/java/org/apache/hadoop/hbase/zookeeper/ZooKeeperWrapper.java
<http://review.cloudera.org/r/467/#comment2357>

    throw exceptions instead of returning false?



src/main/java/org/apache/hadoop/hbase/zookeeper/ZooKeeperWrapper.java
<http://review.cloudera.org/r/467/#comment2358>

    throw exceptions instead of returning false



src/main/java/org/apache/hadoop/hbase/zookeeper/ZooKeeperWrapper.java
<http://review.cloudera.org/r/467/#comment2359>

    throw exceptions to user



src/main/java/org/apache/hadoop/hbase/zookeeper/ZooKeeperWrapper.java
<http://review.cloudera.org/r/467/#comment2360>

    these checks are inherently racy



src/test/java/org/apache/hadoop/hbase/master/TestSnapshot.java
<http://review.cloudera.org/r/467/#comment2361>

    license



src/test/java/org/apache/hadoop/hbase/master/TestSnapshotFailure.java
<http://review.cloudera.org/r/467/#comment2362>

    license



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

    license


- Todd


On 2010-08-01 03:51:43, Chongxin Li wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://review.cloudera.org/r/467/
> -----------------------------------------------------------
> 
> (Updated 2010-08-01 03:51:43)
> 
> 
> 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/TablePartialOpenException.java PRE-CREATION 
>   src/main/java/org/apache/hadoop/hbase/client/HBaseAdmin.java 8b01aa0 
>   src/main/java/org/apache/hadoop/hbase/io/Reference.java 219203c 
>   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/master/BaseScanner.java 69eab39 
>   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/SnapshotLogCleaner.java PRE-CREATION 
>   src/main/java/org/apache/hadoop/hbase/master/SnapshotMonitor.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/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/TestZKSnapshotWatcher.java PRE-CREATION 
> 
> Diff: http://review.cloudera.org/r/467/diff
> 
> 
> Testing
> -------
> 
> Unit tests and integration tests with mini cluster passed.
> 
> 
> Thanks,
> 
> Chongxin
> 
>


Re: Review Request: HBASE-50: Snapshot of table

Posted by Chongxin Li <li...@zju.edu.cn>.

> On 2010-08-03 09:58:06, Ted Yu wrote:
> > src/main/java/org/apache/hadoop/hbase/master/SnapshotMonitor.java, line 246
> > <http://review.cloudera.org/r/467/diff/2/?file=4141#file4141line246>
> >
> >     I think this should be (retries == 4) for 3 retries

this is actually not 'retry' for snapshot, but check whether the snapshot is finished for three times (retries = 0, 1, 2).


- Chongxin


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


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


Re: Review Request: HBASE-50: Snapshot of table

Posted by Ted Yu <te...@yahoo.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://review.cloudera.org/r/467/#review631
-----------------------------------------------------------



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

    I think this should be (retries == 4) for 3 retries


- Ted


On 2010-08-01 03:51:43, Chongxin Li wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://review.cloudera.org/r/467/
> -----------------------------------------------------------
> 
> (Updated 2010-08-01 03:51:43)
> 
> 
> 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/TablePartialOpenException.java PRE-CREATION 
>   src/main/java/org/apache/hadoop/hbase/client/HBaseAdmin.java 8b01aa0 
>   src/main/java/org/apache/hadoop/hbase/io/Reference.java 219203c 
>   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/master/BaseScanner.java 69eab39 
>   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/SnapshotLogCleaner.java PRE-CREATION 
>   src/main/java/org/apache/hadoop/hbase/master/SnapshotMonitor.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/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/TestZKSnapshotWatcher.java PRE-CREATION 
> 
> Diff: http://review.cloudera.org/r/467/diff
> 
> 
> Testing
> -------
> 
> Unit tests and integration tests with mini cluster passed.
> 
> 
> Thanks,
> 
> Chongxin
> 
>