Details

    • Hadoop Flags:
      Reviewed
    • Release Note:
      Convert Reference, .tableinfo, .regioninfo, hbase.version, and hbase.id files to have pb content.
    1. 5453v13.txt
      321 kB
      stack
    2. 5453v12.txt
      321 kB
      stack
    3. 5453v11.txt
      313 kB
      stack
    4. 5453v11.txt
      313 kB
      stack
    5. 5453v10.txt
      316 kB
      stack
    6. 5453v9.txt
      313 kB
      stack
    7. 5453v6.txt
      311 kB
      stack
    8. 5453v3.txt
      311 kB
      stack
    9. 5453v2.txt
      31 kB
      stack
    10. 5453.txt
      12 kB
      stack

      Activity

      Hide
      stack added a comment -

      Files to move over:

      /${HBASE_ROOTDIR}/hbase.version
      /${HBASE_ROOTDIR}/hbase.id
      /${HBASE_ROOTDIR}/TABLENAME/.tableinfo
      /${HBASE_ROOTDIR}/TABLENAME/REGIONNAME/.regioninfo
      /${HBASE_ROOTDIR}/TABLENAME/REGIONNAME/COLUMN_FAMILY/reference
      

      Let me see too where else we write; e.g. into hfiles. Is it necessary bringing these over? They are versioned already w/ readers than can do v1, v2, etc.

      Let me see if I can do it in a portable way.

      Show
      stack added a comment - Files to move over: /${HBASE_ROOTDIR}/hbase.version /${HBASE_ROOTDIR}/hbase.id /${HBASE_ROOTDIR}/TABLENAME/.tableinfo /${HBASE_ROOTDIR}/TABLENAME/REGIONNAME/.regioninfo /${HBASE_ROOTDIR}/TABLENAME/REGIONNAME/COLUMN_FAMILY/reference Let me see too where else we write; e.g. into hfiles. Is it necessary bringing these over? They are versioned already w/ readers than can do v1, v2, etc. Let me see if I can do it in a portable way.
      Hide
      Elliott Clark added a comment -

      Are you talking about KeyValue as well ?

      Show
      Elliott Clark added a comment - Are you talking about KeyValue as well ?
      Hide
      stack added a comment -

      no to kv

      Show
      stack added a comment - no to kv
      Hide
      stack added a comment -

      Move over the hbase.version file.

      Show
      stack added a comment - Move over the hbase.version file.
      Hide
      Jimmy Xiang added a comment -

      Good stuff. Are the proto file and the generated java file in a different patch?

      Show
      Jimmy Xiang added a comment - Good stuff. Are the proto file and the generated java file in a different patch?
      Hide
      stack added a comment -

      Ooops. Thanks Jimmy. You think I should change over the hfile metadata to pb too? I suppose its MapWritable currently so should go over if we want to purge Writables...

      Show
      stack added a comment - Ooops. Thanks Jimmy. You think I should change over the hfile metadata to pb too? I suppose its MapWritable currently so should go over if we want to purge Writables...
      Hide
      Jimmy Xiang added a comment -

      That would be great, I think. Thanks.

      Show
      Jimmy Xiang added a comment - That would be great, I think. Thanks.
      Hide
      Hadoop QA added a comment -

      -1 overall. Here are the results of testing the latest attachment
      http://issues.apache.org/jira/secure/attachment/12526378/5453v2.txt
      against trunk revision .

      +1 @author. The patch does not contain any @author tags.

      +1 tests included. The patch appears to include 3 new or modified tests.

      +1 hadoop23. The patch compiles against the hadoop 0.23.x profile.

      +1 javadoc. The javadoc tool did not generate any warning messages.

      +1 javac. The applied patch does not increase the total number of javac compiler warnings.

      +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

      +1 release audit. The applied patch does not increase the total number of release audit warnings.

      -1 core tests. The patch failed these unit tests:
      org.apache.hadoop.hbase.regionserver.TestColumnSeeking

      Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/1834//testReport/
      Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/1834//artifact/trunk/patchprocess/newPatchFindbugsWarnings.html
      Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/1834//console

      This message is automatically generated.

      Show
      Hadoop QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12526378/5453v2.txt against trunk revision . +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 3 new or modified tests. +1 hadoop23. The patch compiles against the hadoop 0.23.x profile. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. -1 core tests. The patch failed these unit tests: org.apache.hadoop.hbase.regionserver.TestColumnSeeking Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/1834//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/1834//artifact/trunk/patchprocess/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/1834//console This message is automatically generated.
      Hide
      Todd Lipcon added a comment -

      Agree it would be great if HFile used protobuf where possible. But I don't think it needs to block 0.96, since we already have decent compatibility paths here, and the writables used are already fairly generic (mostly maps if I remember correctly).

      Show
      Todd Lipcon added a comment - Agree it would be great if HFile used protobuf where possible. But I don't think it needs to block 0.96, since we already have decent compatibility paths here, and the writables used are already fairly generic (mostly maps if I remember correctly).
      Hide
      Jimmy Xiang added a comment -

      v2 looks good to me.

      Show
      Jimmy Xiang added a comment - v2 looks good to me.
      Hide
      stack added a comment -

      Trying against hadoopqa.

      Moves over the five listed files to pb:

      .tableinfo, .regioninfo, reference, hbase.id and hbase.version

      Will post on pb w/ a better commit note now.

      Show
      stack added a comment - Trying against hadoopqa. Moves over the five listed files to pb: .tableinfo, .regioninfo, reference, hbase.id and hbase.version Will post on pb w/ a better commit note now.
      Hide
      Hadoop QA added a comment -

      -1 overall. Here are the results of testing the latest attachment
      http://issues.apache.org/jira/secure/attachment/12527528/5453v3.txt
      against trunk revision .

      +1 @author. The patch does not contain any @author tags.

      +1 tests included. The patch appears to include 24 new or modified tests.

      -1 patch. The patch command could not apply the patch.

      Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/1879//console

      This message is automatically generated.

      Show
      Hadoop QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12527528/5453v3.txt against trunk revision . +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 24 new or modified tests. -1 patch. The patch command could not apply the patch. Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/1879//console This message is automatically generated.
      Hide
      jiraposter@reviews.apache.org added a comment -

      -----------------------------------------------------------
      This is an automatically generated e-mail. To reply, visit:
      https://reviews.apache.org/r/5130/
      -----------------------------------------------------------

      Review request for hbase.

      Summary
      -------

      A b/src/main/java/org/apache/hadoop/hbase/ClusterId.java
      New class to hold clusterid in.
      M b/src/main/java/org/apache/hadoop/hbase/HColumnDescriptor.java
      Make it so can do pb serialization. Deprecated Writable serialization.
      M b/src/main/java/org/apache/hadoop/hbase/HRegionInfo.java
      Make it so methods in here follow the pattern in HCD an HTD pb 'ing.
      Deprecated Writable serialization.
      M b/src/main/java/org/apache/hadoop/hbase/HTableDescriptor.java
      Make it so can do pb serialization. Deprecated Writable serialization.
      M b/src/main/java/org/apache/hadoop/hbase/client/HConnectionManager.java
      ClusterId under ZK got renamed as ZKClusterId
      M b/src/main/java/org/apache/hadoop/hbase/io/Reference.java
      Hide the Reference#Range enums. Don't let them out of this class.
      Make it so can do pb serialization.
      M b/src/main/java/org/apache/hadoop/hbase/mapreduce/LoadIncrementalHFiles.java
      Use new methods on Reference for getting top and bottom.
      M b/src/main/java/org/apache/hadoop/hbase/master/HMaster.java
      ClusterId under zk has been renamed ZKClusterId.
      Use new ClusterId class too.
      M b/src/main/java/org/apache/hadoop/hbase/master/MasterFileSystem.java
      Use new clusterid class.
      M b/src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java
      Move the RegionInfo convertion up into HRegionInfo instead of here.
      Added generic toDelimitedByteArray helper.
      M b/src/main/java/org/apache/hadoop/hbase/protobuf/RequestConverter.java
      Use HRegionInfo convertions instead.
      M b/src/main/java/org/apache/hadoop/hbase/protobuf/ResponseConverter.java
      Use HRegionInfo convertions instead.
      M b/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
      Use new utility writing out .regioninfo files.
      M b/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java
      Formatting.
      M b/src/main/java/org/apache/hadoop/hbase/regionserver/SplitTransaction.java
      M b/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.java
      Range in Reference is no longer public.
      Range in Reference is no longer public.
      M b/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java
      M b/src/main/java/org/apache/hadoop/hbase/security/token/AuthenticationTokenSecretManager.java
      ClusterId got renamed ZKClusterId
      M b/src/main/java/org/apache/hadoop/hbase/util/FSTableDescriptors.java
      Use new serialization utlity in HTD.
      M b/src/main/java/org/apache/hadoop/hbase/util/FSUtils.java
      Generic method for writing dot file content.
      M b/src/main/java/org/apache/hadoop/hbase/util/HBaseFsck.java
      Reference#Range is not public any more
      M b/src/main/java/org/apache/hadoop/hbase/util/Writables.java
      Deprecated getHRegionInfo, etc.
      D b/src/main/java/org/apache/hadoop/hbase/zookeeper/ClusterId.java
      A b/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKClusterId.java
      Rename
      A b/src/main/protobuf/ClusterId.proto
      Added file for ClusterId only since its written to fs and to zk.
      A b/src/main/protobuf/FS.proto
      Protos for fs files.
      M b/src/main/protobuf/ZooKeeper.proto
      Moved ClusterId out to own proto file
      M b/src/main/protobuf/hbase.proto
      Added TableSchema and ColumnFamilySchema

      This addresses bug hbase-5453.
      https://issues.apache.org/jira/browse/hbase-5453

      Diffs


      src/main/java/org/apache/hadoop/hbase/ClusterId.java PRE-CREATION
      src/main/java/org/apache/hadoop/hbase/HColumnDescriptor.java 5862f15
      src/main/java/org/apache/hadoop/hbase/HRegionInfo.java 8d83ff3
      src/main/java/org/apache/hadoop/hbase/HTableDescriptor.java af89e3e
      src/main/java/org/apache/hadoop/hbase/client/HConnectionManager.java 5cac9af
      src/main/java/org/apache/hadoop/hbase/io/Reference.java 6360059
      src/main/java/org/apache/hadoop/hbase/mapreduce/LoadIncrementalHFiles.java 9e4ada9
      src/main/java/org/apache/hadoop/hbase/master/HMaster.java 947ec5f
      src/main/java/org/apache/hadoop/hbase/master/MasterFileSystem.java 5052878
      src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java ccc964e
      src/main/java/org/apache/hadoop/hbase/protobuf/RequestConverter.java dabfbab
      src/main/java/org/apache/hadoop/hbase/protobuf/ResponseConverter.java 45cb6cf
      src/main/java/org/apache/hadoop/hbase/protobuf/generated/ClusterIdProtos.java PRE-CREATION
      src/main/java/org/apache/hadoop/hbase/protobuf/generated/FSProtos.java PRE-CREATION
      src/main/java/org/apache/hadoop/hbase/protobuf/generated/HBaseProtos.java 058c006
      src/main/java/org/apache/hadoop/hbase/protobuf/generated/ZooKeeperProtos.java 20c7738
      src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java 9f16fee
      src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 9d3898c
      src/main/java/org/apache/hadoop/hbase/regionserver/SplitTransaction.java 6a9f2fe
      src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.java 5e1e16d
      src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java 5050df0
      src/main/java/org/apache/hadoop/hbase/security/token/AuthenticationTokenSecretManager.java 049ed8d
      src/main/java/org/apache/hadoop/hbase/util/FSTableDescriptors.java efb2b84
      src/main/java/org/apache/hadoop/hbase/util/FSUtils.java 3d35d3e
      src/main/java/org/apache/hadoop/hbase/util/HBaseFsck.java 7b4f4a2
      src/main/java/org/apache/hadoop/hbase/util/Writables.java 3d20723
      src/main/java/org/apache/hadoop/hbase/zookeeper/ClusterId.java f804810
      src/main/java/org/apache/hadoop/hbase/zookeeper/ZKClusterId.java PRE-CREATION
      src/main/protobuf/ClusterId.proto PRE-CREATION
      src/main/protobuf/FS.proto PRE-CREATION
      src/main/protobuf/ZooKeeper.proto b72cb28
      src/main/protobuf/hbase.proto 30a4c3f
      src/test/java/org/apache/hadoop/hbase/TestHColumnDescriptor.java e7fa8b2
      src/test/java/org/apache/hadoop/hbase/TestHTableDescriptor.java f7c0cca
      src/test/java/org/apache/hadoop/hbase/catalog/TestCatalogTracker.java 83d8408
      src/test/java/org/apache/hadoop/hbase/master/MockRegionServer.java 69ccc65
      src/test/java/org/apache/hadoop/hbase/master/TestCatalogJanitor.java 1020374
      src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegionInfo.java 6dfba41
      src/test/java/org/apache/hadoop/hbase/regionserver/TestStoreFile.java 988d0bf
      src/test/java/org/apache/hadoop/hbase/util/TestFSUtils.java 339a120

      Diff: https://reviews.apache.org/r/5130/diff

      Testing
      -------

      Thanks,

      Michael

      Show
      jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/5130/ ----------------------------------------------------------- Review request for hbase. Summary ------- A b/src/main/java/org/apache/hadoop/hbase/ClusterId.java New class to hold clusterid in. M b/src/main/java/org/apache/hadoop/hbase/HColumnDescriptor.java Make it so can do pb serialization. Deprecated Writable serialization. M b/src/main/java/org/apache/hadoop/hbase/HRegionInfo.java Make it so methods in here follow the pattern in HCD an HTD pb 'ing. Deprecated Writable serialization. M b/src/main/java/org/apache/hadoop/hbase/HTableDescriptor.java Make it so can do pb serialization. Deprecated Writable serialization. M b/src/main/java/org/apache/hadoop/hbase/client/HConnectionManager.java ClusterId under ZK got renamed as ZKClusterId M b/src/main/java/org/apache/hadoop/hbase/io/Reference.java Hide the Reference#Range enums. Don't let them out of this class. Make it so can do pb serialization. M b/src/main/java/org/apache/hadoop/hbase/mapreduce/LoadIncrementalHFiles.java Use new methods on Reference for getting top and bottom. M b/src/main/java/org/apache/hadoop/hbase/master/HMaster.java ClusterId under zk has been renamed ZKClusterId. Use new ClusterId class too. M b/src/main/java/org/apache/hadoop/hbase/master/MasterFileSystem.java Use new clusterid class. M b/src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java Move the RegionInfo convertion up into HRegionInfo instead of here. Added generic toDelimitedByteArray helper. M b/src/main/java/org/apache/hadoop/hbase/protobuf/RequestConverter.java Use HRegionInfo convertions instead. M b/src/main/java/org/apache/hadoop/hbase/protobuf/ResponseConverter.java Use HRegionInfo convertions instead. M b/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java Use new utility writing out .regioninfo files. M b/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java Formatting. M b/src/main/java/org/apache/hadoop/hbase/regionserver/SplitTransaction.java M b/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.java Range in Reference is no longer public. Range in Reference is no longer public. M b/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java M b/src/main/java/org/apache/hadoop/hbase/security/token/AuthenticationTokenSecretManager.java ClusterId got renamed ZKClusterId M b/src/main/java/org/apache/hadoop/hbase/util/FSTableDescriptors.java Use new serialization utlity in HTD. M b/src/main/java/org/apache/hadoop/hbase/util/FSUtils.java Generic method for writing dot file content. M b/src/main/java/org/apache/hadoop/hbase/util/HBaseFsck.java Reference#Range is not public any more M b/src/main/java/org/apache/hadoop/hbase/util/Writables.java Deprecated getHRegionInfo, etc. D b/src/main/java/org/apache/hadoop/hbase/zookeeper/ClusterId.java A b/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKClusterId.java Rename A b/src/main/protobuf/ClusterId.proto Added file for ClusterId only since its written to fs and to zk. A b/src/main/protobuf/FS.proto Protos for fs files. M b/src/main/protobuf/ZooKeeper.proto Moved ClusterId out to own proto file M b/src/main/protobuf/hbase.proto Added TableSchema and ColumnFamilySchema This addresses bug hbase-5453. https://issues.apache.org/jira/browse/hbase-5453 Diffs src/main/java/org/apache/hadoop/hbase/ClusterId.java PRE-CREATION src/main/java/org/apache/hadoop/hbase/HColumnDescriptor.java 5862f15 src/main/java/org/apache/hadoop/hbase/HRegionInfo.java 8d83ff3 src/main/java/org/apache/hadoop/hbase/HTableDescriptor.java af89e3e src/main/java/org/apache/hadoop/hbase/client/HConnectionManager.java 5cac9af src/main/java/org/apache/hadoop/hbase/io/Reference.java 6360059 src/main/java/org/apache/hadoop/hbase/mapreduce/LoadIncrementalHFiles.java 9e4ada9 src/main/java/org/apache/hadoop/hbase/master/HMaster.java 947ec5f src/main/java/org/apache/hadoop/hbase/master/MasterFileSystem.java 5052878 src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java ccc964e src/main/java/org/apache/hadoop/hbase/protobuf/RequestConverter.java dabfbab src/main/java/org/apache/hadoop/hbase/protobuf/ResponseConverter.java 45cb6cf src/main/java/org/apache/hadoop/hbase/protobuf/generated/ClusterIdProtos.java PRE-CREATION src/main/java/org/apache/hadoop/hbase/protobuf/generated/FSProtos.java PRE-CREATION src/main/java/org/apache/hadoop/hbase/protobuf/generated/HBaseProtos.java 058c006 src/main/java/org/apache/hadoop/hbase/protobuf/generated/ZooKeeperProtos.java 20c7738 src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java 9f16fee src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 9d3898c src/main/java/org/apache/hadoop/hbase/regionserver/SplitTransaction.java 6a9f2fe src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.java 5e1e16d src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java 5050df0 src/main/java/org/apache/hadoop/hbase/security/token/AuthenticationTokenSecretManager.java 049ed8d src/main/java/org/apache/hadoop/hbase/util/FSTableDescriptors.java efb2b84 src/main/java/org/apache/hadoop/hbase/util/FSUtils.java 3d35d3e src/main/java/org/apache/hadoop/hbase/util/HBaseFsck.java 7b4f4a2 src/main/java/org/apache/hadoop/hbase/util/Writables.java 3d20723 src/main/java/org/apache/hadoop/hbase/zookeeper/ClusterId.java f804810 src/main/java/org/apache/hadoop/hbase/zookeeper/ZKClusterId.java PRE-CREATION src/main/protobuf/ClusterId.proto PRE-CREATION src/main/protobuf/FS.proto PRE-CREATION src/main/protobuf/ZooKeeper.proto b72cb28 src/main/protobuf/hbase.proto 30a4c3f src/test/java/org/apache/hadoop/hbase/TestHColumnDescriptor.java e7fa8b2 src/test/java/org/apache/hadoop/hbase/TestHTableDescriptor.java f7c0cca src/test/java/org/apache/hadoop/hbase/catalog/TestCatalogTracker.java 83d8408 src/test/java/org/apache/hadoop/hbase/master/MockRegionServer.java 69ccc65 src/test/java/org/apache/hadoop/hbase/master/TestCatalogJanitor.java 1020374 src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegionInfo.java 6dfba41 src/test/java/org/apache/hadoop/hbase/regionserver/TestStoreFile.java 988d0bf src/test/java/org/apache/hadoop/hbase/util/TestFSUtils.java 339a120 Diff: https://reviews.apache.org/r/5130/diff Testing ------- Thanks, Michael
      Hide
      jiraposter@reviews.apache.org added a comment -

      -----------------------------------------------------------
      This is an automatically generated e-mail. To reply, visit:
      https://reviews.apache.org/r/5130/#review7916
      -----------------------------------------------------------

      Do we have a story for when we can remove the writable stuff?

      src/main/java/org/apache/hadoop/hbase/HColumnDescriptor.java
      <https://reviews.apache.org/r/5130/#comment17217>

      Looks like we are stepping on each other's toes a bit; I defined my own protos for this and HTableDescriptor (see the review request for HBASE-5445). Your definitions are pretty close though; I'll convert mine over.

      src/main/java/org/apache/hadoop/hbase/HRegionInfo.java
      <https://reviews.apache.org/r/5130/#comment17218>

      "not be what ou want" -> "not be what you want"

      src/main/java/org/apache/hadoop/hbase/HTableDescriptor.java
      <https://reviews.apache.org/r/5130/#comment17219>

      Did you give any thought to storing the actual data in a TableSchema rather than converting? Perhaps it's not worth it because:
      1) we still need to maintain the writable for now, so would require rewriting that part, which is a waste
      2) might not perform well, because we have to call copyFrom a bunch of times.

      src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java
      <https://reviews.apache.org/r/5130/#comment17220>

      Nice. I wrote up similar things for HTableDescriptor/HColumnDescriptor; I can just get rid of them and use your convert functions.

      src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
      <https://reviews.apache.org/r/5130/#comment17226>

      Are we sure this assumption is valid (that if the file lengths are the same, the files are the same format)?

      We can't check if there is a pb-prefix at the start of file or something?

      src/main/java/org/apache/hadoop/hbase/util/FSUtils.java
      <https://reviews.apache.org/r/5130/#comment17223>

      Same here, is this assumption valid?

      src/main/protobuf/FS.proto
      <https://reviews.apache.org/r/5130/#comment17221>

      extra whitespace

      src/main/protobuf/hbase.proto
      <https://reviews.apache.org/r/5130/#comment17222>

      extra whitespace

      • Gregory

      On 2012-05-15 22:14:17, Michael Stack wrote:

      -----------------------------------------------------------

      This is an automatically generated e-mail. To reply, visit:

      https://reviews.apache.org/r/5130/

      -----------------------------------------------------------

      (Updated 2012-05-15 22:14:17)

      Review request for hbase.

      Summary

      -------

      A b/src/main/java/org/apache/hadoop/hbase/ClusterId.java

      New class to hold clusterid in.

      M b/src/main/java/org/apache/hadoop/hbase/HColumnDescriptor.java

      Make it so can do pb serialization. Deprecated Writable serialization.

      M b/src/main/java/org/apache/hadoop/hbase/HRegionInfo.java

      Make it so methods in here follow the pattern in HCD an HTD pb 'ing.

      Deprecated Writable serialization.

      M b/src/main/java/org/apache/hadoop/hbase/HTableDescriptor.java

      Make it so can do pb serialization. Deprecated Writable serialization.

      M b/src/main/java/org/apache/hadoop/hbase/client/HConnectionManager.java

      ClusterId under ZK got renamed as ZKClusterId

      M b/src/main/java/org/apache/hadoop/hbase/io/Reference.java

      Hide the Reference#Range enums. Don't let them out of this class.

      Make it so can do pb serialization.

      M b/src/main/java/org/apache/hadoop/hbase/mapreduce/LoadIncrementalHFiles.java

      Use new methods on Reference for getting top and bottom.

      M b/src/main/java/org/apache/hadoop/hbase/master/HMaster.java

      ClusterId under zk has been renamed ZKClusterId.

      Use new ClusterId class too.

      M b/src/main/java/org/apache/hadoop/hbase/master/MasterFileSystem.java

      Use new clusterid class.

      M b/src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java

      Move the RegionInfo convertion up into HRegionInfo instead of here.

      Added generic toDelimitedByteArray helper.

      M b/src/main/java/org/apache/hadoop/hbase/protobuf/RequestConverter.java

      Use HRegionInfo convertions instead.

      M b/src/main/java/org/apache/hadoop/hbase/protobuf/ResponseConverter.java

      Use HRegionInfo convertions instead.

      M b/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java

      Use new utility writing out .regioninfo files.

      M b/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java

      Formatting.

      M b/src/main/java/org/apache/hadoop/hbase/regionserver/SplitTransaction.java

      M b/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.java

      Range in Reference is no longer public.

      Range in Reference is no longer public.

      M b/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java

      M b/src/main/java/org/apache/hadoop/hbase/security/token/AuthenticationTokenSecretManager.java

      ClusterId got renamed ZKClusterId

      M b/src/main/java/org/apache/hadoop/hbase/util/FSTableDescriptors.java

      Use new serialization utlity in HTD.

      M b/src/main/java/org/apache/hadoop/hbase/util/FSUtils.java

      Generic method for writing dot file content.

      M b/src/main/java/org/apache/hadoop/hbase/util/HBaseFsck.java

      Reference#Range is not public any more

      M b/src/main/java/org/apache/hadoop/hbase/util/Writables.java

      Deprecated getHRegionInfo, etc.

      D b/src/main/java/org/apache/hadoop/hbase/zookeeper/ClusterId.java

      A b/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKClusterId.java

      Rename

      A b/src/main/protobuf/ClusterId.proto

      Added file for ClusterId only since its written to fs and to zk.

      A b/src/main/protobuf/FS.proto

      Protos for fs files.

      M b/src/main/protobuf/ZooKeeper.proto

      Moved ClusterId out to own proto file

      M b/src/main/protobuf/hbase.proto

      Added TableSchema and ColumnFamilySchema

      This addresses bug hbase-5453.

      https://issues.apache.org/jira/browse/hbase-5453

      Diffs

      -----

      src/main/java/org/apache/hadoop/hbase/ClusterId.java PRE-CREATION

      src/main/java/org/apache/hadoop/hbase/HColumnDescriptor.java 5862f15

      src/main/java/org/apache/hadoop/hbase/HRegionInfo.java 8d83ff3

      src/main/java/org/apache/hadoop/hbase/HTableDescriptor.java af89e3e

      src/main/java/org/apache/hadoop/hbase/client/HConnectionManager.java 5cac9af

      src/main/java/org/apache/hadoop/hbase/io/Reference.java 6360059

      src/main/java/org/apache/hadoop/hbase/mapreduce/LoadIncrementalHFiles.java 9e4ada9

      src/main/java/org/apache/hadoop/hbase/master/HMaster.java 947ec5f

      src/main/java/org/apache/hadoop/hbase/master/MasterFileSystem.java 5052878

      src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java ccc964e

      src/main/java/org/apache/hadoop/hbase/protobuf/RequestConverter.java dabfbab

      src/main/java/org/apache/hadoop/hbase/protobuf/ResponseConverter.java 45cb6cf

      src/main/java/org/apache/hadoop/hbase/protobuf/generated/ClusterIdProtos.java PRE-CREATION

      src/main/java/org/apache/hadoop/hbase/protobuf/generated/FSProtos.java PRE-CREATION

      src/main/java/org/apache/hadoop/hbase/protobuf/generated/HBaseProtos.java 058c006

      src/main/java/org/apache/hadoop/hbase/protobuf/generated/ZooKeeperProtos.java 20c7738

      src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java 9f16fee

      src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 9d3898c

      src/main/java/org/apache/hadoop/hbase/regionserver/SplitTransaction.java 6a9f2fe

      src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.java 5e1e16d

      src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java 5050df0

      src/main/java/org/apache/hadoop/hbase/security/token/AuthenticationTokenSecretManager.java 049ed8d

      src/main/java/org/apache/hadoop/hbase/util/FSTableDescriptors.java efb2b84

      src/main/java/org/apache/hadoop/hbase/util/FSUtils.java 3d35d3e

      src/main/java/org/apache/hadoop/hbase/util/HBaseFsck.java 7b4f4a2

      src/main/java/org/apache/hadoop/hbase/util/Writables.java 3d20723

      src/main/java/org/apache/hadoop/hbase/zookeeper/ClusterId.java f804810

      src/main/java/org/apache/hadoop/hbase/zookeeper/ZKClusterId.java PRE-CREATION

      src/main/protobuf/ClusterId.proto PRE-CREATION

      src/main/protobuf/FS.proto PRE-CREATION

      src/main/protobuf/ZooKeeper.proto b72cb28

      src/main/protobuf/hbase.proto 30a4c3f

      src/test/java/org/apache/hadoop/hbase/TestHColumnDescriptor.java e7fa8b2

      src/test/java/org/apache/hadoop/hbase/TestHTableDescriptor.java f7c0cca

      src/test/java/org/apache/hadoop/hbase/catalog/TestCatalogTracker.java 83d8408

      src/test/java/org/apache/hadoop/hbase/master/MockRegionServer.java 69ccc65

      src/test/java/org/apache/hadoop/hbase/master/TestCatalogJanitor.java 1020374

      src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegionInfo.java 6dfba41

      src/test/java/org/apache/hadoop/hbase/regionserver/TestStoreFile.java 988d0bf

      src/test/java/org/apache/hadoop/hbase/util/TestFSUtils.java 339a120

      Diff: https://reviews.apache.org/r/5130/diff

      Testing

      -------

      Thanks,

      Michael

      Show
      jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/5130/#review7916 ----------------------------------------------------------- Do we have a story for when we can remove the writable stuff? src/main/java/org/apache/hadoop/hbase/HColumnDescriptor.java < https://reviews.apache.org/r/5130/#comment17217 > Looks like we are stepping on each other's toes a bit; I defined my own protos for this and HTableDescriptor (see the review request for HBASE-5445 ). Your definitions are pretty close though; I'll convert mine over. src/main/java/org/apache/hadoop/hbase/HRegionInfo.java < https://reviews.apache.org/r/5130/#comment17218 > "not be what ou want" -> "not be what you want" src/main/java/org/apache/hadoop/hbase/HTableDescriptor.java < https://reviews.apache.org/r/5130/#comment17219 > Did you give any thought to storing the actual data in a TableSchema rather than converting? Perhaps it's not worth it because: 1) we still need to maintain the writable for now, so would require rewriting that part, which is a waste 2) might not perform well, because we have to call copyFrom a bunch of times. src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java < https://reviews.apache.org/r/5130/#comment17220 > Nice. I wrote up similar things for HTableDescriptor/HColumnDescriptor; I can just get rid of them and use your convert functions. src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java < https://reviews.apache.org/r/5130/#comment17226 > Are we sure this assumption is valid (that if the file lengths are the same, the files are the same format)? We can't check if there is a pb-prefix at the start of file or something? src/main/java/org/apache/hadoop/hbase/util/FSUtils.java < https://reviews.apache.org/r/5130/#comment17223 > Same here, is this assumption valid? src/main/protobuf/FS.proto < https://reviews.apache.org/r/5130/#comment17221 > extra whitespace src/main/protobuf/hbase.proto < https://reviews.apache.org/r/5130/#comment17222 > extra whitespace Gregory On 2012-05-15 22:14:17, Michael Stack wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/5130/ ----------------------------------------------------------- (Updated 2012-05-15 22:14:17) Review request for hbase. Summary ------- A b/src/main/java/org/apache/hadoop/hbase/ClusterId.java New class to hold clusterid in. M b/src/main/java/org/apache/hadoop/hbase/HColumnDescriptor.java Make it so can do pb serialization. Deprecated Writable serialization. M b/src/main/java/org/apache/hadoop/hbase/HRegionInfo.java Make it so methods in here follow the pattern in HCD an HTD pb 'ing. Deprecated Writable serialization. M b/src/main/java/org/apache/hadoop/hbase/HTableDescriptor.java Make it so can do pb serialization. Deprecated Writable serialization. M b/src/main/java/org/apache/hadoop/hbase/client/HConnectionManager.java ClusterId under ZK got renamed as ZKClusterId M b/src/main/java/org/apache/hadoop/hbase/io/Reference.java Hide the Reference#Range enums. Don't let them out of this class. Make it so can do pb serialization. M b/src/main/java/org/apache/hadoop/hbase/mapreduce/LoadIncrementalHFiles.java Use new methods on Reference for getting top and bottom. M b/src/main/java/org/apache/hadoop/hbase/master/HMaster.java ClusterId under zk has been renamed ZKClusterId. Use new ClusterId class too. M b/src/main/java/org/apache/hadoop/hbase/master/MasterFileSystem.java Use new clusterid class. M b/src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java Move the RegionInfo convertion up into HRegionInfo instead of here. Added generic toDelimitedByteArray helper. M b/src/main/java/org/apache/hadoop/hbase/protobuf/RequestConverter.java Use HRegionInfo convertions instead. M b/src/main/java/org/apache/hadoop/hbase/protobuf/ResponseConverter.java Use HRegionInfo convertions instead. M b/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java Use new utility writing out .regioninfo files. M b/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java Formatting. M b/src/main/java/org/apache/hadoop/hbase/regionserver/SplitTransaction.java M b/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.java Range in Reference is no longer public. Range in Reference is no longer public. M b/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java M b/src/main/java/org/apache/hadoop/hbase/security/token/AuthenticationTokenSecretManager.java ClusterId got renamed ZKClusterId M b/src/main/java/org/apache/hadoop/hbase/util/FSTableDescriptors.java Use new serialization utlity in HTD. M b/src/main/java/org/apache/hadoop/hbase/util/FSUtils.java Generic method for writing dot file content. M b/src/main/java/org/apache/hadoop/hbase/util/HBaseFsck.java Reference#Range is not public any more M b/src/main/java/org/apache/hadoop/hbase/util/Writables.java Deprecated getHRegionInfo, etc. D b/src/main/java/org/apache/hadoop/hbase/zookeeper/ClusterId.java A b/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKClusterId.java Rename A b/src/main/protobuf/ClusterId.proto Added file for ClusterId only since its written to fs and to zk. A b/src/main/protobuf/FS.proto Protos for fs files. M b/src/main/protobuf/ZooKeeper.proto Moved ClusterId out to own proto file M b/src/main/protobuf/hbase.proto Added TableSchema and ColumnFamilySchema This addresses bug hbase-5453. https://issues.apache.org/jira/browse/hbase-5453 Diffs ----- src/main/java/org/apache/hadoop/hbase/ClusterId.java PRE-CREATION src/main/java/org/apache/hadoop/hbase/HColumnDescriptor.java 5862f15 src/main/java/org/apache/hadoop/hbase/HRegionInfo.java 8d83ff3 src/main/java/org/apache/hadoop/hbase/HTableDescriptor.java af89e3e src/main/java/org/apache/hadoop/hbase/client/HConnectionManager.java 5cac9af src/main/java/org/apache/hadoop/hbase/io/Reference.java 6360059 src/main/java/org/apache/hadoop/hbase/mapreduce/LoadIncrementalHFiles.java 9e4ada9 src/main/java/org/apache/hadoop/hbase/master/HMaster.java 947ec5f src/main/java/org/apache/hadoop/hbase/master/MasterFileSystem.java 5052878 src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java ccc964e src/main/java/org/apache/hadoop/hbase/protobuf/RequestConverter.java dabfbab src/main/java/org/apache/hadoop/hbase/protobuf/ResponseConverter.java 45cb6cf src/main/java/org/apache/hadoop/hbase/protobuf/generated/ClusterIdProtos.java PRE-CREATION src/main/java/org/apache/hadoop/hbase/protobuf/generated/FSProtos.java PRE-CREATION src/main/java/org/apache/hadoop/hbase/protobuf/generated/HBaseProtos.java 058c006 src/main/java/org/apache/hadoop/hbase/protobuf/generated/ZooKeeperProtos.java 20c7738 src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java 9f16fee src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 9d3898c src/main/java/org/apache/hadoop/hbase/regionserver/SplitTransaction.java 6a9f2fe src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.java 5e1e16d src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java 5050df0 src/main/java/org/apache/hadoop/hbase/security/token/AuthenticationTokenSecretManager.java 049ed8d src/main/java/org/apache/hadoop/hbase/util/FSTableDescriptors.java efb2b84 src/main/java/org/apache/hadoop/hbase/util/FSUtils.java 3d35d3e src/main/java/org/apache/hadoop/hbase/util/HBaseFsck.java 7b4f4a2 src/main/java/org/apache/hadoop/hbase/util/Writables.java 3d20723 src/main/java/org/apache/hadoop/hbase/zookeeper/ClusterId.java f804810 src/main/java/org/apache/hadoop/hbase/zookeeper/ZKClusterId.java PRE-CREATION src/main/protobuf/ClusterId.proto PRE-CREATION src/main/protobuf/FS.proto PRE-CREATION src/main/protobuf/ZooKeeper.proto b72cb28 src/main/protobuf/hbase.proto 30a4c3f src/test/java/org/apache/hadoop/hbase/TestHColumnDescriptor.java e7fa8b2 src/test/java/org/apache/hadoop/hbase/TestHTableDescriptor.java f7c0cca src/test/java/org/apache/hadoop/hbase/catalog/TestCatalogTracker.java 83d8408 src/test/java/org/apache/hadoop/hbase/master/MockRegionServer.java 69ccc65 src/test/java/org/apache/hadoop/hbase/master/TestCatalogJanitor.java 1020374 src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegionInfo.java 6dfba41 src/test/java/org/apache/hadoop/hbase/regionserver/TestStoreFile.java 988d0bf src/test/java/org/apache/hadoop/hbase/util/TestFSUtils.java 339a120 Diff: https://reviews.apache.org/r/5130/diff Testing ------- Thanks, Michael
      Hide
      stack added a comment -

      Stashing v6 here for the moment. Does not address yet Gregory's comments. Working on that now in a v7.

      Show
      stack added a comment - Stashing v6 here for the moment. Does not address yet Gregory's comments. Working on that now in a v7.
      Hide
      jiraposter@reviews.apache.org added a comment -

      On 2012-05-15 23:08:35, Gregory Chanan wrote:

      > Do we have a story for when we can remove the writable stuff?

      No. Smile. Being optimistic. Have to start somewhere. No harm marking this stuff deprecated in meantime as heads-up.

      I think we won't be able to migrate over these old classes. I'm thinking we'll have to do new ones and deprecate the old given they inherit from Writable. It'll be a bit of a pain though given some of these classes come out in our public API.

      But we have to start somewhere.

      On 2012-05-15 23:08:35, Gregory Chanan wrote:

      > src/main/java/org/apache/hadoop/hbase/HColumnDescriptor.java, line 1017

      > <https://reviews.apache.org/r/5130/diff/1/?file=109035#file109035line1017>

      >

      > Looks like we are stepping on each other's toes a bit; I defined my own protos for this and HTableDescriptor (see the review request for HBASE-5445). Your definitions are pretty close though; I'll convert mine over.

      Well, Andrew beat us both to it over in his REST pb stuff. We need to reconcile his w/ ours too....

      On 2012-05-15 23:08:35, Gregory Chanan wrote:

      > src/main/java/org/apache/hadoop/hbase/HRegionInfo.java, line 954

      > <https://reviews.apache.org/r/5130/diff/1/?file=109036#file109036line954>

      >

      > "not be what ou want" -> "not be what you want"

      Fixed

      On 2012-05-15 23:08:35, Gregory Chanan wrote:

      > src/main/java/org/apache/hadoop/hbase/HTableDescriptor.java, line 1240

      > <https://reviews.apache.org/r/5130/diff/1/?file=109037#file109037line1240>

      >

      > Did you give any thought to storing the actual data in a TableSchema rather than converting? Perhaps it's not worth it because:

      > 1) we still need to maintain the writable for now, so would require rewriting that part, which is a waste

      > 2) might not perform well, because we have to call copyFrom a bunch of times.

      Yeah. Thought about it but we want Writable still converting old serializations. Mostly I reasoned that these classes are rarely, relatively serialized, and performance is not important when we do want to pass these classes serialized. Yeah, it'd be a pain converting.

      On 2012-05-15 23:08:35, Gregory Chanan wrote:

      > src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java, line 748

      > <https://reviews.apache.org/r/5130/diff/1/?file=109050#file109050line748>

      >

      > Are we sure this assumption is valid (that if the file lengths are the same, the files are the same format)?

      >

      > We can't check if there is a pb-prefix at the start of file or something?

      This is my way of figuring if I need to convert the file. There is an off chance that file could be same length though serialized with Writables. Good one. Let me fix that.

      • Michael

      -----------------------------------------------------------
      This is an automatically generated e-mail. To reply, visit:
      https://reviews.apache.org/r/5130/#review7916
      -----------------------------------------------------------

      On 2012-05-15 22:14:17, Michael Stack wrote:

      -----------------------------------------------------------

      This is an automatically generated e-mail. To reply, visit:

      https://reviews.apache.org/r/5130/

      -----------------------------------------------------------

      (Updated 2012-05-15 22:14:17)

      Review request for hbase.

      Summary

      -------

      A b/src/main/java/org/apache/hadoop/hbase/ClusterId.java

      New class to hold clusterid in.

      M b/src/main/java/org/apache/hadoop/hbase/HColumnDescriptor.java

      Make it so can do pb serialization. Deprecated Writable serialization.

      M b/src/main/java/org/apache/hadoop/hbase/HRegionInfo.java

      Make it so methods in here follow the pattern in HCD an HTD pb 'ing.

      Deprecated Writable serialization.

      M b/src/main/java/org/apache/hadoop/hbase/HTableDescriptor.java

      Make it so can do pb serialization. Deprecated Writable serialization.

      M b/src/main/java/org/apache/hadoop/hbase/client/HConnectionManager.java

      ClusterId under ZK got renamed as ZKClusterId

      M b/src/main/java/org/apache/hadoop/hbase/io/Reference.java

      Hide the Reference#Range enums. Don't let them out of this class.

      Make it so can do pb serialization.

      M b/src/main/java/org/apache/hadoop/hbase/mapreduce/LoadIncrementalHFiles.java

      Use new methods on Reference for getting top and bottom.

      M b/src/main/java/org/apache/hadoop/hbase/master/HMaster.java

      ClusterId under zk has been renamed ZKClusterId.

      Use new ClusterId class too.

      M b/src/main/java/org/apache/hadoop/hbase/master/MasterFileSystem.java

      Use new clusterid class.

      M b/src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java

      Move the RegionInfo convertion up into HRegionInfo instead of here.

      Added generic toDelimitedByteArray helper.

      M b/src/main/java/org/apache/hadoop/hbase/protobuf/RequestConverter.java

      Use HRegionInfo convertions instead.

      M b/src/main/java/org/apache/hadoop/hbase/protobuf/ResponseConverter.java

      Use HRegionInfo convertions instead.

      M b/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java

      Use new utility writing out .regioninfo files.

      M b/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java

      Formatting.

      M b/src/main/java/org/apache/hadoop/hbase/regionserver/SplitTransaction.java

      M b/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.java

      Range in Reference is no longer public.

      Range in Reference is no longer public.

      M b/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java

      M b/src/main/java/org/apache/hadoop/hbase/security/token/AuthenticationTokenSecretManager.java

      ClusterId got renamed ZKClusterId

      M b/src/main/java/org/apache/hadoop/hbase/util/FSTableDescriptors.java

      Use new serialization utlity in HTD.

      M b/src/main/java/org/apache/hadoop/hbase/util/FSUtils.java

      Generic method for writing dot file content.

      M b/src/main/java/org/apache/hadoop/hbase/util/HBaseFsck.java

      Reference#Range is not public any more

      M b/src/main/java/org/apache/hadoop/hbase/util/Writables.java

      Deprecated getHRegionInfo, etc.

      D b/src/main/java/org/apache/hadoop/hbase/zookeeper/ClusterId.java

      A b/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKClusterId.java

      Rename

      A b/src/main/protobuf/ClusterId.proto

      Added file for ClusterId only since its written to fs and to zk.

      A b/src/main/protobuf/FS.proto

      Protos for fs files.

      M b/src/main/protobuf/ZooKeeper.proto

      Moved ClusterId out to own proto file

      M b/src/main/protobuf/hbase.proto

      Added TableSchema and ColumnFamilySchema

      This addresses bug hbase-5453.

      https://issues.apache.org/jira/browse/hbase-5453

      Diffs

      -----

      src/main/java/org/apache/hadoop/hbase/ClusterId.java PRE-CREATION

      src/main/java/org/apache/hadoop/hbase/HColumnDescriptor.java 5862f15

      src/main/java/org/apache/hadoop/hbase/HRegionInfo.java 8d83ff3

      src/main/java/org/apache/hadoop/hbase/HTableDescriptor.java af89e3e

      src/main/java/org/apache/hadoop/hbase/client/HConnectionManager.java 5cac9af

      src/main/java/org/apache/hadoop/hbase/io/Reference.java 6360059

      src/main/java/org/apache/hadoop/hbase/mapreduce/LoadIncrementalHFiles.java 9e4ada9

      src/main/java/org/apache/hadoop/hbase/master/HMaster.java 947ec5f

      src/main/java/org/apache/hadoop/hbase/master/MasterFileSystem.java 5052878

      src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java ccc964e

      src/main/java/org/apache/hadoop/hbase/protobuf/RequestConverter.java dabfbab

      src/main/java/org/apache/hadoop/hbase/protobuf/ResponseConverter.java 45cb6cf

      src/main/java/org/apache/hadoop/hbase/protobuf/generated/ClusterIdProtos.java PRE-CREATION

      src/main/java/org/apache/hadoop/hbase/protobuf/generated/FSProtos.java PRE-CREATION

      src/main/java/org/apache/hadoop/hbase/protobuf/generated/HBaseProtos.java 058c006

      src/main/java/org/apache/hadoop/hbase/protobuf/generated/ZooKeeperProtos.java 20c7738

      src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java 9f16fee

      src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 9d3898c

      src/main/java/org/apache/hadoop/hbase/regionserver/SplitTransaction.java 6a9f2fe

      src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.java 5e1e16d

      src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java 5050df0

      src/main/java/org/apache/hadoop/hbase/security/token/AuthenticationTokenSecretManager.java 049ed8d

      src/main/java/org/apache/hadoop/hbase/util/FSTableDescriptors.java efb2b84

      src/main/java/org/apache/hadoop/hbase/util/FSUtils.java 3d35d3e

      src/main/java/org/apache/hadoop/hbase/util/HBaseFsck.java 7b4f4a2

      src/main/java/org/apache/hadoop/hbase/util/Writables.java 3d20723

      src/main/java/org/apache/hadoop/hbase/zookeeper/ClusterId.java f804810

      src/main/java/org/apache/hadoop/hbase/zookeeper/ZKClusterId.java PRE-CREATION

      src/main/protobuf/ClusterId.proto PRE-CREATION

      src/main/protobuf/FS.proto PRE-CREATION

      src/main/protobuf/ZooKeeper.proto b72cb28

      src/main/protobuf/hbase.proto 30a4c3f

      src/test/java/org/apache/hadoop/hbase/TestHColumnDescriptor.java e7fa8b2

      src/test/java/org/apache/hadoop/hbase/TestHTableDescriptor.java f7c0cca

      src/test/java/org/apache/hadoop/hbase/catalog/TestCatalogTracker.java 83d8408

      src/test/java/org/apache/hadoop/hbase/master/MockRegionServer.java 69ccc65

      src/test/java/org/apache/hadoop/hbase/master/TestCatalogJanitor.java 1020374

      src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegionInfo.java 6dfba41

      src/test/java/org/apache/hadoop/hbase/regionserver/TestStoreFile.java 988d0bf

      src/test/java/org/apache/hadoop/hbase/util/TestFSUtils.java 339a120

      Diff: https://reviews.apache.org/r/5130/diff

      Testing

      -------

      Thanks,

      Michael

      Show
      jiraposter@reviews.apache.org added a comment - On 2012-05-15 23:08:35, Gregory Chanan wrote: > Do we have a story for when we can remove the writable stuff? No. Smile. Being optimistic. Have to start somewhere. No harm marking this stuff deprecated in meantime as heads-up. I think we won't be able to migrate over these old classes. I'm thinking we'll have to do new ones and deprecate the old given they inherit from Writable. It'll be a bit of a pain though given some of these classes come out in our public API. But we have to start somewhere. On 2012-05-15 23:08:35, Gregory Chanan wrote: > src/main/java/org/apache/hadoop/hbase/HColumnDescriptor.java, line 1017 > < https://reviews.apache.org/r/5130/diff/1/?file=109035#file109035line1017 > > > Looks like we are stepping on each other's toes a bit; I defined my own protos for this and HTableDescriptor (see the review request for HBASE-5445 ). Your definitions are pretty close though; I'll convert mine over. Well, Andrew beat us both to it over in his REST pb stuff. We need to reconcile his w/ ours too.... On 2012-05-15 23:08:35, Gregory Chanan wrote: > src/main/java/org/apache/hadoop/hbase/HRegionInfo.java, line 954 > < https://reviews.apache.org/r/5130/diff/1/?file=109036#file109036line954 > > > "not be what ou want" -> "not be what you want" Fixed On 2012-05-15 23:08:35, Gregory Chanan wrote: > src/main/java/org/apache/hadoop/hbase/HTableDescriptor.java, line 1240 > < https://reviews.apache.org/r/5130/diff/1/?file=109037#file109037line1240 > > > Did you give any thought to storing the actual data in a TableSchema rather than converting? Perhaps it's not worth it because: > 1) we still need to maintain the writable for now, so would require rewriting that part, which is a waste > 2) might not perform well, because we have to call copyFrom a bunch of times. Yeah. Thought about it but we want Writable still converting old serializations. Mostly I reasoned that these classes are rarely, relatively serialized, and performance is not important when we do want to pass these classes serialized. Yeah, it'd be a pain converting. On 2012-05-15 23:08:35, Gregory Chanan wrote: > src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java, line 748 > < https://reviews.apache.org/r/5130/diff/1/?file=109050#file109050line748 > > > Are we sure this assumption is valid (that if the file lengths are the same, the files are the same format)? > > We can't check if there is a pb-prefix at the start of file or something? This is my way of figuring if I need to convert the file. There is an off chance that file could be same length though serialized with Writables. Good one. Let me fix that. Michael ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/5130/#review7916 ----------------------------------------------------------- On 2012-05-15 22:14:17, Michael Stack wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/5130/ ----------------------------------------------------------- (Updated 2012-05-15 22:14:17) Review request for hbase. Summary ------- A b/src/main/java/org/apache/hadoop/hbase/ClusterId.java New class to hold clusterid in. M b/src/main/java/org/apache/hadoop/hbase/HColumnDescriptor.java Make it so can do pb serialization. Deprecated Writable serialization. M b/src/main/java/org/apache/hadoop/hbase/HRegionInfo.java Make it so methods in here follow the pattern in HCD an HTD pb 'ing. Deprecated Writable serialization. M b/src/main/java/org/apache/hadoop/hbase/HTableDescriptor.java Make it so can do pb serialization. Deprecated Writable serialization. M b/src/main/java/org/apache/hadoop/hbase/client/HConnectionManager.java ClusterId under ZK got renamed as ZKClusterId M b/src/main/java/org/apache/hadoop/hbase/io/Reference.java Hide the Reference#Range enums. Don't let them out of this class. Make it so can do pb serialization. M b/src/main/java/org/apache/hadoop/hbase/mapreduce/LoadIncrementalHFiles.java Use new methods on Reference for getting top and bottom. M b/src/main/java/org/apache/hadoop/hbase/master/HMaster.java ClusterId under zk has been renamed ZKClusterId. Use new ClusterId class too. M b/src/main/java/org/apache/hadoop/hbase/master/MasterFileSystem.java Use new clusterid class. M b/src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java Move the RegionInfo convertion up into HRegionInfo instead of here. Added generic toDelimitedByteArray helper. M b/src/main/java/org/apache/hadoop/hbase/protobuf/RequestConverter.java Use HRegionInfo convertions instead. M b/src/main/java/org/apache/hadoop/hbase/protobuf/ResponseConverter.java Use HRegionInfo convertions instead. M b/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java Use new utility writing out .regioninfo files. M b/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java Formatting. M b/src/main/java/org/apache/hadoop/hbase/regionserver/SplitTransaction.java M b/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.java Range in Reference is no longer public. Range in Reference is no longer public. M b/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java M b/src/main/java/org/apache/hadoop/hbase/security/token/AuthenticationTokenSecretManager.java ClusterId got renamed ZKClusterId M b/src/main/java/org/apache/hadoop/hbase/util/FSTableDescriptors.java Use new serialization utlity in HTD. M b/src/main/java/org/apache/hadoop/hbase/util/FSUtils.java Generic method for writing dot file content. M b/src/main/java/org/apache/hadoop/hbase/util/HBaseFsck.java Reference#Range is not public any more M b/src/main/java/org/apache/hadoop/hbase/util/Writables.java Deprecated getHRegionInfo, etc. D b/src/main/java/org/apache/hadoop/hbase/zookeeper/ClusterId.java A b/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKClusterId.java Rename A b/src/main/protobuf/ClusterId.proto Added file for ClusterId only since its written to fs and to zk. A b/src/main/protobuf/FS.proto Protos for fs files. M b/src/main/protobuf/ZooKeeper.proto Moved ClusterId out to own proto file M b/src/main/protobuf/hbase.proto Added TableSchema and ColumnFamilySchema This addresses bug hbase-5453. https://issues.apache.org/jira/browse/hbase-5453 Diffs ----- src/main/java/org/apache/hadoop/hbase/ClusterId.java PRE-CREATION src/main/java/org/apache/hadoop/hbase/HColumnDescriptor.java 5862f15 src/main/java/org/apache/hadoop/hbase/HRegionInfo.java 8d83ff3 src/main/java/org/apache/hadoop/hbase/HTableDescriptor.java af89e3e src/main/java/org/apache/hadoop/hbase/client/HConnectionManager.java 5cac9af src/main/java/org/apache/hadoop/hbase/io/Reference.java 6360059 src/main/java/org/apache/hadoop/hbase/mapreduce/LoadIncrementalHFiles.java 9e4ada9 src/main/java/org/apache/hadoop/hbase/master/HMaster.java 947ec5f src/main/java/org/apache/hadoop/hbase/master/MasterFileSystem.java 5052878 src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java ccc964e src/main/java/org/apache/hadoop/hbase/protobuf/RequestConverter.java dabfbab src/main/java/org/apache/hadoop/hbase/protobuf/ResponseConverter.java 45cb6cf src/main/java/org/apache/hadoop/hbase/protobuf/generated/ClusterIdProtos.java PRE-CREATION src/main/java/org/apache/hadoop/hbase/protobuf/generated/FSProtos.java PRE-CREATION src/main/java/org/apache/hadoop/hbase/protobuf/generated/HBaseProtos.java 058c006 src/main/java/org/apache/hadoop/hbase/protobuf/generated/ZooKeeperProtos.java 20c7738 src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java 9f16fee src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 9d3898c src/main/java/org/apache/hadoop/hbase/regionserver/SplitTransaction.java 6a9f2fe src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.java 5e1e16d src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java 5050df0 src/main/java/org/apache/hadoop/hbase/security/token/AuthenticationTokenSecretManager.java 049ed8d src/main/java/org/apache/hadoop/hbase/util/FSTableDescriptors.java efb2b84 src/main/java/org/apache/hadoop/hbase/util/FSUtils.java 3d35d3e src/main/java/org/apache/hadoop/hbase/util/HBaseFsck.java 7b4f4a2 src/main/java/org/apache/hadoop/hbase/util/Writables.java 3d20723 src/main/java/org/apache/hadoop/hbase/zookeeper/ClusterId.java f804810 src/main/java/org/apache/hadoop/hbase/zookeeper/ZKClusterId.java PRE-CREATION src/main/protobuf/ClusterId.proto PRE-CREATION src/main/protobuf/FS.proto PRE-CREATION src/main/protobuf/ZooKeeper.proto b72cb28 src/main/protobuf/hbase.proto 30a4c3f src/test/java/org/apache/hadoop/hbase/TestHColumnDescriptor.java e7fa8b2 src/test/java/org/apache/hadoop/hbase/TestHTableDescriptor.java f7c0cca src/test/java/org/apache/hadoop/hbase/catalog/TestCatalogTracker.java 83d8408 src/test/java/org/apache/hadoop/hbase/master/MockRegionServer.java 69ccc65 src/test/java/org/apache/hadoop/hbase/master/TestCatalogJanitor.java 1020374 src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegionInfo.java 6dfba41 src/test/java/org/apache/hadoop/hbase/regionserver/TestStoreFile.java 988d0bf src/test/java/org/apache/hadoop/hbase/util/TestFSUtils.java 339a120 Diff: https://reviews.apache.org/r/5130/diff Testing ------- Thanks, Michael
      Hide
      stack added a comment -

      Address Gregory's comments.

      I've changed the format of .regioninfo and .tableinfo. Now instead of serialized Writable followed by toString of the serialized object, instead its just the serialized pb.

      This removes our having a human readable .regioninfo/.tablinfo file but my guess no one relied on this anyways.

      Having just serialized content in the file means a check of file length should be enough figuring whether the file properly serialized. If ever a chance that a Writable + its toString + two '\n' characters was equal to a serialized pb, I'd think this likely a pathological state. If this state is not cleared up 'naturally' by splits or a schema change, then lets deal if it happens.

      I only need this length-checking in one place on region open. I want to avoid reading the .regioninfo file on region open. The alternative means more load on NN and DNs at region open time which could be problematic at big-bang cluster start (Thinking 500 nodes w/ 80k regions, an actual known case).

      Otherwise, Gregory's comments led to me to check and I was missing convertion of fs files to pb in all cases. This should be fixed now.

      There are some failing tests still but running by hadoopqa to see what it says anyways. Also putting up on rb to get feedback if problem w/ this approach.

      Show
      stack added a comment - Address Gregory's comments. I've changed the format of .regioninfo and .tableinfo. Now instead of serialized Writable followed by toString of the serialized object, instead its just the serialized pb. This removes our having a human readable .regioninfo/.tablinfo file but my guess no one relied on this anyways. Having just serialized content in the file means a check of file length should be enough figuring whether the file properly serialized. If ever a chance that a Writable + its toString + two '\n' characters was equal to a serialized pb, I'd think this likely a pathological state. If this state is not cleared up 'naturally' by splits or a schema change, then lets deal if it happens. I only need this length-checking in one place on region open. I want to avoid reading the .regioninfo file on region open. The alternative means more load on NN and DNs at region open time which could be problematic at big-bang cluster start (Thinking 500 nodes w/ 80k regions, an actual known case). Otherwise, Gregory's comments led to me to check and I was missing convertion of fs files to pb in all cases. This should be fixed now. There are some failing tests still but running by hadoopqa to see what it says anyways. Also putting up on rb to get feedback if problem w/ this approach.
      Hide
      jiraposter@reviews.apache.org added a comment -

      -----------------------------------------------------------
      This is an automatically generated e-mail. To reply, visit:
      https://reviews.apache.org/r/5130/
      -----------------------------------------------------------

      (Updated 2012-05-16 17:02:35.527708)

      Review request for hbase.

      Changes
      -------

      Gregory. I addressed your comments.

      I've changed the format of .regioninfo and .tableinfo. Now instead of serialized Writable followed by toString of the serialized object, instead its just the serialized pb. This removes our having a human readable .regioninfo/.tablinfo file but my guess no one relied on this anyways.

      Having just serialized content in the file means a check of file length should be enough figuring whether the file properly serialized. If ever a chance that a Writable + its toString + two '\n' characters was equal to a serialized pb, I'd think this a pathological state. If this state is not cleared up 'naturally' by splits or a schema change, then lets deal if it happens.

      I only need this length-checking in one place on region open. I want to avoid reading the .regioninfo file on region open. The alternative means more load on NN and DNs at region open time which could be problematic at big-bang cluster start (Thinking 500 nodes w/ 80k regions, an actual known case – this is the case I have in mind when I'm trying to avoid more load on NN/DNs).

      Otherwise, Gregory's comments led to me to check and I was missing convertion of fs files to pb in all cases (I was just reading the clusterid and hbase.version files, not converting if still Writable). This should be fixed now.

      There are some failing tests still but running by hadoopqa to see what it says anyways. Also putting up on rb to get feedback if problem w/ this approach.

      Summary
      -------

      A b/src/main/java/org/apache/hadoop/hbase/ClusterId.java
      New class to hold clusterid in.
      M b/src/main/java/org/apache/hadoop/hbase/HColumnDescriptor.java
      Make it so can do pb serialization. Deprecated Writable serialization.
      M b/src/main/java/org/apache/hadoop/hbase/HRegionInfo.java
      Make it so methods in here follow the pattern in HCD an HTD pb 'ing.
      Deprecated Writable serialization.
      M b/src/main/java/org/apache/hadoop/hbase/HTableDescriptor.java
      Make it so can do pb serialization. Deprecated Writable serialization.
      M b/src/main/java/org/apache/hadoop/hbase/client/HConnectionManager.java
      ClusterId under ZK got renamed as ZKClusterId
      M b/src/main/java/org/apache/hadoop/hbase/io/Reference.java
      Hide the Reference#Range enums. Don't let them out of this class.
      Make it so can do pb serialization.
      M b/src/main/java/org/apache/hadoop/hbase/mapreduce/LoadIncrementalHFiles.java
      Use new methods on Reference for getting top and bottom.
      M b/src/main/java/org/apache/hadoop/hbase/master/HMaster.java
      ClusterId under zk has been renamed ZKClusterId.
      Use new ClusterId class too.
      M b/src/main/java/org/apache/hadoop/hbase/master/MasterFileSystem.java
      Use new clusterid class.
      M b/src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java
      Move the RegionInfo convertion up into HRegionInfo instead of here.
      Added generic toDelimitedByteArray helper.
      M b/src/main/java/org/apache/hadoop/hbase/protobuf/RequestConverter.java
      Use HRegionInfo convertions instead.
      M b/src/main/java/org/apache/hadoop/hbase/protobuf/ResponseConverter.java
      Use HRegionInfo convertions instead.
      M b/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
      Use new utility writing out .regioninfo files.
      M b/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java
      Formatting.
      M b/src/main/java/org/apache/hadoop/hbase/regionserver/SplitTransaction.java
      M b/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.java
      Range in Reference is no longer public.
      Range in Reference is no longer public.
      M b/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java
      M b/src/main/java/org/apache/hadoop/hbase/security/token/AuthenticationTokenSecretManager.java
      ClusterId got renamed ZKClusterId
      M b/src/main/java/org/apache/hadoop/hbase/util/FSTableDescriptors.java
      Use new serialization utlity in HTD.
      M b/src/main/java/org/apache/hadoop/hbase/util/FSUtils.java
      Generic method for writing dot file content.
      M b/src/main/java/org/apache/hadoop/hbase/util/HBaseFsck.java
      Reference#Range is not public any more
      M b/src/main/java/org/apache/hadoop/hbase/util/Writables.java
      Deprecated getHRegionInfo, etc.
      D b/src/main/java/org/apache/hadoop/hbase/zookeeper/ClusterId.java
      A b/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKClusterId.java
      Rename
      A b/src/main/protobuf/ClusterId.proto
      Added file for ClusterId only since its written to fs and to zk.
      A b/src/main/protobuf/FS.proto
      Protos for fs files.
      M b/src/main/protobuf/ZooKeeper.proto
      Moved ClusterId out to own proto file
      M b/src/main/protobuf/hbase.proto
      Added TableSchema and ColumnFamilySchema

      This addresses bug hbase-5453.
      https://issues.apache.org/jira/browse/hbase-5453

      Diffs (updated)


      src/main/java/org/apache/hadoop/hbase/ClusterId.java PRE-CREATION
      src/main/java/org/apache/hadoop/hbase/HColumnDescriptor.java 5862f15
      src/main/java/org/apache/hadoop/hbase/HRegionInfo.java 8d83ff3
      src/main/java/org/apache/hadoop/hbase/HTableDescriptor.java af89e3e
      src/main/java/org/apache/hadoop/hbase/client/HConnectionManager.java 5cac9af
      src/main/java/org/apache/hadoop/hbase/io/Reference.java 6360059
      src/main/java/org/apache/hadoop/hbase/mapreduce/LoadIncrementalHFiles.java 9e4ada9
      src/main/java/org/apache/hadoop/hbase/master/HMaster.java 947ec5f
      src/main/java/org/apache/hadoop/hbase/master/MasterFileSystem.java 5052878
      src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java ccc964e
      src/main/java/org/apache/hadoop/hbase/protobuf/RequestConverter.java dabfbab
      src/main/java/org/apache/hadoop/hbase/protobuf/ResponseConverter.java 45cb6cf
      src/main/java/org/apache/hadoop/hbase/protobuf/generated/ClusterIdProtos.java PRE-CREATION
      src/main/java/org/apache/hadoop/hbase/protobuf/generated/FSProtos.java PRE-CREATION
      src/main/java/org/apache/hadoop/hbase/protobuf/generated/HBaseProtos.java 058c006
      src/main/java/org/apache/hadoop/hbase/protobuf/generated/ZooKeeperProtos.java 20c7738
      src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java 9f16fee
      src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 6dc0517
      src/main/java/org/apache/hadoop/hbase/regionserver/SplitTransaction.java 6a9f2fe
      src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.java 5e1e16d
      src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java 5050df0
      src/main/java/org/apache/hadoop/hbase/security/token/AuthenticationTokenSecretManager.java 049ed8d
      src/main/java/org/apache/hadoop/hbase/util/FSTableDescriptors.java efb2b84
      src/main/java/org/apache/hadoop/hbase/util/FSUtils.java 3d35d3e
      src/main/java/org/apache/hadoop/hbase/util/HBaseFsck.java 7b4f4a2
      src/main/java/org/apache/hadoop/hbase/util/Writables.java 3d20723
      src/main/java/org/apache/hadoop/hbase/zookeeper/ClusterId.java f804810
      src/main/java/org/apache/hadoop/hbase/zookeeper/ZKClusterId.java PRE-CREATION
      src/main/protobuf/ClusterId.proto PRE-CREATION
      src/main/protobuf/FS.proto PRE-CREATION
      src/main/protobuf/ZooKeeper.proto b72cb28
      src/main/protobuf/hbase.proto 30a4c3f
      src/test/java/org/apache/hadoop/hbase/TestHColumnDescriptor.java e7fa8b2
      src/test/java/org/apache/hadoop/hbase/TestHTableDescriptor.java f7c0cca
      src/test/java/org/apache/hadoop/hbase/catalog/TestCatalogTracker.java 83d8408
      src/test/java/org/apache/hadoop/hbase/master/MockRegionServer.java 69ccc65
      src/test/java/org/apache/hadoop/hbase/master/TestCatalogJanitor.java 1020374
      src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegionInfo.java 6dfba41
      src/test/java/org/apache/hadoop/hbase/regionserver/TestStoreFile.java 988d0bf
      src/test/java/org/apache/hadoop/hbase/util/TestFSUtils.java 339a120

      Diff: https://reviews.apache.org/r/5130/diff

      Testing
      -------

      Thanks,

      Michael

      Show
      jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/5130/ ----------------------------------------------------------- (Updated 2012-05-16 17:02:35.527708) Review request for hbase. Changes ------- Gregory. I addressed your comments. I've changed the format of .regioninfo and .tableinfo. Now instead of serialized Writable followed by toString of the serialized object, instead its just the serialized pb. This removes our having a human readable .regioninfo/.tablinfo file but my guess no one relied on this anyways. Having just serialized content in the file means a check of file length should be enough figuring whether the file properly serialized. If ever a chance that a Writable + its toString + two '\n' characters was equal to a serialized pb, I'd think this a pathological state. If this state is not cleared up 'naturally' by splits or a schema change, then lets deal if it happens. I only need this length-checking in one place on region open. I want to avoid reading the .regioninfo file on region open. The alternative means more load on NN and DNs at region open time which could be problematic at big-bang cluster start (Thinking 500 nodes w/ 80k regions, an actual known case – this is the case I have in mind when I'm trying to avoid more load on NN/DNs). Otherwise, Gregory's comments led to me to check and I was missing convertion of fs files to pb in all cases (I was just reading the clusterid and hbase.version files, not converting if still Writable). This should be fixed now. There are some failing tests still but running by hadoopqa to see what it says anyways. Also putting up on rb to get feedback if problem w/ this approach. Summary ------- A b/src/main/java/org/apache/hadoop/hbase/ClusterId.java New class to hold clusterid in. M b/src/main/java/org/apache/hadoop/hbase/HColumnDescriptor.java Make it so can do pb serialization. Deprecated Writable serialization. M b/src/main/java/org/apache/hadoop/hbase/HRegionInfo.java Make it so methods in here follow the pattern in HCD an HTD pb 'ing. Deprecated Writable serialization. M b/src/main/java/org/apache/hadoop/hbase/HTableDescriptor.java Make it so can do pb serialization. Deprecated Writable serialization. M b/src/main/java/org/apache/hadoop/hbase/client/HConnectionManager.java ClusterId under ZK got renamed as ZKClusterId M b/src/main/java/org/apache/hadoop/hbase/io/Reference.java Hide the Reference#Range enums. Don't let them out of this class. Make it so can do pb serialization. M b/src/main/java/org/apache/hadoop/hbase/mapreduce/LoadIncrementalHFiles.java Use new methods on Reference for getting top and bottom. M b/src/main/java/org/apache/hadoop/hbase/master/HMaster.java ClusterId under zk has been renamed ZKClusterId. Use new ClusterId class too. M b/src/main/java/org/apache/hadoop/hbase/master/MasterFileSystem.java Use new clusterid class. M b/src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java Move the RegionInfo convertion up into HRegionInfo instead of here. Added generic toDelimitedByteArray helper. M b/src/main/java/org/apache/hadoop/hbase/protobuf/RequestConverter.java Use HRegionInfo convertions instead. M b/src/main/java/org/apache/hadoop/hbase/protobuf/ResponseConverter.java Use HRegionInfo convertions instead. M b/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java Use new utility writing out .regioninfo files. M b/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java Formatting. M b/src/main/java/org/apache/hadoop/hbase/regionserver/SplitTransaction.java M b/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.java Range in Reference is no longer public. Range in Reference is no longer public. M b/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java M b/src/main/java/org/apache/hadoop/hbase/security/token/AuthenticationTokenSecretManager.java ClusterId got renamed ZKClusterId M b/src/main/java/org/apache/hadoop/hbase/util/FSTableDescriptors.java Use new serialization utlity in HTD. M b/src/main/java/org/apache/hadoop/hbase/util/FSUtils.java Generic method for writing dot file content. M b/src/main/java/org/apache/hadoop/hbase/util/HBaseFsck.java Reference#Range is not public any more M b/src/main/java/org/apache/hadoop/hbase/util/Writables.java Deprecated getHRegionInfo, etc. D b/src/main/java/org/apache/hadoop/hbase/zookeeper/ClusterId.java A b/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKClusterId.java Rename A b/src/main/protobuf/ClusterId.proto Added file for ClusterId only since its written to fs and to zk. A b/src/main/protobuf/FS.proto Protos for fs files. M b/src/main/protobuf/ZooKeeper.proto Moved ClusterId out to own proto file M b/src/main/protobuf/hbase.proto Added TableSchema and ColumnFamilySchema This addresses bug hbase-5453. https://issues.apache.org/jira/browse/hbase-5453 Diffs (updated) src/main/java/org/apache/hadoop/hbase/ClusterId.java PRE-CREATION src/main/java/org/apache/hadoop/hbase/HColumnDescriptor.java 5862f15 src/main/java/org/apache/hadoop/hbase/HRegionInfo.java 8d83ff3 src/main/java/org/apache/hadoop/hbase/HTableDescriptor.java af89e3e src/main/java/org/apache/hadoop/hbase/client/HConnectionManager.java 5cac9af src/main/java/org/apache/hadoop/hbase/io/Reference.java 6360059 src/main/java/org/apache/hadoop/hbase/mapreduce/LoadIncrementalHFiles.java 9e4ada9 src/main/java/org/apache/hadoop/hbase/master/HMaster.java 947ec5f src/main/java/org/apache/hadoop/hbase/master/MasterFileSystem.java 5052878 src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java ccc964e src/main/java/org/apache/hadoop/hbase/protobuf/RequestConverter.java dabfbab src/main/java/org/apache/hadoop/hbase/protobuf/ResponseConverter.java 45cb6cf src/main/java/org/apache/hadoop/hbase/protobuf/generated/ClusterIdProtos.java PRE-CREATION src/main/java/org/apache/hadoop/hbase/protobuf/generated/FSProtos.java PRE-CREATION src/main/java/org/apache/hadoop/hbase/protobuf/generated/HBaseProtos.java 058c006 src/main/java/org/apache/hadoop/hbase/protobuf/generated/ZooKeeperProtos.java 20c7738 src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java 9f16fee src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 6dc0517 src/main/java/org/apache/hadoop/hbase/regionserver/SplitTransaction.java 6a9f2fe src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.java 5e1e16d src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java 5050df0 src/main/java/org/apache/hadoop/hbase/security/token/AuthenticationTokenSecretManager.java 049ed8d src/main/java/org/apache/hadoop/hbase/util/FSTableDescriptors.java efb2b84 src/main/java/org/apache/hadoop/hbase/util/FSUtils.java 3d35d3e src/main/java/org/apache/hadoop/hbase/util/HBaseFsck.java 7b4f4a2 src/main/java/org/apache/hadoop/hbase/util/Writables.java 3d20723 src/main/java/org/apache/hadoop/hbase/zookeeper/ClusterId.java f804810 src/main/java/org/apache/hadoop/hbase/zookeeper/ZKClusterId.java PRE-CREATION src/main/protobuf/ClusterId.proto PRE-CREATION src/main/protobuf/FS.proto PRE-CREATION src/main/protobuf/ZooKeeper.proto b72cb28 src/main/protobuf/hbase.proto 30a4c3f src/test/java/org/apache/hadoop/hbase/TestHColumnDescriptor.java e7fa8b2 src/test/java/org/apache/hadoop/hbase/TestHTableDescriptor.java f7c0cca src/test/java/org/apache/hadoop/hbase/catalog/TestCatalogTracker.java 83d8408 src/test/java/org/apache/hadoop/hbase/master/MockRegionServer.java 69ccc65 src/test/java/org/apache/hadoop/hbase/master/TestCatalogJanitor.java 1020374 src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegionInfo.java 6dfba41 src/test/java/org/apache/hadoop/hbase/regionserver/TestStoreFile.java 988d0bf src/test/java/org/apache/hadoop/hbase/util/TestFSUtils.java 339a120 Diff: https://reviews.apache.org/r/5130/diff Testing ------- Thanks, Michael
      Hide
      stack added a comment -

      Fix bug in my Reference file serialize/deserialize

      Show
      stack added a comment - Fix bug in my Reference file serialize/deserialize
      Hide
      jiraposter@reviews.apache.org added a comment -

      -----------------------------------------------------------
      This is an automatically generated e-mail. To reply, visit:
      https://reviews.apache.org/r/5130/#review7944
      -----------------------------------------------------------

      In a previous comment you said (about the HTableDescriptor/HColumnDesriptor pb stuff):
      "Well, Andrew beat us both to it over in his REST pb stuff. We need to reconcile his w/ ours too...."

      Where is his stuff? I couldn't find it. Should we create a JIRA about reconciling? It would be nice to have something, however imperfect, up in trunk to work against, then we could fix up later.

      src/main/java/org/apache/hadoop/hbase/HTableDescriptor.java
      <https://reviews.apache.org/r/5130/#comment17274>

      This isn't ever read?

      src/main/java/org/apache/hadoop/hbase/HTableDescriptor.java
      <https://reviews.apache.org/r/5130/#comment17273>

      This constructor sets pbMade to false, but it should be true in this case, right?

      • Gregory

      On 2012-05-16 17:02:35, Michael Stack wrote:

      -----------------------------------------------------------

      This is an automatically generated e-mail. To reply, visit:

      https://reviews.apache.org/r/5130/

      -----------------------------------------------------------

      (Updated 2012-05-16 17:02:35)

      Review request for hbase.

      Summary

      -------

      A b/src/main/java/org/apache/hadoop/hbase/ClusterId.java

      New class to hold clusterid in.

      M b/src/main/java/org/apache/hadoop/hbase/HColumnDescriptor.java

      Make it so can do pb serialization. Deprecated Writable serialization.

      M b/src/main/java/org/apache/hadoop/hbase/HRegionInfo.java

      Make it so methods in here follow the pattern in HCD an HTD pb 'ing.

      Deprecated Writable serialization.

      M b/src/main/java/org/apache/hadoop/hbase/HTableDescriptor.java

      Make it so can do pb serialization. Deprecated Writable serialization.

      M b/src/main/java/org/apache/hadoop/hbase/client/HConnectionManager.java

      ClusterId under ZK got renamed as ZKClusterId

      M b/src/main/java/org/apache/hadoop/hbase/io/Reference.java

      Hide the Reference#Range enums. Don't let them out of this class.

      Make it so can do pb serialization.

      M b/src/main/java/org/apache/hadoop/hbase/mapreduce/LoadIncrementalHFiles.java

      Use new methods on Reference for getting top and bottom.

      M b/src/main/java/org/apache/hadoop/hbase/master/HMaster.java

      ClusterId under zk has been renamed ZKClusterId.

      Use new ClusterId class too.

      M b/src/main/java/org/apache/hadoop/hbase/master/MasterFileSystem.java

      Use new clusterid class.

      M b/src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java

      Move the RegionInfo convertion up into HRegionInfo instead of here.

      Added generic toDelimitedByteArray helper.

      M b/src/main/java/org/apache/hadoop/hbase/protobuf/RequestConverter.java

      Use HRegionInfo convertions instead.

      M b/src/main/java/org/apache/hadoop/hbase/protobuf/ResponseConverter.java

      Use HRegionInfo convertions instead.

      M b/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java

      Use new utility writing out .regioninfo files.

      M b/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java

      Formatting.

      M b/src/main/java/org/apache/hadoop/hbase/regionserver/SplitTransaction.java

      M b/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.java

      Range in Reference is no longer public.

      Range in Reference is no longer public.

      M b/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java

      M b/src/main/java/org/apache/hadoop/hbase/security/token/AuthenticationTokenSecretManager.java

      ClusterId got renamed ZKClusterId

      M b/src/main/java/org/apache/hadoop/hbase/util/FSTableDescriptors.java

      Use new serialization utlity in HTD.

      M b/src/main/java/org/apache/hadoop/hbase/util/FSUtils.java

      Generic method for writing dot file content.

      M b/src/main/java/org/apache/hadoop/hbase/util/HBaseFsck.java

      Reference#Range is not public any more

      M b/src/main/java/org/apache/hadoop/hbase/util/Writables.java

      Deprecated getHRegionInfo, etc.

      D b/src/main/java/org/apache/hadoop/hbase/zookeeper/ClusterId.java

      A b/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKClusterId.java

      Rename

      A b/src/main/protobuf/ClusterId.proto

      Added file for ClusterId only since its written to fs and to zk.

      A b/src/main/protobuf/FS.proto

      Protos for fs files.

      M b/src/main/protobuf/ZooKeeper.proto

      Moved ClusterId out to own proto file

      M b/src/main/protobuf/hbase.proto

      Added TableSchema and ColumnFamilySchema

      This addresses bug hbase-5453.

      https://issues.apache.org/jira/browse/hbase-5453

      Diffs

      -----

      src/main/java/org/apache/hadoop/hbase/ClusterId.java PRE-CREATION

      src/main/java/org/apache/hadoop/hbase/HColumnDescriptor.java 5862f15

      src/main/java/org/apache/hadoop/hbase/HRegionInfo.java 8d83ff3

      src/main/java/org/apache/hadoop/hbase/HTableDescriptor.java af89e3e

      src/main/java/org/apache/hadoop/hbase/client/HConnectionManager.java 5cac9af

      src/main/java/org/apache/hadoop/hbase/io/Reference.java 6360059

      src/main/java/org/apache/hadoop/hbase/mapreduce/LoadIncrementalHFiles.java 9e4ada9

      src/main/java/org/apache/hadoop/hbase/master/HMaster.java 947ec5f

      src/main/java/org/apache/hadoop/hbase/master/MasterFileSystem.java 5052878

      src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java ccc964e

      src/main/java/org/apache/hadoop/hbase/protobuf/RequestConverter.java dabfbab

      src/main/java/org/apache/hadoop/hbase/protobuf/ResponseConverter.java 45cb6cf

      src/main/java/org/apache/hadoop/hbase/protobuf/generated/ClusterIdProtos.java PRE-CREATION

      src/main/java/org/apache/hadoop/hbase/protobuf/generated/FSProtos.java PRE-CREATION

      src/main/java/org/apache/hadoop/hbase/protobuf/generated/HBaseProtos.java 058c006

      src/main/java/org/apache/hadoop/hbase/protobuf/generated/ZooKeeperProtos.java 20c7738

      src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java 9f16fee

      src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 6dc0517

      src/main/java/org/apache/hadoop/hbase/regionserver/SplitTransaction.java 6a9f2fe

      src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.java 5e1e16d

      src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java 5050df0

      src/main/java/org/apache/hadoop/hbase/security/token/AuthenticationTokenSecretManager.java 049ed8d

      src/main/java/org/apache/hadoop/hbase/util/FSTableDescriptors.java efb2b84

      src/main/java/org/apache/hadoop/hbase/util/FSUtils.java 3d35d3e

      src/main/java/org/apache/hadoop/hbase/util/HBaseFsck.java 7b4f4a2

      src/main/java/org/apache/hadoop/hbase/util/Writables.java 3d20723

      src/main/java/org/apache/hadoop/hbase/zookeeper/ClusterId.java f804810

      src/main/java/org/apache/hadoop/hbase/zookeeper/ZKClusterId.java PRE-CREATION

      src/main/protobuf/ClusterId.proto PRE-CREATION

      src/main/protobuf/FS.proto PRE-CREATION

      src/main/protobuf/ZooKeeper.proto b72cb28

      src/main/protobuf/hbase.proto 30a4c3f

      src/test/java/org/apache/hadoop/hbase/TestHColumnDescriptor.java e7fa8b2

      src/test/java/org/apache/hadoop/hbase/TestHTableDescriptor.java f7c0cca

      src/test/java/org/apache/hadoop/hbase/catalog/TestCatalogTracker.java 83d8408

      src/test/java/org/apache/hadoop/hbase/master/MockRegionServer.java 69ccc65

      src/test/java/org/apache/hadoop/hbase/master/TestCatalogJanitor.java 1020374

      src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegionInfo.java 6dfba41

      src/test/java/org/apache/hadoop/hbase/regionserver/TestStoreFile.java 988d0bf

      src/test/java/org/apache/hadoop/hbase/util/TestFSUtils.java 339a120

      Diff: https://reviews.apache.org/r/5130/diff

      Testing

      -------

      Thanks,

      Michael

      Show
      jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/5130/#review7944 ----------------------------------------------------------- In a previous comment you said (about the HTableDescriptor/HColumnDesriptor pb stuff): "Well, Andrew beat us both to it over in his REST pb stuff. We need to reconcile his w/ ours too...." Where is his stuff? I couldn't find it. Should we create a JIRA about reconciling? It would be nice to have something, however imperfect, up in trunk to work against, then we could fix up later. src/main/java/org/apache/hadoop/hbase/HTableDescriptor.java < https://reviews.apache.org/r/5130/#comment17274 > This isn't ever read? src/main/java/org/apache/hadoop/hbase/HTableDescriptor.java < https://reviews.apache.org/r/5130/#comment17273 > This constructor sets pbMade to false, but it should be true in this case, right? Gregory On 2012-05-16 17:02:35, Michael Stack wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/5130/ ----------------------------------------------------------- (Updated 2012-05-16 17:02:35) Review request for hbase. Summary ------- A b/src/main/java/org/apache/hadoop/hbase/ClusterId.java New class to hold clusterid in. M b/src/main/java/org/apache/hadoop/hbase/HColumnDescriptor.java Make it so can do pb serialization. Deprecated Writable serialization. M b/src/main/java/org/apache/hadoop/hbase/HRegionInfo.java Make it so methods in here follow the pattern in HCD an HTD pb 'ing. Deprecated Writable serialization. M b/src/main/java/org/apache/hadoop/hbase/HTableDescriptor.java Make it so can do pb serialization. Deprecated Writable serialization. M b/src/main/java/org/apache/hadoop/hbase/client/HConnectionManager.java ClusterId under ZK got renamed as ZKClusterId M b/src/main/java/org/apache/hadoop/hbase/io/Reference.java Hide the Reference#Range enums. Don't let them out of this class. Make it so can do pb serialization. M b/src/main/java/org/apache/hadoop/hbase/mapreduce/LoadIncrementalHFiles.java Use new methods on Reference for getting top and bottom. M b/src/main/java/org/apache/hadoop/hbase/master/HMaster.java ClusterId under zk has been renamed ZKClusterId. Use new ClusterId class too. M b/src/main/java/org/apache/hadoop/hbase/master/MasterFileSystem.java Use new clusterid class. M b/src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java Move the RegionInfo convertion up into HRegionInfo instead of here. Added generic toDelimitedByteArray helper. M b/src/main/java/org/apache/hadoop/hbase/protobuf/RequestConverter.java Use HRegionInfo convertions instead. M b/src/main/java/org/apache/hadoop/hbase/protobuf/ResponseConverter.java Use HRegionInfo convertions instead. M b/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java Use new utility writing out .regioninfo files. M b/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java Formatting. M b/src/main/java/org/apache/hadoop/hbase/regionserver/SplitTransaction.java M b/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.java Range in Reference is no longer public. Range in Reference is no longer public. M b/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java M b/src/main/java/org/apache/hadoop/hbase/security/token/AuthenticationTokenSecretManager.java ClusterId got renamed ZKClusterId M b/src/main/java/org/apache/hadoop/hbase/util/FSTableDescriptors.java Use new serialization utlity in HTD. M b/src/main/java/org/apache/hadoop/hbase/util/FSUtils.java Generic method for writing dot file content. M b/src/main/java/org/apache/hadoop/hbase/util/HBaseFsck.java Reference#Range is not public any more M b/src/main/java/org/apache/hadoop/hbase/util/Writables.java Deprecated getHRegionInfo, etc. D b/src/main/java/org/apache/hadoop/hbase/zookeeper/ClusterId.java A b/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKClusterId.java Rename A b/src/main/protobuf/ClusterId.proto Added file for ClusterId only since its written to fs and to zk. A b/src/main/protobuf/FS.proto Protos for fs files. M b/src/main/protobuf/ZooKeeper.proto Moved ClusterId out to own proto file M b/src/main/protobuf/hbase.proto Added TableSchema and ColumnFamilySchema This addresses bug hbase-5453. https://issues.apache.org/jira/browse/hbase-5453 Diffs ----- src/main/java/org/apache/hadoop/hbase/ClusterId.java PRE-CREATION src/main/java/org/apache/hadoop/hbase/HColumnDescriptor.java 5862f15 src/main/java/org/apache/hadoop/hbase/HRegionInfo.java 8d83ff3 src/main/java/org/apache/hadoop/hbase/HTableDescriptor.java af89e3e src/main/java/org/apache/hadoop/hbase/client/HConnectionManager.java 5cac9af src/main/java/org/apache/hadoop/hbase/io/Reference.java 6360059 src/main/java/org/apache/hadoop/hbase/mapreduce/LoadIncrementalHFiles.java 9e4ada9 src/main/java/org/apache/hadoop/hbase/master/HMaster.java 947ec5f src/main/java/org/apache/hadoop/hbase/master/MasterFileSystem.java 5052878 src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java ccc964e src/main/java/org/apache/hadoop/hbase/protobuf/RequestConverter.java dabfbab src/main/java/org/apache/hadoop/hbase/protobuf/ResponseConverter.java 45cb6cf src/main/java/org/apache/hadoop/hbase/protobuf/generated/ClusterIdProtos.java PRE-CREATION src/main/java/org/apache/hadoop/hbase/protobuf/generated/FSProtos.java PRE-CREATION src/main/java/org/apache/hadoop/hbase/protobuf/generated/HBaseProtos.java 058c006 src/main/java/org/apache/hadoop/hbase/protobuf/generated/ZooKeeperProtos.java 20c7738 src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java 9f16fee src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 6dc0517 src/main/java/org/apache/hadoop/hbase/regionserver/SplitTransaction.java 6a9f2fe src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.java 5e1e16d src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java 5050df0 src/main/java/org/apache/hadoop/hbase/security/token/AuthenticationTokenSecretManager.java 049ed8d src/main/java/org/apache/hadoop/hbase/util/FSTableDescriptors.java efb2b84 src/main/java/org/apache/hadoop/hbase/util/FSUtils.java 3d35d3e src/main/java/org/apache/hadoop/hbase/util/HBaseFsck.java 7b4f4a2 src/main/java/org/apache/hadoop/hbase/util/Writables.java 3d20723 src/main/java/org/apache/hadoop/hbase/zookeeper/ClusterId.java f804810 src/main/java/org/apache/hadoop/hbase/zookeeper/ZKClusterId.java PRE-CREATION src/main/protobuf/ClusterId.proto PRE-CREATION src/main/protobuf/FS.proto PRE-CREATION src/main/protobuf/ZooKeeper.proto b72cb28 src/main/protobuf/hbase.proto 30a4c3f src/test/java/org/apache/hadoop/hbase/TestHColumnDescriptor.java e7fa8b2 src/test/java/org/apache/hadoop/hbase/TestHTableDescriptor.java f7c0cca src/test/java/org/apache/hadoop/hbase/catalog/TestCatalogTracker.java 83d8408 src/test/java/org/apache/hadoop/hbase/master/MockRegionServer.java 69ccc65 src/test/java/org/apache/hadoop/hbase/master/TestCatalogJanitor.java 1020374 src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegionInfo.java 6dfba41 src/test/java/org/apache/hadoop/hbase/regionserver/TestStoreFile.java 988d0bf src/test/java/org/apache/hadoop/hbase/util/TestFSUtils.java 339a120 Diff: https://reviews.apache.org/r/5130/diff Testing ------- Thanks, Michael
      Hide
      jiraposter@reviews.apache.org added a comment -

      On 2012-05-16 22:12:10, Gregory Chanan wrote:

      > In a previous comment you said (about the HTableDescriptor/HColumnDesriptor pb stuff):

      > "Well, Andrew beat us both to it over in his REST pb stuff. We need to reconcile his w/ ours too...."

      >

      > Where is his stuff? I couldn't find it. Should we create a JIRA about reconciling? It would be nice to have something, however imperfect, up in trunk to work against, then we could fix up later.

      Its under src/main/resources/org.... I just tripped over it myself yesterday. I made HBASE-6026 to do the reconcile (after HBASE-6000 goes in).

      Are you down w/ the change in .regioninfo Gregory as means of 'ensuring' we don't have a Writable and pb serialization end up as same size?

      Thanks for review.

      On 2012-05-16 22:12:10, Gregory Chanan wrote:

      > src/main/java/org/apache/hadoop/hbase/HTableDescriptor.java, line 276

      > <https://reviews.apache.org/r/5130/diff/1-2/?file=109037#file109037line276>

      >

      > This isn't ever read?

      This is gone now. I used another technique figuring if object serialized – read file fully into byte array and test for the pb prefix – rather than this hacky setting attribute on class.

      • Michael

      -----------------------------------------------------------
      This is an automatically generated e-mail. To reply, visit:
      https://reviews.apache.org/r/5130/#review7944
      -----------------------------------------------------------

      On 2012-05-16 17:02:35, Michael Stack wrote:

      -----------------------------------------------------------

      This is an automatically generated e-mail. To reply, visit:

      https://reviews.apache.org/r/5130/

      -----------------------------------------------------------

      (Updated 2012-05-16 17:02:35)

      Review request for hbase.

      Summary

      -------

      A b/src/main/java/org/apache/hadoop/hbase/ClusterId.java

      New class to hold clusterid in.

      M b/src/main/java/org/apache/hadoop/hbase/HColumnDescriptor.java

      Make it so can do pb serialization. Deprecated Writable serialization.

      M b/src/main/java/org/apache/hadoop/hbase/HRegionInfo.java

      Make it so methods in here follow the pattern in HCD an HTD pb 'ing.

      Deprecated Writable serialization.

      M b/src/main/java/org/apache/hadoop/hbase/HTableDescriptor.java

      Make it so can do pb serialization. Deprecated Writable serialization.

      M b/src/main/java/org/apache/hadoop/hbase/client/HConnectionManager.java

      ClusterId under ZK got renamed as ZKClusterId

      M b/src/main/java/org/apache/hadoop/hbase/io/Reference.java

      Hide the Reference#Range enums. Don't let them out of this class.

      Make it so can do pb serialization.

      M b/src/main/java/org/apache/hadoop/hbase/mapreduce/LoadIncrementalHFiles.java

      Use new methods on Reference for getting top and bottom.

      M b/src/main/java/org/apache/hadoop/hbase/master/HMaster.java

      ClusterId under zk has been renamed ZKClusterId.

      Use new ClusterId class too.

      M b/src/main/java/org/apache/hadoop/hbase/master/MasterFileSystem.java

      Use new clusterid class.

      M b/src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java

      Move the RegionInfo convertion up into HRegionInfo instead of here.

      Added generic toDelimitedByteArray helper.

      M b/src/main/java/org/apache/hadoop/hbase/protobuf/RequestConverter.java

      Use HRegionInfo convertions instead.

      M b/src/main/java/org/apache/hadoop/hbase/protobuf/ResponseConverter.java

      Use HRegionInfo convertions instead.

      M b/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java

      Use new utility writing out .regioninfo files.

      M b/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java

      Formatting.

      M b/src/main/java/org/apache/hadoop/hbase/regionserver/SplitTransaction.java

      M b/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.java

      Range in Reference is no longer public.

      Range in Reference is no longer public.

      M b/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java

      M b/src/main/java/org/apache/hadoop/hbase/security/token/AuthenticationTokenSecretManager.java

      ClusterId got renamed ZKClusterId

      M b/src/main/java/org/apache/hadoop/hbase/util/FSTableDescriptors.java

      Use new serialization utlity in HTD.

      M b/src/main/java/org/apache/hadoop/hbase/util/FSUtils.java

      Generic method for writing dot file content.

      M b/src/main/java/org/apache/hadoop/hbase/util/HBaseFsck.java

      Reference#Range is not public any more

      M b/src/main/java/org/apache/hadoop/hbase/util/Writables.java

      Deprecated getHRegionInfo, etc.

      D b/src/main/java/org/apache/hadoop/hbase/zookeeper/ClusterId.java

      A b/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKClusterId.java

      Rename

      A b/src/main/protobuf/ClusterId.proto

      Added file for ClusterId only since its written to fs and to zk.

      A b/src/main/protobuf/FS.proto

      Protos for fs files.

      M b/src/main/protobuf/ZooKeeper.proto

      Moved ClusterId out to own proto file

      M b/src/main/protobuf/hbase.proto

      Added TableSchema and ColumnFamilySchema

      This addresses bug hbase-5453.

      https://issues.apache.org/jira/browse/hbase-5453

      Diffs

      -----

      src/main/java/org/apache/hadoop/hbase/ClusterId.java PRE-CREATION

      src/main/java/org/apache/hadoop/hbase/HColumnDescriptor.java 5862f15

      src/main/java/org/apache/hadoop/hbase/HRegionInfo.java 8d83ff3

      src/main/java/org/apache/hadoop/hbase/HTableDescriptor.java af89e3e

      src/main/java/org/apache/hadoop/hbase/client/HConnectionManager.java 5cac9af

      src/main/java/org/apache/hadoop/hbase/io/Reference.java 6360059

      src/main/java/org/apache/hadoop/hbase/mapreduce/LoadIncrementalHFiles.java 9e4ada9

      src/main/java/org/apache/hadoop/hbase/master/HMaster.java 947ec5f

      src/main/java/org/apache/hadoop/hbase/master/MasterFileSystem.java 5052878

      src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java ccc964e

      src/main/java/org/apache/hadoop/hbase/protobuf/RequestConverter.java dabfbab

      src/main/java/org/apache/hadoop/hbase/protobuf/ResponseConverter.java 45cb6cf

      src/main/java/org/apache/hadoop/hbase/protobuf/generated/ClusterIdProtos.java PRE-CREATION

      src/main/java/org/apache/hadoop/hbase/protobuf/generated/FSProtos.java PRE-CREATION

      src/main/java/org/apache/hadoop/hbase/protobuf/generated/HBaseProtos.java 058c006

      src/main/java/org/apache/hadoop/hbase/protobuf/generated/ZooKeeperProtos.java 20c7738

      src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java 9f16fee

      src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 6dc0517

      src/main/java/org/apache/hadoop/hbase/regionserver/SplitTransaction.java 6a9f2fe

      src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.java 5e1e16d

      src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java 5050df0

      src/main/java/org/apache/hadoop/hbase/security/token/AuthenticationTokenSecretManager.java 049ed8d

      src/main/java/org/apache/hadoop/hbase/util/FSTableDescriptors.java efb2b84

      src/main/java/org/apache/hadoop/hbase/util/FSUtils.java 3d35d3e

      src/main/java/org/apache/hadoop/hbase/util/HBaseFsck.java 7b4f4a2

      src/main/java/org/apache/hadoop/hbase/util/Writables.java 3d20723

      src/main/java/org/apache/hadoop/hbase/zookeeper/ClusterId.java f804810

      src/main/java/org/apache/hadoop/hbase/zookeeper/ZKClusterId.java PRE-CREATION

      src/main/protobuf/ClusterId.proto PRE-CREATION

      src/main/protobuf/FS.proto PRE-CREATION

      src/main/protobuf/ZooKeeper.proto b72cb28

      src/main/protobuf/hbase.proto 30a4c3f

      src/test/java/org/apache/hadoop/hbase/TestHColumnDescriptor.java e7fa8b2

      src/test/java/org/apache/hadoop/hbase/TestHTableDescriptor.java f7c0cca

      src/test/java/org/apache/hadoop/hbase/catalog/TestCatalogTracker.java 83d8408

      src/test/java/org/apache/hadoop/hbase/master/MockRegionServer.java 69ccc65

      src/test/java/org/apache/hadoop/hbase/master/TestCatalogJanitor.java 1020374

      src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegionInfo.java 6dfba41

      src/test/java/org/apache/hadoop/hbase/regionserver/TestStoreFile.java 988d0bf

      src/test/java/org/apache/hadoop/hbase/util/TestFSUtils.java 339a120

      Diff: https://reviews.apache.org/r/5130/diff

      Testing

      -------

      Thanks,

      Michael

      Show
      jiraposter@reviews.apache.org added a comment - On 2012-05-16 22:12:10, Gregory Chanan wrote: > In a previous comment you said (about the HTableDescriptor/HColumnDesriptor pb stuff): > "Well, Andrew beat us both to it over in his REST pb stuff. We need to reconcile his w/ ours too...." > > Where is his stuff? I couldn't find it. Should we create a JIRA about reconciling? It would be nice to have something, however imperfect, up in trunk to work against, then we could fix up later. Its under src/main/resources/org.... I just tripped over it myself yesterday. I made HBASE-6026 to do the reconcile (after HBASE-6000 goes in). Are you down w/ the change in .regioninfo Gregory as means of 'ensuring' we don't have a Writable and pb serialization end up as same size? Thanks for review. On 2012-05-16 22:12:10, Gregory Chanan wrote: > src/main/java/org/apache/hadoop/hbase/HTableDescriptor.java, line 276 > < https://reviews.apache.org/r/5130/diff/1-2/?file=109037#file109037line276 > > > This isn't ever read? This is gone now. I used another technique figuring if object serialized – read file fully into byte array and test for the pb prefix – rather than this hacky setting attribute on class. Michael ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/5130/#review7944 ----------------------------------------------------------- On 2012-05-16 17:02:35, Michael Stack wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/5130/ ----------------------------------------------------------- (Updated 2012-05-16 17:02:35) Review request for hbase. Summary ------- A b/src/main/java/org/apache/hadoop/hbase/ClusterId.java New class to hold clusterid in. M b/src/main/java/org/apache/hadoop/hbase/HColumnDescriptor.java Make it so can do pb serialization. Deprecated Writable serialization. M b/src/main/java/org/apache/hadoop/hbase/HRegionInfo.java Make it so methods in here follow the pattern in HCD an HTD pb 'ing. Deprecated Writable serialization. M b/src/main/java/org/apache/hadoop/hbase/HTableDescriptor.java Make it so can do pb serialization. Deprecated Writable serialization. M b/src/main/java/org/apache/hadoop/hbase/client/HConnectionManager.java ClusterId under ZK got renamed as ZKClusterId M b/src/main/java/org/apache/hadoop/hbase/io/Reference.java Hide the Reference#Range enums. Don't let them out of this class. Make it so can do pb serialization. M b/src/main/java/org/apache/hadoop/hbase/mapreduce/LoadIncrementalHFiles.java Use new methods on Reference for getting top and bottom. M b/src/main/java/org/apache/hadoop/hbase/master/HMaster.java ClusterId under zk has been renamed ZKClusterId. Use new ClusterId class too. M b/src/main/java/org/apache/hadoop/hbase/master/MasterFileSystem.java Use new clusterid class. M b/src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java Move the RegionInfo convertion up into HRegionInfo instead of here. Added generic toDelimitedByteArray helper. M b/src/main/java/org/apache/hadoop/hbase/protobuf/RequestConverter.java Use HRegionInfo convertions instead. M b/src/main/java/org/apache/hadoop/hbase/protobuf/ResponseConverter.java Use HRegionInfo convertions instead. M b/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java Use new utility writing out .regioninfo files. M b/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java Formatting. M b/src/main/java/org/apache/hadoop/hbase/regionserver/SplitTransaction.java M b/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.java Range in Reference is no longer public. Range in Reference is no longer public. M b/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java M b/src/main/java/org/apache/hadoop/hbase/security/token/AuthenticationTokenSecretManager.java ClusterId got renamed ZKClusterId M b/src/main/java/org/apache/hadoop/hbase/util/FSTableDescriptors.java Use new serialization utlity in HTD. M b/src/main/java/org/apache/hadoop/hbase/util/FSUtils.java Generic method for writing dot file content. M b/src/main/java/org/apache/hadoop/hbase/util/HBaseFsck.java Reference#Range is not public any more M b/src/main/java/org/apache/hadoop/hbase/util/Writables.java Deprecated getHRegionInfo, etc. D b/src/main/java/org/apache/hadoop/hbase/zookeeper/ClusterId.java A b/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKClusterId.java Rename A b/src/main/protobuf/ClusterId.proto Added file for ClusterId only since its written to fs and to zk. A b/src/main/protobuf/FS.proto Protos for fs files. M b/src/main/protobuf/ZooKeeper.proto Moved ClusterId out to own proto file M b/src/main/protobuf/hbase.proto Added TableSchema and ColumnFamilySchema This addresses bug hbase-5453. https://issues.apache.org/jira/browse/hbase-5453 Diffs ----- src/main/java/org/apache/hadoop/hbase/ClusterId.java PRE-CREATION src/main/java/org/apache/hadoop/hbase/HColumnDescriptor.java 5862f15 src/main/java/org/apache/hadoop/hbase/HRegionInfo.java 8d83ff3 src/main/java/org/apache/hadoop/hbase/HTableDescriptor.java af89e3e src/main/java/org/apache/hadoop/hbase/client/HConnectionManager.java 5cac9af src/main/java/org/apache/hadoop/hbase/io/Reference.java 6360059 src/main/java/org/apache/hadoop/hbase/mapreduce/LoadIncrementalHFiles.java 9e4ada9 src/main/java/org/apache/hadoop/hbase/master/HMaster.java 947ec5f src/main/java/org/apache/hadoop/hbase/master/MasterFileSystem.java 5052878 src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java ccc964e src/main/java/org/apache/hadoop/hbase/protobuf/RequestConverter.java dabfbab src/main/java/org/apache/hadoop/hbase/protobuf/ResponseConverter.java 45cb6cf src/main/java/org/apache/hadoop/hbase/protobuf/generated/ClusterIdProtos.java PRE-CREATION src/main/java/org/apache/hadoop/hbase/protobuf/generated/FSProtos.java PRE-CREATION src/main/java/org/apache/hadoop/hbase/protobuf/generated/HBaseProtos.java 058c006 src/main/java/org/apache/hadoop/hbase/protobuf/generated/ZooKeeperProtos.java 20c7738 src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java 9f16fee src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 6dc0517 src/main/java/org/apache/hadoop/hbase/regionserver/SplitTransaction.java 6a9f2fe src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.java 5e1e16d src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java 5050df0 src/main/java/org/apache/hadoop/hbase/security/token/AuthenticationTokenSecretManager.java 049ed8d src/main/java/org/apache/hadoop/hbase/util/FSTableDescriptors.java efb2b84 src/main/java/org/apache/hadoop/hbase/util/FSUtils.java 3d35d3e src/main/java/org/apache/hadoop/hbase/util/HBaseFsck.java 7b4f4a2 src/main/java/org/apache/hadoop/hbase/util/Writables.java 3d20723 src/main/java/org/apache/hadoop/hbase/zookeeper/ClusterId.java f804810 src/main/java/org/apache/hadoop/hbase/zookeeper/ZKClusterId.java PRE-CREATION src/main/protobuf/ClusterId.proto PRE-CREATION src/main/protobuf/FS.proto PRE-CREATION src/main/protobuf/ZooKeeper.proto b72cb28 src/main/protobuf/hbase.proto 30a4c3f src/test/java/org/apache/hadoop/hbase/TestHColumnDescriptor.java e7fa8b2 src/test/java/org/apache/hadoop/hbase/TestHTableDescriptor.java f7c0cca src/test/java/org/apache/hadoop/hbase/catalog/TestCatalogTracker.java 83d8408 src/test/java/org/apache/hadoop/hbase/master/MockRegionServer.java 69ccc65 src/test/java/org/apache/hadoop/hbase/master/TestCatalogJanitor.java 1020374 src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegionInfo.java 6dfba41 src/test/java/org/apache/hadoop/hbase/regionserver/TestStoreFile.java 988d0bf src/test/java/org/apache/hadoop/hbase/util/TestFSUtils.java 339a120 Diff: https://reviews.apache.org/r/5130/diff Testing ------- Thanks, Michael
      Hide
      jiraposter@reviews.apache.org added a comment -

      -----------------------------------------------------------
      This is an automatically generated e-mail. To reply, visit:
      https://reviews.apache.org/r/5130/
      -----------------------------------------------------------

      (Updated 2012-05-16 23:56:18.496225)

      Review request for hbase.

      Changes
      -------

      v10... the last thing I added to JIRA

      Summary
      -------

      A b/src/main/java/org/apache/hadoop/hbase/ClusterId.java
      New class to hold clusterid in.
      M b/src/main/java/org/apache/hadoop/hbase/HColumnDescriptor.java
      Make it so can do pb serialization. Deprecated Writable serialization.
      M b/src/main/java/org/apache/hadoop/hbase/HRegionInfo.java
      Make it so methods in here follow the pattern in HCD an HTD pb 'ing.
      Deprecated Writable serialization.
      M b/src/main/java/org/apache/hadoop/hbase/HTableDescriptor.java
      Make it so can do pb serialization. Deprecated Writable serialization.
      M b/src/main/java/org/apache/hadoop/hbase/client/HConnectionManager.java
      ClusterId under ZK got renamed as ZKClusterId
      M b/src/main/java/org/apache/hadoop/hbase/io/Reference.java
      Hide the Reference#Range enums. Don't let them out of this class.
      Make it so can do pb serialization.
      M b/src/main/java/org/apache/hadoop/hbase/mapreduce/LoadIncrementalHFiles.java
      Use new methods on Reference for getting top and bottom.
      M b/src/main/java/org/apache/hadoop/hbase/master/HMaster.java
      ClusterId under zk has been renamed ZKClusterId.
      Use new ClusterId class too.
      M b/src/main/java/org/apache/hadoop/hbase/master/MasterFileSystem.java
      Use new clusterid class.
      M b/src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java
      Move the RegionInfo convertion up into HRegionInfo instead of here.
      Added generic toDelimitedByteArray helper.
      M b/src/main/java/org/apache/hadoop/hbase/protobuf/RequestConverter.java
      Use HRegionInfo convertions instead.
      M b/src/main/java/org/apache/hadoop/hbase/protobuf/ResponseConverter.java
      Use HRegionInfo convertions instead.
      M b/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
      Use new utility writing out .regioninfo files.
      M b/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java
      Formatting.
      M b/src/main/java/org/apache/hadoop/hbase/regionserver/SplitTransaction.java
      M b/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.java
      Range in Reference is no longer public.
      Range in Reference is no longer public.
      M b/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java
      M b/src/main/java/org/apache/hadoop/hbase/security/token/AuthenticationTokenSecretManager.java
      ClusterId got renamed ZKClusterId
      M b/src/main/java/org/apache/hadoop/hbase/util/FSTableDescriptors.java
      Use new serialization utlity in HTD.
      M b/src/main/java/org/apache/hadoop/hbase/util/FSUtils.java
      Generic method for writing dot file content.
      M b/src/main/java/org/apache/hadoop/hbase/util/HBaseFsck.java
      Reference#Range is not public any more
      M b/src/main/java/org/apache/hadoop/hbase/util/Writables.java
      Deprecated getHRegionInfo, etc.
      D b/src/main/java/org/apache/hadoop/hbase/zookeeper/ClusterId.java
      A b/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKClusterId.java
      Rename
      A b/src/main/protobuf/ClusterId.proto
      Added file for ClusterId only since its written to fs and to zk.
      A b/src/main/protobuf/FS.proto
      Protos for fs files.
      M b/src/main/protobuf/ZooKeeper.proto
      Moved ClusterId out to own proto file
      M b/src/main/protobuf/hbase.proto
      Added TableSchema and ColumnFamilySchema

      This addresses bug hbase-5453.
      https://issues.apache.org/jira/browse/hbase-5453

      Diffs (updated)


      src/main/java/org/apache/hadoop/hbase/ClusterId.java PRE-CREATION
      src/main/java/org/apache/hadoop/hbase/HColumnDescriptor.java 5862f15
      src/main/java/org/apache/hadoop/hbase/HRegionInfo.java 8d83ff3
      src/main/java/org/apache/hadoop/hbase/HTableDescriptor.java af89e3e
      src/main/java/org/apache/hadoop/hbase/client/HConnectionManager.java 5cac9af
      src/main/java/org/apache/hadoop/hbase/io/Reference.java 6360059
      src/main/java/org/apache/hadoop/hbase/mapreduce/LoadIncrementalHFiles.java 9e4ada9
      src/main/java/org/apache/hadoop/hbase/master/HMaster.java 947ec5f
      src/main/java/org/apache/hadoop/hbase/master/MasterFileSystem.java 5052878
      src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java ccc964e
      src/main/java/org/apache/hadoop/hbase/protobuf/RequestConverter.java dabfbab
      src/main/java/org/apache/hadoop/hbase/protobuf/ResponseConverter.java 45cb6cf
      src/main/java/org/apache/hadoop/hbase/protobuf/generated/ClusterIdProtos.java PRE-CREATION
      src/main/java/org/apache/hadoop/hbase/protobuf/generated/FSProtos.java PRE-CREATION
      src/main/java/org/apache/hadoop/hbase/protobuf/generated/HBaseProtos.java 058c006
      src/main/java/org/apache/hadoop/hbase/protobuf/generated/ZooKeeperProtos.java 20c7738
      src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java 9f16fee
      src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 6dc0517
      src/main/java/org/apache/hadoop/hbase/regionserver/SplitTransaction.java 6a9f2fe
      src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.java 5e1e16d
      src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java 5050df0
      src/main/java/org/apache/hadoop/hbase/security/token/AuthenticationTokenSecretManager.java 049ed8d
      src/main/java/org/apache/hadoop/hbase/util/Bytes.java 2ccda17
      src/main/java/org/apache/hadoop/hbase/util/FSTableDescriptors.java efb2b84
      src/main/java/org/apache/hadoop/hbase/util/FSUtils.java 3d35d3e
      src/main/java/org/apache/hadoop/hbase/util/HBaseFsck.java 7b4f4a2
      src/main/java/org/apache/hadoop/hbase/util/Writables.java 3d20723
      src/main/java/org/apache/hadoop/hbase/zookeeper/ClusterId.java f804810
      src/main/java/org/apache/hadoop/hbase/zookeeper/ZKClusterId.java PRE-CREATION
      src/main/protobuf/ClusterId.proto PRE-CREATION
      src/main/protobuf/FS.proto PRE-CREATION
      src/main/protobuf/ZooKeeper.proto b72cb28
      src/main/protobuf/hbase.proto 30a4c3f
      src/test/java/org/apache/hadoop/hbase/TestHColumnDescriptor.java e7fa8b2
      src/test/java/org/apache/hadoop/hbase/TestHTableDescriptor.java f7c0cca
      src/test/java/org/apache/hadoop/hbase/catalog/TestCatalogTracker.java 83d8408
      src/test/java/org/apache/hadoop/hbase/master/MockRegionServer.java 69ccc65
      src/test/java/org/apache/hadoop/hbase/master/TestCatalogJanitor.java 1020374
      src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegionInfo.java 6dfba41
      src/test/java/org/apache/hadoop/hbase/regionserver/TestSplitTransaction.java 98c09eb
      src/test/java/org/apache/hadoop/hbase/regionserver/TestStoreFile.java 988d0bf
      src/test/java/org/apache/hadoop/hbase/util/TestFSUtils.java 339a120

      Diff: https://reviews.apache.org/r/5130/diff

      Testing
      -------

      Thanks,

      Michael

      Show
      jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/5130/ ----------------------------------------------------------- (Updated 2012-05-16 23:56:18.496225) Review request for hbase. Changes ------- v10... the last thing I added to JIRA Summary ------- A b/src/main/java/org/apache/hadoop/hbase/ClusterId.java New class to hold clusterid in. M b/src/main/java/org/apache/hadoop/hbase/HColumnDescriptor.java Make it so can do pb serialization. Deprecated Writable serialization. M b/src/main/java/org/apache/hadoop/hbase/HRegionInfo.java Make it so methods in here follow the pattern in HCD an HTD pb 'ing. Deprecated Writable serialization. M b/src/main/java/org/apache/hadoop/hbase/HTableDescriptor.java Make it so can do pb serialization. Deprecated Writable serialization. M b/src/main/java/org/apache/hadoop/hbase/client/HConnectionManager.java ClusterId under ZK got renamed as ZKClusterId M b/src/main/java/org/apache/hadoop/hbase/io/Reference.java Hide the Reference#Range enums. Don't let them out of this class. Make it so can do pb serialization. M b/src/main/java/org/apache/hadoop/hbase/mapreduce/LoadIncrementalHFiles.java Use new methods on Reference for getting top and bottom. M b/src/main/java/org/apache/hadoop/hbase/master/HMaster.java ClusterId under zk has been renamed ZKClusterId. Use new ClusterId class too. M b/src/main/java/org/apache/hadoop/hbase/master/MasterFileSystem.java Use new clusterid class. M b/src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java Move the RegionInfo convertion up into HRegionInfo instead of here. Added generic toDelimitedByteArray helper. M b/src/main/java/org/apache/hadoop/hbase/protobuf/RequestConverter.java Use HRegionInfo convertions instead. M b/src/main/java/org/apache/hadoop/hbase/protobuf/ResponseConverter.java Use HRegionInfo convertions instead. M b/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java Use new utility writing out .regioninfo files. M b/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java Formatting. M b/src/main/java/org/apache/hadoop/hbase/regionserver/SplitTransaction.java M b/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.java Range in Reference is no longer public. Range in Reference is no longer public. M b/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java M b/src/main/java/org/apache/hadoop/hbase/security/token/AuthenticationTokenSecretManager.java ClusterId got renamed ZKClusterId M b/src/main/java/org/apache/hadoop/hbase/util/FSTableDescriptors.java Use new serialization utlity in HTD. M b/src/main/java/org/apache/hadoop/hbase/util/FSUtils.java Generic method for writing dot file content. M b/src/main/java/org/apache/hadoop/hbase/util/HBaseFsck.java Reference#Range is not public any more M b/src/main/java/org/apache/hadoop/hbase/util/Writables.java Deprecated getHRegionInfo, etc. D b/src/main/java/org/apache/hadoop/hbase/zookeeper/ClusterId.java A b/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKClusterId.java Rename A b/src/main/protobuf/ClusterId.proto Added file for ClusterId only since its written to fs and to zk. A b/src/main/protobuf/FS.proto Protos for fs files. M b/src/main/protobuf/ZooKeeper.proto Moved ClusterId out to own proto file M b/src/main/protobuf/hbase.proto Added TableSchema and ColumnFamilySchema This addresses bug hbase-5453. https://issues.apache.org/jira/browse/hbase-5453 Diffs (updated) src/main/java/org/apache/hadoop/hbase/ClusterId.java PRE-CREATION src/main/java/org/apache/hadoop/hbase/HColumnDescriptor.java 5862f15 src/main/java/org/apache/hadoop/hbase/HRegionInfo.java 8d83ff3 src/main/java/org/apache/hadoop/hbase/HTableDescriptor.java af89e3e src/main/java/org/apache/hadoop/hbase/client/HConnectionManager.java 5cac9af src/main/java/org/apache/hadoop/hbase/io/Reference.java 6360059 src/main/java/org/apache/hadoop/hbase/mapreduce/LoadIncrementalHFiles.java 9e4ada9 src/main/java/org/apache/hadoop/hbase/master/HMaster.java 947ec5f src/main/java/org/apache/hadoop/hbase/master/MasterFileSystem.java 5052878 src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java ccc964e src/main/java/org/apache/hadoop/hbase/protobuf/RequestConverter.java dabfbab src/main/java/org/apache/hadoop/hbase/protobuf/ResponseConverter.java 45cb6cf src/main/java/org/apache/hadoop/hbase/protobuf/generated/ClusterIdProtos.java PRE-CREATION src/main/java/org/apache/hadoop/hbase/protobuf/generated/FSProtos.java PRE-CREATION src/main/java/org/apache/hadoop/hbase/protobuf/generated/HBaseProtos.java 058c006 src/main/java/org/apache/hadoop/hbase/protobuf/generated/ZooKeeperProtos.java 20c7738 src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java 9f16fee src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 6dc0517 src/main/java/org/apache/hadoop/hbase/regionserver/SplitTransaction.java 6a9f2fe src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.java 5e1e16d src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java 5050df0 src/main/java/org/apache/hadoop/hbase/security/token/AuthenticationTokenSecretManager.java 049ed8d src/main/java/org/apache/hadoop/hbase/util/Bytes.java 2ccda17 src/main/java/org/apache/hadoop/hbase/util/FSTableDescriptors.java efb2b84 src/main/java/org/apache/hadoop/hbase/util/FSUtils.java 3d35d3e src/main/java/org/apache/hadoop/hbase/util/HBaseFsck.java 7b4f4a2 src/main/java/org/apache/hadoop/hbase/util/Writables.java 3d20723 src/main/java/org/apache/hadoop/hbase/zookeeper/ClusterId.java f804810 src/main/java/org/apache/hadoop/hbase/zookeeper/ZKClusterId.java PRE-CREATION src/main/protobuf/ClusterId.proto PRE-CREATION src/main/protobuf/FS.proto PRE-CREATION src/main/protobuf/ZooKeeper.proto b72cb28 src/main/protobuf/hbase.proto 30a4c3f src/test/java/org/apache/hadoop/hbase/TestHColumnDescriptor.java e7fa8b2 src/test/java/org/apache/hadoop/hbase/TestHTableDescriptor.java f7c0cca src/test/java/org/apache/hadoop/hbase/catalog/TestCatalogTracker.java 83d8408 src/test/java/org/apache/hadoop/hbase/master/MockRegionServer.java 69ccc65 src/test/java/org/apache/hadoop/hbase/master/TestCatalogJanitor.java 1020374 src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegionInfo.java 6dfba41 src/test/java/org/apache/hadoop/hbase/regionserver/TestSplitTransaction.java 98c09eb src/test/java/org/apache/hadoop/hbase/regionserver/TestStoreFile.java 988d0bf src/test/java/org/apache/hadoop/hbase/util/TestFSUtils.java 339a120 Diff: https://reviews.apache.org/r/5130/diff Testing ------- Thanks, Michael
      Hide
      jiraposter@reviews.apache.org added a comment -

      -----------------------------------------------------------
      This is an automatically generated e-mail. To reply, visit:
      https://reviews.apache.org/r/5130/#review7951
      -----------------------------------------------------------

      src/main/java/org/apache/hadoop/hbase/HTableDescriptor.java
      <https://reviews.apache.org/r/5130/#comment17291>

      This is still here? Or just reviewboard showing it for some reason?

      src/main/java/org/apache/hadoop/hbase/util/FSUtils.java
      <https://reviews.apache.org/r/5130/#comment17290>

      This is what you are using to ensure the sizes cannot be the same? Very nice!

      I cannot find a call-site for this, though, maybe I missed it?

      • Gregory

      On 2012-05-16 23:56:18, Michael Stack wrote:

      -----------------------------------------------------------

      This is an automatically generated e-mail. To reply, visit:

      https://reviews.apache.org/r/5130/

      -----------------------------------------------------------

      (Updated 2012-05-16 23:56:18)

      Review request for hbase.

      Summary

      -------

      A b/src/main/java/org/apache/hadoop/hbase/ClusterId.java

      New class to hold clusterid in.

      M b/src/main/java/org/apache/hadoop/hbase/HColumnDescriptor.java

      Make it so can do pb serialization. Deprecated Writable serialization.

      M b/src/main/java/org/apache/hadoop/hbase/HRegionInfo.java

      Make it so methods in here follow the pattern in HCD an HTD pb 'ing.

      Deprecated Writable serialization.

      M b/src/main/java/org/apache/hadoop/hbase/HTableDescriptor.java

      Make it so can do pb serialization. Deprecated Writable serialization.

      M b/src/main/java/org/apache/hadoop/hbase/client/HConnectionManager.java

      ClusterId under ZK got renamed as ZKClusterId

      M b/src/main/java/org/apache/hadoop/hbase/io/Reference.java

      Hide the Reference#Range enums. Don't let them out of this class.

      Make it so can do pb serialization.

      M b/src/main/java/org/apache/hadoop/hbase/mapreduce/LoadIncrementalHFiles.java

      Use new methods on Reference for getting top and bottom.

      M b/src/main/java/org/apache/hadoop/hbase/master/HMaster.java

      ClusterId under zk has been renamed ZKClusterId.

      Use new ClusterId class too.

      M b/src/main/java/org/apache/hadoop/hbase/master/MasterFileSystem.java

      Use new clusterid class.

      M b/src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java

      Move the RegionInfo convertion up into HRegionInfo instead of here.

      Added generic toDelimitedByteArray helper.

      M b/src/main/java/org/apache/hadoop/hbase/protobuf/RequestConverter.java

      Use HRegionInfo convertions instead.

      M b/src/main/java/org/apache/hadoop/hbase/protobuf/ResponseConverter.java

      Use HRegionInfo convertions instead.

      M b/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java

      Use new utility writing out .regioninfo files.

      M b/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java

      Formatting.

      M b/src/main/java/org/apache/hadoop/hbase/regionserver/SplitTransaction.java

      M b/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.java

      Range in Reference is no longer public.

      Range in Reference is no longer public.

      M b/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java

      M b/src/main/java/org/apache/hadoop/hbase/security/token/AuthenticationTokenSecretManager.java

      ClusterId got renamed ZKClusterId

      M b/src/main/java/org/apache/hadoop/hbase/util/FSTableDescriptors.java

      Use new serialization utlity in HTD.

      M b/src/main/java/org/apache/hadoop/hbase/util/FSUtils.java

      Generic method for writing dot file content.

      M b/src/main/java/org/apache/hadoop/hbase/util/HBaseFsck.java

      Reference#Range is not public any more

      M b/src/main/java/org/apache/hadoop/hbase/util/Writables.java

      Deprecated getHRegionInfo, etc.

      D b/src/main/java/org/apache/hadoop/hbase/zookeeper/ClusterId.java

      A b/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKClusterId.java

      Rename

      A b/src/main/protobuf/ClusterId.proto

      Added file for ClusterId only since its written to fs and to zk.

      A b/src/main/protobuf/FS.proto

      Protos for fs files.

      M b/src/main/protobuf/ZooKeeper.proto

      Moved ClusterId out to own proto file

      M b/src/main/protobuf/hbase.proto

      Added TableSchema and ColumnFamilySchema

      This addresses bug hbase-5453.

      https://issues.apache.org/jira/browse/hbase-5453

      Diffs

      -----

      src/main/java/org/apache/hadoop/hbase/ClusterId.java PRE-CREATION

      src/main/java/org/apache/hadoop/hbase/HColumnDescriptor.java 5862f15

      src/main/java/org/apache/hadoop/hbase/HRegionInfo.java 8d83ff3

      src/main/java/org/apache/hadoop/hbase/HTableDescriptor.java af89e3e

      src/main/java/org/apache/hadoop/hbase/client/HConnectionManager.java 5cac9af

      src/main/java/org/apache/hadoop/hbase/io/Reference.java 6360059

      src/main/java/org/apache/hadoop/hbase/mapreduce/LoadIncrementalHFiles.java 9e4ada9

      src/main/java/org/apache/hadoop/hbase/master/HMaster.java 947ec5f

      src/main/java/org/apache/hadoop/hbase/master/MasterFileSystem.java 5052878

      src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java ccc964e

      src/main/java/org/apache/hadoop/hbase/protobuf/RequestConverter.java dabfbab

      src/main/java/org/apache/hadoop/hbase/protobuf/ResponseConverter.java 45cb6cf

      src/main/java/org/apache/hadoop/hbase/protobuf/generated/ClusterIdProtos.java PRE-CREATION

      src/main/java/org/apache/hadoop/hbase/protobuf/generated/FSProtos.java PRE-CREATION

      src/main/java/org/apache/hadoop/hbase/protobuf/generated/HBaseProtos.java 058c006

      src/main/java/org/apache/hadoop/hbase/protobuf/generated/ZooKeeperProtos.java 20c7738

      src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java 9f16fee

      src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 6dc0517

      src/main/java/org/apache/hadoop/hbase/regionserver/SplitTransaction.java 6a9f2fe

      src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.java 5e1e16d

      src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java 5050df0

      src/main/java/org/apache/hadoop/hbase/security/token/AuthenticationTokenSecretManager.java 049ed8d

      src/main/java/org/apache/hadoop/hbase/util/Bytes.java 2ccda17

      src/main/java/org/apache/hadoop/hbase/util/FSTableDescriptors.java efb2b84

      src/main/java/org/apache/hadoop/hbase/util/FSUtils.java 3d35d3e

      src/main/java/org/apache/hadoop/hbase/util/HBaseFsck.java 7b4f4a2

      src/main/java/org/apache/hadoop/hbase/util/Writables.java 3d20723

      src/main/java/org/apache/hadoop/hbase/zookeeper/ClusterId.java f804810

      src/main/java/org/apache/hadoop/hbase/zookeeper/ZKClusterId.java PRE-CREATION

      src/main/protobuf/ClusterId.proto PRE-CREATION

      src/main/protobuf/FS.proto PRE-CREATION

      src/main/protobuf/ZooKeeper.proto b72cb28

      src/main/protobuf/hbase.proto 30a4c3f

      src/test/java/org/apache/hadoop/hbase/TestHColumnDescriptor.java e7fa8b2

      src/test/java/org/apache/hadoop/hbase/TestHTableDescriptor.java f7c0cca

      src/test/java/org/apache/hadoop/hbase/catalog/TestCatalogTracker.java 83d8408

      src/test/java/org/apache/hadoop/hbase/master/MockRegionServer.java 69ccc65

      src/test/java/org/apache/hadoop/hbase/master/TestCatalogJanitor.java 1020374

      src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegionInfo.java 6dfba41

      src/test/java/org/apache/hadoop/hbase/regionserver/TestSplitTransaction.java 98c09eb

      src/test/java/org/apache/hadoop/hbase/regionserver/TestStoreFile.java 988d0bf

      src/test/java/org/apache/hadoop/hbase/util/TestFSUtils.java 339a120

      Diff: https://reviews.apache.org/r/5130/diff

      Testing

      -------

      Thanks,

      Michael

      Show
      jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/5130/#review7951 ----------------------------------------------------------- src/main/java/org/apache/hadoop/hbase/HTableDescriptor.java < https://reviews.apache.org/r/5130/#comment17291 > This is still here? Or just reviewboard showing it for some reason? src/main/java/org/apache/hadoop/hbase/util/FSUtils.java < https://reviews.apache.org/r/5130/#comment17290 > This is what you are using to ensure the sizes cannot be the same? Very nice! I cannot find a call-site for this, though, maybe I missed it? Gregory On 2012-05-16 23:56:18, Michael Stack wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/5130/ ----------------------------------------------------------- (Updated 2012-05-16 23:56:18) Review request for hbase. Summary ------- A b/src/main/java/org/apache/hadoop/hbase/ClusterId.java New class to hold clusterid in. M b/src/main/java/org/apache/hadoop/hbase/HColumnDescriptor.java Make it so can do pb serialization. Deprecated Writable serialization. M b/src/main/java/org/apache/hadoop/hbase/HRegionInfo.java Make it so methods in here follow the pattern in HCD an HTD pb 'ing. Deprecated Writable serialization. M b/src/main/java/org/apache/hadoop/hbase/HTableDescriptor.java Make it so can do pb serialization. Deprecated Writable serialization. M b/src/main/java/org/apache/hadoop/hbase/client/HConnectionManager.java ClusterId under ZK got renamed as ZKClusterId M b/src/main/java/org/apache/hadoop/hbase/io/Reference.java Hide the Reference#Range enums. Don't let them out of this class. Make it so can do pb serialization. M b/src/main/java/org/apache/hadoop/hbase/mapreduce/LoadIncrementalHFiles.java Use new methods on Reference for getting top and bottom. M b/src/main/java/org/apache/hadoop/hbase/master/HMaster.java ClusterId under zk has been renamed ZKClusterId. Use new ClusterId class too. M b/src/main/java/org/apache/hadoop/hbase/master/MasterFileSystem.java Use new clusterid class. M b/src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java Move the RegionInfo convertion up into HRegionInfo instead of here. Added generic toDelimitedByteArray helper. M b/src/main/java/org/apache/hadoop/hbase/protobuf/RequestConverter.java Use HRegionInfo convertions instead. M b/src/main/java/org/apache/hadoop/hbase/protobuf/ResponseConverter.java Use HRegionInfo convertions instead. M b/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java Use new utility writing out .regioninfo files. M b/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java Formatting. M b/src/main/java/org/apache/hadoop/hbase/regionserver/SplitTransaction.java M b/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.java Range in Reference is no longer public. Range in Reference is no longer public. M b/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java M b/src/main/java/org/apache/hadoop/hbase/security/token/AuthenticationTokenSecretManager.java ClusterId got renamed ZKClusterId M b/src/main/java/org/apache/hadoop/hbase/util/FSTableDescriptors.java Use new serialization utlity in HTD. M b/src/main/java/org/apache/hadoop/hbase/util/FSUtils.java Generic method for writing dot file content. M b/src/main/java/org/apache/hadoop/hbase/util/HBaseFsck.java Reference#Range is not public any more M b/src/main/java/org/apache/hadoop/hbase/util/Writables.java Deprecated getHRegionInfo, etc. D b/src/main/java/org/apache/hadoop/hbase/zookeeper/ClusterId.java A b/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKClusterId.java Rename A b/src/main/protobuf/ClusterId.proto Added file for ClusterId only since its written to fs and to zk. A b/src/main/protobuf/FS.proto Protos for fs files. M b/src/main/protobuf/ZooKeeper.proto Moved ClusterId out to own proto file M b/src/main/protobuf/hbase.proto Added TableSchema and ColumnFamilySchema This addresses bug hbase-5453. https://issues.apache.org/jira/browse/hbase-5453 Diffs ----- src/main/java/org/apache/hadoop/hbase/ClusterId.java PRE-CREATION src/main/java/org/apache/hadoop/hbase/HColumnDescriptor.java 5862f15 src/main/java/org/apache/hadoop/hbase/HRegionInfo.java 8d83ff3 src/main/java/org/apache/hadoop/hbase/HTableDescriptor.java af89e3e src/main/java/org/apache/hadoop/hbase/client/HConnectionManager.java 5cac9af src/main/java/org/apache/hadoop/hbase/io/Reference.java 6360059 src/main/java/org/apache/hadoop/hbase/mapreduce/LoadIncrementalHFiles.java 9e4ada9 src/main/java/org/apache/hadoop/hbase/master/HMaster.java 947ec5f src/main/java/org/apache/hadoop/hbase/master/MasterFileSystem.java 5052878 src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java ccc964e src/main/java/org/apache/hadoop/hbase/protobuf/RequestConverter.java dabfbab src/main/java/org/apache/hadoop/hbase/protobuf/ResponseConverter.java 45cb6cf src/main/java/org/apache/hadoop/hbase/protobuf/generated/ClusterIdProtos.java PRE-CREATION src/main/java/org/apache/hadoop/hbase/protobuf/generated/FSProtos.java PRE-CREATION src/main/java/org/apache/hadoop/hbase/protobuf/generated/HBaseProtos.java 058c006 src/main/java/org/apache/hadoop/hbase/protobuf/generated/ZooKeeperProtos.java 20c7738 src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java 9f16fee src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 6dc0517 src/main/java/org/apache/hadoop/hbase/regionserver/SplitTransaction.java 6a9f2fe src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.java 5e1e16d src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java 5050df0 src/main/java/org/apache/hadoop/hbase/security/token/AuthenticationTokenSecretManager.java 049ed8d src/main/java/org/apache/hadoop/hbase/util/Bytes.java 2ccda17 src/main/java/org/apache/hadoop/hbase/util/FSTableDescriptors.java efb2b84 src/main/java/org/apache/hadoop/hbase/util/FSUtils.java 3d35d3e src/main/java/org/apache/hadoop/hbase/util/HBaseFsck.java 7b4f4a2 src/main/java/org/apache/hadoop/hbase/util/Writables.java 3d20723 src/main/java/org/apache/hadoop/hbase/zookeeper/ClusterId.java f804810 src/main/java/org/apache/hadoop/hbase/zookeeper/ZKClusterId.java PRE-CREATION src/main/protobuf/ClusterId.proto PRE-CREATION src/main/protobuf/FS.proto PRE-CREATION src/main/protobuf/ZooKeeper.proto b72cb28 src/main/protobuf/hbase.proto 30a4c3f src/test/java/org/apache/hadoop/hbase/TestHColumnDescriptor.java e7fa8b2 src/test/java/org/apache/hadoop/hbase/TestHTableDescriptor.java f7c0cca src/test/java/org/apache/hadoop/hbase/catalog/TestCatalogTracker.java 83d8408 src/test/java/org/apache/hadoop/hbase/master/MockRegionServer.java 69ccc65 src/test/java/org/apache/hadoop/hbase/master/TestCatalogJanitor.java 1020374 src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegionInfo.java 6dfba41 src/test/java/org/apache/hadoop/hbase/regionserver/TestSplitTransaction.java 98c09eb src/test/java/org/apache/hadoop/hbase/regionserver/TestStoreFile.java 988d0bf src/test/java/org/apache/hadoop/hbase/util/TestFSUtils.java 339a120 Diff: https://reviews.apache.org/r/5130/diff Testing ------- Thanks, Michael
      Hide
      jiraposter@reviews.apache.org added a comment -

      On 2012-05-17 05:41:53, Gregory Chanan wrote:

      > src/main/java/org/apache/hadoop/hbase/HTableDescriptor.java, line 275

      > <https://reviews.apache.org/r/5130/diff/3/?file=109313#file109313line275>

      >

      > This is still here? Or just reviewboard showing it for some reason?

      Flotsam. An abandoned direction that I failed to clean up. Thanks.

      On 2012-05-17 05:41:53, Gregory Chanan wrote:

      > src/main/java/org/apache/hadoop/hbase/util/FSUtils.java, line 91

      > <https://reviews.apache.org/r/5130/diff/3/?file=109334#file109334line91>

      >

      > This is what you are using to ensure the sizes cannot be the same? Very nice!

      >

      > I cannot find a call-site for this, though, maybe I missed it?

      Ditto. I was going to use this to write .regioninfo and .tableinfo but then I ran into your review. Study this method which I'll purge in the next version. See how it puts serialized class at top, two '\n's, and then a toString on the class? Thats how .regioninfo files and .tableinfo files are currently written. My patch now changes it so these files only contained the serialized object... no '\n' and no toString. I do this so its very unlikely a Writable length will class w/ the length of a pb'd content (See HRegion for where we write .regioninfo and in FSTableDescriptors for where we write .tableinfo only we don't need the 'trick' here since we have actually read the file and can see if its pb'd).

      Thanks for the review G.

      • Michael

      -----------------------------------------------------------
      This is an automatically generated e-mail. To reply, visit:
      https://reviews.apache.org/r/5130/#review7951
      -----------------------------------------------------------

      On 2012-05-16 23:56:18, Michael Stack wrote:

      -----------------------------------------------------------

      This is an automatically generated e-mail. To reply, visit:

      https://reviews.apache.org/r/5130/

      -----------------------------------------------------------

      (Updated 2012-05-16 23:56:18)

      Review request for hbase.

      Summary

      -------

      A b/src/main/java/org/apache/hadoop/hbase/ClusterId.java

      New class to hold clusterid in.

      M b/src/main/java/org/apache/hadoop/hbase/HColumnDescriptor.java

      Make it so can do pb serialization. Deprecated Writable serialization.

      M b/src/main/java/org/apache/hadoop/hbase/HRegionInfo.java

      Make it so methods in here follow the pattern in HCD an HTD pb 'ing.

      Deprecated Writable serialization.

      M b/src/main/java/org/apache/hadoop/hbase/HTableDescriptor.java

      Make it so can do pb serialization. Deprecated Writable serialization.

      M b/src/main/java/org/apache/hadoop/hbase/client/HConnectionManager.java

      ClusterId under ZK got renamed as ZKClusterId

      M b/src/main/java/org/apache/hadoop/hbase/io/Reference.java

      Hide the Reference#Range enums. Don't let them out of this class.

      Make it so can do pb serialization.

      M b/src/main/java/org/apache/hadoop/hbase/mapreduce/LoadIncrementalHFiles.java

      Use new methods on Reference for getting top and bottom.

      M b/src/main/java/org/apache/hadoop/hbase/master/HMaster.java

      ClusterId under zk has been renamed ZKClusterId.

      Use new ClusterId class too.

      M b/src/main/java/org/apache/hadoop/hbase/master/MasterFileSystem.java

      Use new clusterid class.

      M b/src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java

      Move the RegionInfo convertion up into HRegionInfo instead of here.

      Added generic toDelimitedByteArray helper.

      M b/src/main/java/org/apache/hadoop/hbase/protobuf/RequestConverter.java

      Use HRegionInfo convertions instead.

      M b/src/main/java/org/apache/hadoop/hbase/protobuf/ResponseConverter.java

      Use HRegionInfo convertions instead.

      M b/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java

      Use new utility writing out .regioninfo files.

      M b/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java

      Formatting.

      M b/src/main/java/org/apache/hadoop/hbase/regionserver/SplitTransaction.java

      M b/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.java

      Range in Reference is no longer public.

      Range in Reference is no longer public.

      M b/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java

      M b/src/main/java/org/apache/hadoop/hbase/security/token/AuthenticationTokenSecretManager.java

      ClusterId got renamed ZKClusterId

      M b/src/main/java/org/apache/hadoop/hbase/util/FSTableDescriptors.java

      Use new serialization utlity in HTD.

      M b/src/main/java/org/apache/hadoop/hbase/util/FSUtils.java

      Generic method for writing dot file content.

      M b/src/main/java/org/apache/hadoop/hbase/util/HBaseFsck.java

      Reference#Range is not public any more

      M b/src/main/java/org/apache/hadoop/hbase/util/Writables.java

      Deprecated getHRegionInfo, etc.

      D b/src/main/java/org/apache/hadoop/hbase/zookeeper/ClusterId.java

      A b/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKClusterId.java

      Rename

      A b/src/main/protobuf/ClusterId.proto

      Added file for ClusterId only since its written to fs and to zk.

      A b/src/main/protobuf/FS.proto

      Protos for fs files.

      M b/src/main/protobuf/ZooKeeper.proto

      Moved ClusterId out to own proto file

      M b/src/main/protobuf/hbase.proto

      Added TableSchema and ColumnFamilySchema

      This addresses bug hbase-5453.

      https://issues.apache.org/jira/browse/hbase-5453

      Diffs

      -----

      src/main/java/org/apache/hadoop/hbase/ClusterId.java PRE-CREATION

      src/main/java/org/apache/hadoop/hbase/HColumnDescriptor.java 5862f15

      src/main/java/org/apache/hadoop/hbase/HRegionInfo.java 8d83ff3

      src/main/java/org/apache/hadoop/hbase/HTableDescriptor.java af89e3e

      src/main/java/org/apache/hadoop/hbase/client/HConnectionManager.java 5cac9af

      src/main/java/org/apache/hadoop/hbase/io/Reference.java 6360059

      src/main/java/org/apache/hadoop/hbase/mapreduce/LoadIncrementalHFiles.java 9e4ada9

      src/main/java/org/apache/hadoop/hbase/master/HMaster.java 947ec5f

      src/main/java/org/apache/hadoop/hbase/master/MasterFileSystem.java 5052878

      src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java ccc964e

      src/main/java/org/apache/hadoop/hbase/protobuf/RequestConverter.java dabfbab

      src/main/java/org/apache/hadoop/hbase/protobuf/ResponseConverter.java 45cb6cf

      src/main/java/org/apache/hadoop/hbase/protobuf/generated/ClusterIdProtos.java PRE-CREATION

      src/main/java/org/apache/hadoop/hbase/protobuf/generated/FSProtos.java PRE-CREATION

      src/main/java/org/apache/hadoop/hbase/protobuf/generated/HBaseProtos.java 058c006

      src/main/java/org/apache/hadoop/hbase/protobuf/generated/ZooKeeperProtos.java 20c7738

      src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java 9f16fee

      src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 6dc0517

      src/main/java/org/apache/hadoop/hbase/regionserver/SplitTransaction.java 6a9f2fe

      src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.java 5e1e16d

      src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java 5050df0

      src/main/java/org/apache/hadoop/hbase/security/token/AuthenticationTokenSecretManager.java 049ed8d

      src/main/java/org/apache/hadoop/hbase/util/Bytes.java 2ccda17

      src/main/java/org/apache/hadoop/hbase/util/FSTableDescriptors.java efb2b84

      src/main/java/org/apache/hadoop/hbase/util/FSUtils.java 3d35d3e

      src/main/java/org/apache/hadoop/hbase/util/HBaseFsck.java 7b4f4a2

      src/main/java/org/apache/hadoop/hbase/util/Writables.java 3d20723

      src/main/java/org/apache/hadoop/hbase/zookeeper/ClusterId.java f804810

      src/main/java/org/apache/hadoop/hbase/zookeeper/ZKClusterId.java PRE-CREATION

      src/main/protobuf/ClusterId.proto PRE-CREATION

      src/main/protobuf/FS.proto PRE-CREATION

      src/main/protobuf/ZooKeeper.proto b72cb28

      src/main/protobuf/hbase.proto 30a4c3f

      src/test/java/org/apache/hadoop/hbase/TestHColumnDescriptor.java e7fa8b2

      src/test/java/org/apache/hadoop/hbase/TestHTableDescriptor.java f7c0cca

      src/test/java/org/apache/hadoop/hbase/catalog/TestCatalogTracker.java 83d8408

      src/test/java/org/apache/hadoop/hbase/master/MockRegionServer.java 69ccc65

      src/test/java/org/apache/hadoop/hbase/master/TestCatalogJanitor.java 1020374

      src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegionInfo.java 6dfba41

      src/test/java/org/apache/hadoop/hbase/regionserver/TestSplitTransaction.java 98c09eb

      src/test/java/org/apache/hadoop/hbase/regionserver/TestStoreFile.java 988d0bf

      src/test/java/org/apache/hadoop/hbase/util/TestFSUtils.java 339a120

      Diff: https://reviews.apache.org/r/5130/diff

      Testing

      -------

      Thanks,

      Michael

      Show
      jiraposter@reviews.apache.org added a comment - On 2012-05-17 05:41:53, Gregory Chanan wrote: > src/main/java/org/apache/hadoop/hbase/HTableDescriptor.java, line 275 > < https://reviews.apache.org/r/5130/diff/3/?file=109313#file109313line275 > > > This is still here? Or just reviewboard showing it for some reason? Flotsam. An abandoned direction that I failed to clean up. Thanks. On 2012-05-17 05:41:53, Gregory Chanan wrote: > src/main/java/org/apache/hadoop/hbase/util/FSUtils.java, line 91 > < https://reviews.apache.org/r/5130/diff/3/?file=109334#file109334line91 > > > This is what you are using to ensure the sizes cannot be the same? Very nice! > > I cannot find a call-site for this, though, maybe I missed it? Ditto. I was going to use this to write .regioninfo and .tableinfo but then I ran into your review. Study this method which I'll purge in the next version. See how it puts serialized class at top, two '\n's, and then a toString on the class? Thats how .regioninfo files and .tableinfo files are currently written. My patch now changes it so these files only contained the serialized object... no '\n' and no toString. I do this so its very unlikely a Writable length will class w/ the length of a pb'd content (See HRegion for where we write .regioninfo and in FSTableDescriptors for where we write .tableinfo only we don't need the 'trick' here since we have actually read the file and can see if its pb'd). Thanks for the review G. Michael ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/5130/#review7951 ----------------------------------------------------------- On 2012-05-16 23:56:18, Michael Stack wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/5130/ ----------------------------------------------------------- (Updated 2012-05-16 23:56:18) Review request for hbase. Summary ------- A b/src/main/java/org/apache/hadoop/hbase/ClusterId.java New class to hold clusterid in. M b/src/main/java/org/apache/hadoop/hbase/HColumnDescriptor.java Make it so can do pb serialization. Deprecated Writable serialization. M b/src/main/java/org/apache/hadoop/hbase/HRegionInfo.java Make it so methods in here follow the pattern in HCD an HTD pb 'ing. Deprecated Writable serialization. M b/src/main/java/org/apache/hadoop/hbase/HTableDescriptor.java Make it so can do pb serialization. Deprecated Writable serialization. M b/src/main/java/org/apache/hadoop/hbase/client/HConnectionManager.java ClusterId under ZK got renamed as ZKClusterId M b/src/main/java/org/apache/hadoop/hbase/io/Reference.java Hide the Reference#Range enums. Don't let them out of this class. Make it so can do pb serialization. M b/src/main/java/org/apache/hadoop/hbase/mapreduce/LoadIncrementalHFiles.java Use new methods on Reference for getting top and bottom. M b/src/main/java/org/apache/hadoop/hbase/master/HMaster.java ClusterId under zk has been renamed ZKClusterId. Use new ClusterId class too. M b/src/main/java/org/apache/hadoop/hbase/master/MasterFileSystem.java Use new clusterid class. M b/src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java Move the RegionInfo convertion up into HRegionInfo instead of here. Added generic toDelimitedByteArray helper. M b/src/main/java/org/apache/hadoop/hbase/protobuf/RequestConverter.java Use HRegionInfo convertions instead. M b/src/main/java/org/apache/hadoop/hbase/protobuf/ResponseConverter.java Use HRegionInfo convertions instead. M b/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java Use new utility writing out .regioninfo files. M b/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java Formatting. M b/src/main/java/org/apache/hadoop/hbase/regionserver/SplitTransaction.java M b/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.java Range in Reference is no longer public. Range in Reference is no longer public. M b/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java M b/src/main/java/org/apache/hadoop/hbase/security/token/AuthenticationTokenSecretManager.java ClusterId got renamed ZKClusterId M b/src/main/java/org/apache/hadoop/hbase/util/FSTableDescriptors.java Use new serialization utlity in HTD. M b/src/main/java/org/apache/hadoop/hbase/util/FSUtils.java Generic method for writing dot file content. M b/src/main/java/org/apache/hadoop/hbase/util/HBaseFsck.java Reference#Range is not public any more M b/src/main/java/org/apache/hadoop/hbase/util/Writables.java Deprecated getHRegionInfo, etc. D b/src/main/java/org/apache/hadoop/hbase/zookeeper/ClusterId.java A b/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKClusterId.java Rename A b/src/main/protobuf/ClusterId.proto Added file for ClusterId only since its written to fs and to zk. A b/src/main/protobuf/FS.proto Protos for fs files. M b/src/main/protobuf/ZooKeeper.proto Moved ClusterId out to own proto file M b/src/main/protobuf/hbase.proto Added TableSchema and ColumnFamilySchema This addresses bug hbase-5453. https://issues.apache.org/jira/browse/hbase-5453 Diffs ----- src/main/java/org/apache/hadoop/hbase/ClusterId.java PRE-CREATION src/main/java/org/apache/hadoop/hbase/HColumnDescriptor.java 5862f15 src/main/java/org/apache/hadoop/hbase/HRegionInfo.java 8d83ff3 src/main/java/org/apache/hadoop/hbase/HTableDescriptor.java af89e3e src/main/java/org/apache/hadoop/hbase/client/HConnectionManager.java 5cac9af src/main/java/org/apache/hadoop/hbase/io/Reference.java 6360059 src/main/java/org/apache/hadoop/hbase/mapreduce/LoadIncrementalHFiles.java 9e4ada9 src/main/java/org/apache/hadoop/hbase/master/HMaster.java 947ec5f src/main/java/org/apache/hadoop/hbase/master/MasterFileSystem.java 5052878 src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java ccc964e src/main/java/org/apache/hadoop/hbase/protobuf/RequestConverter.java dabfbab src/main/java/org/apache/hadoop/hbase/protobuf/ResponseConverter.java 45cb6cf src/main/java/org/apache/hadoop/hbase/protobuf/generated/ClusterIdProtos.java PRE-CREATION src/main/java/org/apache/hadoop/hbase/protobuf/generated/FSProtos.java PRE-CREATION src/main/java/org/apache/hadoop/hbase/protobuf/generated/HBaseProtos.java 058c006 src/main/java/org/apache/hadoop/hbase/protobuf/generated/ZooKeeperProtos.java 20c7738 src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java 9f16fee src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 6dc0517 src/main/java/org/apache/hadoop/hbase/regionserver/SplitTransaction.java 6a9f2fe src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.java 5e1e16d src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java 5050df0 src/main/java/org/apache/hadoop/hbase/security/token/AuthenticationTokenSecretManager.java 049ed8d src/main/java/org/apache/hadoop/hbase/util/Bytes.java 2ccda17 src/main/java/org/apache/hadoop/hbase/util/FSTableDescriptors.java efb2b84 src/main/java/org/apache/hadoop/hbase/util/FSUtils.java 3d35d3e src/main/java/org/apache/hadoop/hbase/util/HBaseFsck.java 7b4f4a2 src/main/java/org/apache/hadoop/hbase/util/Writables.java 3d20723 src/main/java/org/apache/hadoop/hbase/zookeeper/ClusterId.java f804810 src/main/java/org/apache/hadoop/hbase/zookeeper/ZKClusterId.java PRE-CREATION src/main/protobuf/ClusterId.proto PRE-CREATION src/main/protobuf/FS.proto PRE-CREATION src/main/protobuf/ZooKeeper.proto b72cb28 src/main/protobuf/hbase.proto 30a4c3f src/test/java/org/apache/hadoop/hbase/TestHColumnDescriptor.java e7fa8b2 src/test/java/org/apache/hadoop/hbase/TestHTableDescriptor.java f7c0cca src/test/java/org/apache/hadoop/hbase/catalog/TestCatalogTracker.java 83d8408 src/test/java/org/apache/hadoop/hbase/master/MockRegionServer.java 69ccc65 src/test/java/org/apache/hadoop/hbase/master/TestCatalogJanitor.java 1020374 src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegionInfo.java 6dfba41 src/test/java/org/apache/hadoop/hbase/regionserver/TestSplitTransaction.java 98c09eb src/test/java/org/apache/hadoop/hbase/regionserver/TestStoreFile.java 988d0bf src/test/java/org/apache/hadoop/hbase/util/TestFSUtils.java 339a120 Diff: https://reviews.apache.org/r/5130/diff Testing ------- Thanks, Michael
      Hide
      stack added a comment -

      v11 addressing Gregory's last review comments.

      Show
      stack added a comment - v11 addressing Gregory's last review comments.
      Hide
      jiraposter@reviews.apache.org added a comment -

      -----------------------------------------------------------
      This is an automatically generated e-mail. To reply, visit:
      https://reviews.apache.org/r/5130/
      -----------------------------------------------------------

      (Updated 2012-05-17 05:58:11.354772)

      Review request for hbase.

      Changes
      -------

      Address Gregory's two review comments.

      Summary
      -------

      A b/src/main/java/org/apache/hadoop/hbase/ClusterId.java
      New class to hold clusterid in.
      M b/src/main/java/org/apache/hadoop/hbase/HColumnDescriptor.java
      Make it so can do pb serialization. Deprecated Writable serialization.
      M b/src/main/java/org/apache/hadoop/hbase/HRegionInfo.java
      Make it so methods in here follow the pattern in HCD an HTD pb 'ing.
      Deprecated Writable serialization.
      M b/src/main/java/org/apache/hadoop/hbase/HTableDescriptor.java
      Make it so can do pb serialization. Deprecated Writable serialization.
      M b/src/main/java/org/apache/hadoop/hbase/client/HConnectionManager.java
      ClusterId under ZK got renamed as ZKClusterId
      M b/src/main/java/org/apache/hadoop/hbase/io/Reference.java
      Hide the Reference#Range enums. Don't let them out of this class.
      Make it so can do pb serialization.
      M b/src/main/java/org/apache/hadoop/hbase/mapreduce/LoadIncrementalHFiles.java
      Use new methods on Reference for getting top and bottom.
      M b/src/main/java/org/apache/hadoop/hbase/master/HMaster.java
      ClusterId under zk has been renamed ZKClusterId.
      Use new ClusterId class too.
      M b/src/main/java/org/apache/hadoop/hbase/master/MasterFileSystem.java
      Use new clusterid class.
      M b/src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java
      Move the RegionInfo convertion up into HRegionInfo instead of here.
      Added generic toDelimitedByteArray helper.
      M b/src/main/java/org/apache/hadoop/hbase/protobuf/RequestConverter.java
      Use HRegionInfo convertions instead.
      M b/src/main/java/org/apache/hadoop/hbase/protobuf/ResponseConverter.java
      Use HRegionInfo convertions instead.
      M b/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
      Use new utility writing out .regioninfo files.
      M b/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java
      Formatting.
      M b/src/main/java/org/apache/hadoop/hbase/regionserver/SplitTransaction.java
      M b/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.java
      Range in Reference is no longer public.
      Range in Reference is no longer public.
      M b/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java
      M b/src/main/java/org/apache/hadoop/hbase/security/token/AuthenticationTokenSecretManager.java
      ClusterId got renamed ZKClusterId
      M b/src/main/java/org/apache/hadoop/hbase/util/FSTableDescriptors.java
      Use new serialization utlity in HTD.
      M b/src/main/java/org/apache/hadoop/hbase/util/FSUtils.java
      Generic method for writing dot file content.
      M b/src/main/java/org/apache/hadoop/hbase/util/HBaseFsck.java
      Reference#Range is not public any more
      M b/src/main/java/org/apache/hadoop/hbase/util/Writables.java
      Deprecated getHRegionInfo, etc.
      D b/src/main/java/org/apache/hadoop/hbase/zookeeper/ClusterId.java
      A b/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKClusterId.java
      Rename
      A b/src/main/protobuf/ClusterId.proto
      Added file for ClusterId only since its written to fs and to zk.
      A b/src/main/protobuf/FS.proto
      Protos for fs files.
      M b/src/main/protobuf/ZooKeeper.proto
      Moved ClusterId out to own proto file
      M b/src/main/protobuf/hbase.proto
      Added TableSchema and ColumnFamilySchema

      This addresses bug hbase-5453.
      https://issues.apache.org/jira/browse/hbase-5453

      Diffs (updated)


      src/main/java/org/apache/hadoop/hbase/ClusterId.java PRE-CREATION
      src/main/java/org/apache/hadoop/hbase/HColumnDescriptor.java 5862f15
      src/main/java/org/apache/hadoop/hbase/HRegionInfo.java 8d83ff3
      src/main/java/org/apache/hadoop/hbase/HTableDescriptor.java af89e3e
      src/main/java/org/apache/hadoop/hbase/client/HConnectionManager.java 5cac9af
      src/main/java/org/apache/hadoop/hbase/io/Reference.java 6360059
      src/main/java/org/apache/hadoop/hbase/mapreduce/LoadIncrementalHFiles.java 9e4ada9
      src/main/java/org/apache/hadoop/hbase/master/HMaster.java 947ec5f
      src/main/java/org/apache/hadoop/hbase/master/MasterFileSystem.java 5052878
      src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java ccc964e
      src/main/java/org/apache/hadoop/hbase/protobuf/RequestConverter.java dabfbab
      src/main/java/org/apache/hadoop/hbase/protobuf/ResponseConverter.java 45cb6cf
      src/main/java/org/apache/hadoop/hbase/protobuf/generated/ClusterIdProtos.java PRE-CREATION
      src/main/java/org/apache/hadoop/hbase/protobuf/generated/FSProtos.java PRE-CREATION
      src/main/java/org/apache/hadoop/hbase/protobuf/generated/HBaseProtos.java 058c006
      src/main/java/org/apache/hadoop/hbase/protobuf/generated/ZooKeeperProtos.java 20c7738
      src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java 9f16fee
      src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 6dc0517
      src/main/java/org/apache/hadoop/hbase/regionserver/SplitTransaction.java 6a9f2fe
      src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.java 5e1e16d
      src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java 5050df0
      src/main/java/org/apache/hadoop/hbase/security/token/AuthenticationTokenSecretManager.java 049ed8d
      src/main/java/org/apache/hadoop/hbase/util/Bytes.java 2ccda17
      src/main/java/org/apache/hadoop/hbase/util/FSTableDescriptors.java efb2b84
      src/main/java/org/apache/hadoop/hbase/util/FSUtils.java 3d35d3e
      src/main/java/org/apache/hadoop/hbase/util/HBaseFsck.java 7b4f4a2
      src/main/java/org/apache/hadoop/hbase/util/Writables.java 3d20723
      src/main/java/org/apache/hadoop/hbase/zookeeper/ClusterId.java f804810
      src/main/java/org/apache/hadoop/hbase/zookeeper/ZKClusterId.java PRE-CREATION
      src/main/protobuf/ClusterId.proto PRE-CREATION
      src/main/protobuf/FS.proto PRE-CREATION
      src/main/protobuf/ZooKeeper.proto b72cb28
      src/main/protobuf/hbase.proto 30a4c3f
      src/test/java/org/apache/hadoop/hbase/TestHColumnDescriptor.java e7fa8b2
      src/test/java/org/apache/hadoop/hbase/TestHTableDescriptor.java f7c0cca
      src/test/java/org/apache/hadoop/hbase/catalog/TestCatalogTracker.java 83d8408
      src/test/java/org/apache/hadoop/hbase/master/MockRegionServer.java 69ccc65
      src/test/java/org/apache/hadoop/hbase/master/TestCatalogJanitor.java 1020374
      src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegionInfo.java 6dfba41
      src/test/java/org/apache/hadoop/hbase/regionserver/TestSplitTransaction.java 98c09eb
      src/test/java/org/apache/hadoop/hbase/regionserver/TestStoreFile.java 988d0bf
      src/test/java/org/apache/hadoop/hbase/util/TestFSUtils.java 339a120

      Diff: https://reviews.apache.org/r/5130/diff

      Testing
      -------

      Thanks,

      Michael

      Show
      jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/5130/ ----------------------------------------------------------- (Updated 2012-05-17 05:58:11.354772) Review request for hbase. Changes ------- Address Gregory's two review comments. Summary ------- A b/src/main/java/org/apache/hadoop/hbase/ClusterId.java New class to hold clusterid in. M b/src/main/java/org/apache/hadoop/hbase/HColumnDescriptor.java Make it so can do pb serialization. Deprecated Writable serialization. M b/src/main/java/org/apache/hadoop/hbase/HRegionInfo.java Make it so methods in here follow the pattern in HCD an HTD pb 'ing. Deprecated Writable serialization. M b/src/main/java/org/apache/hadoop/hbase/HTableDescriptor.java Make it so can do pb serialization. Deprecated Writable serialization. M b/src/main/java/org/apache/hadoop/hbase/client/HConnectionManager.java ClusterId under ZK got renamed as ZKClusterId M b/src/main/java/org/apache/hadoop/hbase/io/Reference.java Hide the Reference#Range enums. Don't let them out of this class. Make it so can do pb serialization. M b/src/main/java/org/apache/hadoop/hbase/mapreduce/LoadIncrementalHFiles.java Use new methods on Reference for getting top and bottom. M b/src/main/java/org/apache/hadoop/hbase/master/HMaster.java ClusterId under zk has been renamed ZKClusterId. Use new ClusterId class too. M b/src/main/java/org/apache/hadoop/hbase/master/MasterFileSystem.java Use new clusterid class. M b/src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java Move the RegionInfo convertion up into HRegionInfo instead of here. Added generic toDelimitedByteArray helper. M b/src/main/java/org/apache/hadoop/hbase/protobuf/RequestConverter.java Use HRegionInfo convertions instead. M b/src/main/java/org/apache/hadoop/hbase/protobuf/ResponseConverter.java Use HRegionInfo convertions instead. M b/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java Use new utility writing out .regioninfo files. M b/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java Formatting. M b/src/main/java/org/apache/hadoop/hbase/regionserver/SplitTransaction.java M b/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.java Range in Reference is no longer public. Range in Reference is no longer public. M b/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java M b/src/main/java/org/apache/hadoop/hbase/security/token/AuthenticationTokenSecretManager.java ClusterId got renamed ZKClusterId M b/src/main/java/org/apache/hadoop/hbase/util/FSTableDescriptors.java Use new serialization utlity in HTD. M b/src/main/java/org/apache/hadoop/hbase/util/FSUtils.java Generic method for writing dot file content. M b/src/main/java/org/apache/hadoop/hbase/util/HBaseFsck.java Reference#Range is not public any more M b/src/main/java/org/apache/hadoop/hbase/util/Writables.java Deprecated getHRegionInfo, etc. D b/src/main/java/org/apache/hadoop/hbase/zookeeper/ClusterId.java A b/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKClusterId.java Rename A b/src/main/protobuf/ClusterId.proto Added file for ClusterId only since its written to fs and to zk. A b/src/main/protobuf/FS.proto Protos for fs files. M b/src/main/protobuf/ZooKeeper.proto Moved ClusterId out to own proto file M b/src/main/protobuf/hbase.proto Added TableSchema and ColumnFamilySchema This addresses bug hbase-5453. https://issues.apache.org/jira/browse/hbase-5453 Diffs (updated) src/main/java/org/apache/hadoop/hbase/ClusterId.java PRE-CREATION src/main/java/org/apache/hadoop/hbase/HColumnDescriptor.java 5862f15 src/main/java/org/apache/hadoop/hbase/HRegionInfo.java 8d83ff3 src/main/java/org/apache/hadoop/hbase/HTableDescriptor.java af89e3e src/main/java/org/apache/hadoop/hbase/client/HConnectionManager.java 5cac9af src/main/java/org/apache/hadoop/hbase/io/Reference.java 6360059 src/main/java/org/apache/hadoop/hbase/mapreduce/LoadIncrementalHFiles.java 9e4ada9 src/main/java/org/apache/hadoop/hbase/master/HMaster.java 947ec5f src/main/java/org/apache/hadoop/hbase/master/MasterFileSystem.java 5052878 src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java ccc964e src/main/java/org/apache/hadoop/hbase/protobuf/RequestConverter.java dabfbab src/main/java/org/apache/hadoop/hbase/protobuf/ResponseConverter.java 45cb6cf src/main/java/org/apache/hadoop/hbase/protobuf/generated/ClusterIdProtos.java PRE-CREATION src/main/java/org/apache/hadoop/hbase/protobuf/generated/FSProtos.java PRE-CREATION src/main/java/org/apache/hadoop/hbase/protobuf/generated/HBaseProtos.java 058c006 src/main/java/org/apache/hadoop/hbase/protobuf/generated/ZooKeeperProtos.java 20c7738 src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java 9f16fee src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 6dc0517 src/main/java/org/apache/hadoop/hbase/regionserver/SplitTransaction.java 6a9f2fe src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.java 5e1e16d src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java 5050df0 src/main/java/org/apache/hadoop/hbase/security/token/AuthenticationTokenSecretManager.java 049ed8d src/main/java/org/apache/hadoop/hbase/util/Bytes.java 2ccda17 src/main/java/org/apache/hadoop/hbase/util/FSTableDescriptors.java efb2b84 src/main/java/org/apache/hadoop/hbase/util/FSUtils.java 3d35d3e src/main/java/org/apache/hadoop/hbase/util/HBaseFsck.java 7b4f4a2 src/main/java/org/apache/hadoop/hbase/util/Writables.java 3d20723 src/main/java/org/apache/hadoop/hbase/zookeeper/ClusterId.java f804810 src/main/java/org/apache/hadoop/hbase/zookeeper/ZKClusterId.java PRE-CREATION src/main/protobuf/ClusterId.proto PRE-CREATION src/main/protobuf/FS.proto PRE-CREATION src/main/protobuf/ZooKeeper.proto b72cb28 src/main/protobuf/hbase.proto 30a4c3f src/test/java/org/apache/hadoop/hbase/TestHColumnDescriptor.java e7fa8b2 src/test/java/org/apache/hadoop/hbase/TestHTableDescriptor.java f7c0cca src/test/java/org/apache/hadoop/hbase/catalog/TestCatalogTracker.java 83d8408 src/test/java/org/apache/hadoop/hbase/master/MockRegionServer.java 69ccc65 src/test/java/org/apache/hadoop/hbase/master/TestCatalogJanitor.java 1020374 src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegionInfo.java 6dfba41 src/test/java/org/apache/hadoop/hbase/regionserver/TestSplitTransaction.java 98c09eb src/test/java/org/apache/hadoop/hbase/regionserver/TestStoreFile.java 988d0bf src/test/java/org/apache/hadoop/hbase/util/TestFSUtils.java 339a120 Diff: https://reviews.apache.org/r/5130/diff Testing ------- Thanks, Michael
      Hide
      Hadoop QA added a comment -

      -1 overall. Here are the results of testing the latest attachment
      http://issues.apache.org/jira/secure/attachment/12527785/5453v11.txt
      against trunk revision .

      +1 @author. The patch does not contain any @author tags.

      +1 tests included. The patch appears to include 27 new or modified tests.

      +1 hadoop23. The patch compiles against the hadoop 0.23.x profile.

      +1 javadoc. The javadoc tool did not generate any warning messages.

      +1 javac. The applied patch does not increase the total number of javac compiler warnings.

      -1 findbugs. The patch appears to introduce 32 new Findbugs (version 1.3.9) warnings.

      +1 release audit. The applied patch does not increase the total number of release audit warnings.

      -1 core tests. The patch failed these unit tests:
      org.apache.hadoop.hbase.client.TestMultipleTimestamps
      org.apache.hadoop.hbase.client.TestMetaMigrationRemovingHTD
      org.apache.hadoop.hbase.util.hbck.TestOfflineMetaRebuildOverlap
      org.apache.hadoop.hbase.replication.TestReplication
      org.apache.hadoop.hbase.client.TestTimestampsFilter
      org.apache.hadoop.hbase.replication.TestMultiSlaveReplication
      org.apache.hadoop.hbase.client.TestFromClientSide
      org.apache.hadoop.hbase.mapreduce.TestImportExport
      org.apache.hadoop.hbase.replication.TestMasterReplication

      Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/1906//testReport/
      Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/1906//artifact/trunk/patchprocess/newPatchFindbugsWarnings.html
      Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/1906//console

      This message is automatically generated.

      Show
      Hadoop QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12527785/5453v11.txt against trunk revision . +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 27 new or modified tests. +1 hadoop23. The patch compiles against the hadoop 0.23.x profile. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. -1 findbugs. The patch appears to introduce 32 new Findbugs (version 1.3.9) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. -1 core tests. The patch failed these unit tests: org.apache.hadoop.hbase.client.TestMultipleTimestamps org.apache.hadoop.hbase.client.TestMetaMigrationRemovingHTD org.apache.hadoop.hbase.util.hbck.TestOfflineMetaRebuildOverlap org.apache.hadoop.hbase.replication.TestReplication org.apache.hadoop.hbase.client.TestTimestampsFilter org.apache.hadoop.hbase.replication.TestMultiSlaveReplication org.apache.hadoop.hbase.client.TestFromClientSide org.apache.hadoop.hbase.mapreduce.TestImportExport org.apache.hadoop.hbase.replication.TestMasterReplication Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/1906//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/1906//artifact/trunk/patchprocess/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/1906//console This message is automatically generated.
      Hide
      jiraposter@reviews.apache.org added a comment -

      -----------------------------------------------------------
      This is an automatically generated e-mail. To reply, visit:
      https://reviews.apache.org/r/5130/#review7963
      -----------------------------------------------------------

      Ship it!

      src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
      <https://reviews.apache.org/r/5130/#comment17303>

      Your logic sounds correct and I agree we should avoid having to read the file each time.

      It would be cool to just see an example of the old writable vs new pb size to ensure our intuition is correct.

      Worst case (let's say they happen to be the same) we could write out the new pb stuff concatenated with the old way of writing and then they are guaranteed to be different sizes. Hopefully we don't have to do something like that, though.

      • Gregory

      On 2012-05-17 05:58:11, Michael Stack wrote:

      -----------------------------------------------------------

      This is an automatically generated e-mail. To reply, visit:

      https://reviews.apache.org/r/5130/

      -----------------------------------------------------------

      (Updated 2012-05-17 05:58:11)

      Review request for hbase.

      Summary

      -------

      A b/src/main/java/org/apache/hadoop/hbase/ClusterId.java

      New class to hold clusterid in.

      M b/src/main/java/org/apache/hadoop/hbase/HColumnDescriptor.java

      Make it so can do pb serialization. Deprecated Writable serialization.

      M b/src/main/java/org/apache/hadoop/hbase/HRegionInfo.java

      Make it so methods in here follow the pattern in HCD an HTD pb 'ing.

      Deprecated Writable serialization.

      M b/src/main/java/org/apache/hadoop/hbase/HTableDescriptor.java

      Make it so can do pb serialization. Deprecated Writable serialization.

      M b/src/main/java/org/apache/hadoop/hbase/client/HConnectionManager.java

      ClusterId under ZK got renamed as ZKClusterId

      M b/src/main/java/org/apache/hadoop/hbase/io/Reference.java

      Hide the Reference#Range enums. Don't let them out of this class.

      Make it so can do pb serialization.

      M b/src/main/java/org/apache/hadoop/hbase/mapreduce/LoadIncrementalHFiles.java

      Use new methods on Reference for getting top and bottom.

      M b/src/main/java/org/apache/hadoop/hbase/master/HMaster.java

      ClusterId under zk has been renamed ZKClusterId.

      Use new ClusterId class too.

      M b/src/main/java/org/apache/hadoop/hbase/master/MasterFileSystem.java

      Use new clusterid class.

      M b/src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java

      Move the RegionInfo convertion up into HRegionInfo instead of here.

      Added generic toDelimitedByteArray helper.

      M b/src/main/java/org/apache/hadoop/hbase/protobuf/RequestConverter.java

      Use HRegionInfo convertions instead.

      M b/src/main/java/org/apache/hadoop/hbase/protobuf/ResponseConverter.java

      Use HRegionInfo convertions instead.

      M b/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java

      Use new utility writing out .regioninfo files.

      M b/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java

      Formatting.

      M b/src/main/java/org/apache/hadoop/hbase/regionserver/SplitTransaction.java

      M b/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.java

      Range in Reference is no longer public.

      Range in Reference is no longer public.

      M b/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java

      M b/src/main/java/org/apache/hadoop/hbase/security/token/AuthenticationTokenSecretManager.java

      ClusterId got renamed ZKClusterId

      M b/src/main/java/org/apache/hadoop/hbase/util/FSTableDescriptors.java

      Use new serialization utlity in HTD.

      M b/src/main/java/org/apache/hadoop/hbase/util/FSUtils.java

      Generic method for writing dot file content.

      M b/src/main/java/org/apache/hadoop/hbase/util/HBaseFsck.java

      Reference#Range is not public any more

      M b/src/main/java/org/apache/hadoop/hbase/util/Writables.java

      Deprecated getHRegionInfo, etc.

      D b/src/main/java/org/apache/hadoop/hbase/zookeeper/ClusterId.java

      A b/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKClusterId.java

      Rename

      A b/src/main/protobuf/ClusterId.proto

      Added file for ClusterId only since its written to fs and to zk.

      A b/src/main/protobuf/FS.proto

      Protos for fs files.

      M b/src/main/protobuf/ZooKeeper.proto

      Moved ClusterId out to own proto file

      M b/src/main/protobuf/hbase.proto

      Added TableSchema and ColumnFamilySchema

      This addresses bug hbase-5453.

      https://issues.apache.org/jira/browse/hbase-5453

      Diffs

      -----

      src/main/java/org/apache/hadoop/hbase/ClusterId.java PRE-CREATION

      src/main/java/org/apache/hadoop/hbase/HColumnDescriptor.java 5862f15

      src/main/java/org/apache/hadoop/hbase/HRegionInfo.java 8d83ff3

      src/main/java/org/apache/hadoop/hbase/HTableDescriptor.java af89e3e

      src/main/java/org/apache/hadoop/hbase/client/HConnectionManager.java 5cac9af

      src/main/java/org/apache/hadoop/hbase/io/Reference.java 6360059

      src/main/java/org/apache/hadoop/hbase/mapreduce/LoadIncrementalHFiles.java 9e4ada9

      src/main/java/org/apache/hadoop/hbase/master/HMaster.java 947ec5f

      src/main/java/org/apache/hadoop/hbase/master/MasterFileSystem.java 5052878

      src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java ccc964e

      src/main/java/org/apache/hadoop/hbase/protobuf/RequestConverter.java dabfbab

      src/main/java/org/apache/hadoop/hbase/protobuf/ResponseConverter.java 45cb6cf

      src/main/java/org/apache/hadoop/hbase/protobuf/generated/ClusterIdProtos.java PRE-CREATION

      src/main/java/org/apache/hadoop/hbase/protobuf/generated/FSProtos.java PRE-CREATION

      src/main/java/org/apache/hadoop/hbase/protobuf/generated/HBaseProtos.java 058c006

      src/main/java/org/apache/hadoop/hbase/protobuf/generated/ZooKeeperProtos.java 20c7738

      src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java 9f16fee

      src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 6dc0517

      src/main/java/org/apache/hadoop/hbase/regionserver/SplitTransaction.java 6a9f2fe

      src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.java 5e1e16d

      src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java 5050df0

      src/main/java/org/apache/hadoop/hbase/security/token/AuthenticationTokenSecretManager.java 049ed8d

      src/main/java/org/apache/hadoop/hbase/util/Bytes.java 2ccda17

      src/main/java/org/apache/hadoop/hbase/util/FSTableDescriptors.java efb2b84

      src/main/java/org/apache/hadoop/hbase/util/FSUtils.java 3d35d3e

      src/main/java/org/apache/hadoop/hbase/util/HBaseFsck.java 7b4f4a2

      src/main/java/org/apache/hadoop/hbase/util/Writables.java 3d20723

      src/main/java/org/apache/hadoop/hbase/zookeeper/ClusterId.java f804810

      src/main/java/org/apache/hadoop/hbase/zookeeper/ZKClusterId.java PRE-CREATION

      src/main/protobuf/ClusterId.proto PRE-CREATION

      src/main/protobuf/FS.proto PRE-CREATION

      src/main/protobuf/ZooKeeper.proto b72cb28

      src/main/protobuf/hbase.proto 30a4c3f

      src/test/java/org/apache/hadoop/hbase/TestHColumnDescriptor.java e7fa8b2

      src/test/java/org/apache/hadoop/hbase/TestHTableDescriptor.java f7c0cca

      src/test/java/org/apache/hadoop/hbase/catalog/TestCatalogTracker.java 83d8408

      src/test/java/org/apache/hadoop/hbase/master/MockRegionServer.java 69ccc65

      src/test/java/org/apache/hadoop/hbase/master/TestCatalogJanitor.java 1020374

      src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegionInfo.java 6dfba41

      src/test/java/org/apache/hadoop/hbase/regionserver/TestSplitTransaction.java 98c09eb

      src/test/java/org/apache/hadoop/hbase/regionserver/TestStoreFile.java 988d0bf

      src/test/java/org/apache/hadoop/hbase/util/TestFSUtils.java 339a120

      Diff: https://reviews.apache.org/r/5130/diff

      Testing

      -------

      Thanks,

      Michael

      Show
      jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/5130/#review7963 ----------------------------------------------------------- Ship it! src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java < https://reviews.apache.org/r/5130/#comment17303 > Your logic sounds correct and I agree we should avoid having to read the file each time. It would be cool to just see an example of the old writable vs new pb size to ensure our intuition is correct. Worst case (let's say they happen to be the same) we could write out the new pb stuff concatenated with the old way of writing and then they are guaranteed to be different sizes. Hopefully we don't have to do something like that, though. Gregory On 2012-05-17 05:58:11, Michael Stack wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/5130/ ----------------------------------------------------------- (Updated 2012-05-17 05:58:11) Review request for hbase. Summary ------- A b/src/main/java/org/apache/hadoop/hbase/ClusterId.java New class to hold clusterid in. M b/src/main/java/org/apache/hadoop/hbase/HColumnDescriptor.java Make it so can do pb serialization. Deprecated Writable serialization. M b/src/main/java/org/apache/hadoop/hbase/HRegionInfo.java Make it so methods in here follow the pattern in HCD an HTD pb 'ing. Deprecated Writable serialization. M b/src/main/java/org/apache/hadoop/hbase/HTableDescriptor.java Make it so can do pb serialization. Deprecated Writable serialization. M b/src/main/java/org/apache/hadoop/hbase/client/HConnectionManager.java ClusterId under ZK got renamed as ZKClusterId M b/src/main/java/org/apache/hadoop/hbase/io/Reference.java Hide the Reference#Range enums. Don't let them out of this class. Make it so can do pb serialization. M b/src/main/java/org/apache/hadoop/hbase/mapreduce/LoadIncrementalHFiles.java Use new methods on Reference for getting top and bottom. M b/src/main/java/org/apache/hadoop/hbase/master/HMaster.java ClusterId under zk has been renamed ZKClusterId. Use new ClusterId class too. M b/src/main/java/org/apache/hadoop/hbase/master/MasterFileSystem.java Use new clusterid class. M b/src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java Move the RegionInfo convertion up into HRegionInfo instead of here. Added generic toDelimitedByteArray helper. M b/src/main/java/org/apache/hadoop/hbase/protobuf/RequestConverter.java Use HRegionInfo convertions instead. M b/src/main/java/org/apache/hadoop/hbase/protobuf/ResponseConverter.java Use HRegionInfo convertions instead. M b/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java Use new utility writing out .regioninfo files. M b/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java Formatting. M b/src/main/java/org/apache/hadoop/hbase/regionserver/SplitTransaction.java M b/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.java Range in Reference is no longer public. Range in Reference is no longer public. M b/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java M b/src/main/java/org/apache/hadoop/hbase/security/token/AuthenticationTokenSecretManager.java ClusterId got renamed ZKClusterId M b/src/main/java/org/apache/hadoop/hbase/util/FSTableDescriptors.java Use new serialization utlity in HTD. M b/src/main/java/org/apache/hadoop/hbase/util/FSUtils.java Generic method for writing dot file content. M b/src/main/java/org/apache/hadoop/hbase/util/HBaseFsck.java Reference#Range is not public any more M b/src/main/java/org/apache/hadoop/hbase/util/Writables.java Deprecated getHRegionInfo, etc. D b/src/main/java/org/apache/hadoop/hbase/zookeeper/ClusterId.java A b/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKClusterId.java Rename A b/src/main/protobuf/ClusterId.proto Added file for ClusterId only since its written to fs and to zk. A b/src/main/protobuf/FS.proto Protos for fs files. M b/src/main/protobuf/ZooKeeper.proto Moved ClusterId out to own proto file M b/src/main/protobuf/hbase.proto Added TableSchema and ColumnFamilySchema This addresses bug hbase-5453. https://issues.apache.org/jira/browse/hbase-5453 Diffs ----- src/main/java/org/apache/hadoop/hbase/ClusterId.java PRE-CREATION src/main/java/org/apache/hadoop/hbase/HColumnDescriptor.java 5862f15 src/main/java/org/apache/hadoop/hbase/HRegionInfo.java 8d83ff3 src/main/java/org/apache/hadoop/hbase/HTableDescriptor.java af89e3e src/main/java/org/apache/hadoop/hbase/client/HConnectionManager.java 5cac9af src/main/java/org/apache/hadoop/hbase/io/Reference.java 6360059 src/main/java/org/apache/hadoop/hbase/mapreduce/LoadIncrementalHFiles.java 9e4ada9 src/main/java/org/apache/hadoop/hbase/master/HMaster.java 947ec5f src/main/java/org/apache/hadoop/hbase/master/MasterFileSystem.java 5052878 src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java ccc964e src/main/java/org/apache/hadoop/hbase/protobuf/RequestConverter.java dabfbab src/main/java/org/apache/hadoop/hbase/protobuf/ResponseConverter.java 45cb6cf src/main/java/org/apache/hadoop/hbase/protobuf/generated/ClusterIdProtos.java PRE-CREATION src/main/java/org/apache/hadoop/hbase/protobuf/generated/FSProtos.java PRE-CREATION src/main/java/org/apache/hadoop/hbase/protobuf/generated/HBaseProtos.java 058c006 src/main/java/org/apache/hadoop/hbase/protobuf/generated/ZooKeeperProtos.java 20c7738 src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java 9f16fee src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 6dc0517 src/main/java/org/apache/hadoop/hbase/regionserver/SplitTransaction.java 6a9f2fe src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.java 5e1e16d src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java 5050df0 src/main/java/org/apache/hadoop/hbase/security/token/AuthenticationTokenSecretManager.java 049ed8d src/main/java/org/apache/hadoop/hbase/util/Bytes.java 2ccda17 src/main/java/org/apache/hadoop/hbase/util/FSTableDescriptors.java efb2b84 src/main/java/org/apache/hadoop/hbase/util/FSUtils.java 3d35d3e src/main/java/org/apache/hadoop/hbase/util/HBaseFsck.java 7b4f4a2 src/main/java/org/apache/hadoop/hbase/util/Writables.java 3d20723 src/main/java/org/apache/hadoop/hbase/zookeeper/ClusterId.java f804810 src/main/java/org/apache/hadoop/hbase/zookeeper/ZKClusterId.java PRE-CREATION src/main/protobuf/ClusterId.proto PRE-CREATION src/main/protobuf/FS.proto PRE-CREATION src/main/protobuf/ZooKeeper.proto b72cb28 src/main/protobuf/hbase.proto 30a4c3f src/test/java/org/apache/hadoop/hbase/TestHColumnDescriptor.java e7fa8b2 src/test/java/org/apache/hadoop/hbase/TestHTableDescriptor.java f7c0cca src/test/java/org/apache/hadoop/hbase/catalog/TestCatalogTracker.java 83d8408 src/test/java/org/apache/hadoop/hbase/master/MockRegionServer.java 69ccc65 src/test/java/org/apache/hadoop/hbase/master/TestCatalogJanitor.java 1020374 src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegionInfo.java 6dfba41 src/test/java/org/apache/hadoop/hbase/regionserver/TestSplitTransaction.java 98c09eb src/test/java/org/apache/hadoop/hbase/regionserver/TestStoreFile.java 988d0bf src/test/java/org/apache/hadoop/hbase/util/TestFSUtils.java 339a120 Diff: https://reviews.apache.org/r/5130/diff Testing ------- Thanks, Michael
      Hide
      stack added a comment -

      Fixed nasty bug. Default value system and deserialization of HColumnDescriptor clashed. Resulting in cryptic odd failed scanner issue. Yuck.

      Show
      stack added a comment - Fixed nasty bug. Default value system and deserialization of HColumnDescriptor clashed. Resulting in cryptic odd failed scanner issue. Yuck.
      Hide
      Hadoop QA added a comment -

      -1 overall. Here are the results of testing the latest attachment
      http://issues.apache.org/jira/secure/attachment/12528107/5453v12.txt
      against trunk revision .

      +1 @author. The patch does not contain any @author tags.

      +1 tests included. The patch appears to include 30 new or modified tests.

      +1 hadoop23. The patch compiles against the hadoop 0.23.x profile.

      +1 javadoc. The javadoc tool did not generate any warning messages.

      +1 javac. The applied patch does not increase the total number of javac compiler warnings.

      -1 findbugs. The patch appears to introduce 32 new Findbugs (version 1.3.9) warnings.

      +1 release audit. The applied patch does not increase the total number of release audit warnings.

      -1 core tests. The patch failed these unit tests:
      org.apache.hadoop.hbase.util.hbck.TestOfflineMetaRebuildOverlap
      org.apache.hadoop.hbase.replication.TestReplication
      org.apache.hadoop.hbase.regionserver.TestSplitTransactionOnCluster
      org.apache.hadoop.hbase.replication.TestMultiSlaveReplication
      org.apache.hadoop.hbase.coprocessor.TestClassLoading
      org.apache.hadoop.hbase.replication.TestMasterReplication

      Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/1934//testReport/
      Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/1934//artifact/trunk/patchprocess/newPatchFindbugsWarnings.html
      Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/1934//console

      This message is automatically generated.

      Show
      Hadoop QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12528107/5453v12.txt against trunk revision . +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 30 new or modified tests. +1 hadoop23. The patch compiles against the hadoop 0.23.x profile. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. -1 findbugs. The patch appears to introduce 32 new Findbugs (version 1.3.9) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. -1 core tests. The patch failed these unit tests: org.apache.hadoop.hbase.util.hbck.TestOfflineMetaRebuildOverlap org.apache.hadoop.hbase.replication.TestReplication org.apache.hadoop.hbase.regionserver.TestSplitTransactionOnCluster org.apache.hadoop.hbase.replication.TestMultiSlaveReplication org.apache.hadoop.hbase.coprocessor.TestClassLoading org.apache.hadoop.hbase.replication.TestMasterReplication Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/1934//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/1934//artifact/trunk/patchprocess/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/1934//console This message is automatically generated.
      Hide
      stack added a comment -

      Fix the hbck failure (was creating a .regioninfo using old serializations)

      Show
      stack added a comment - Fix the hbck failure (was creating a .regioninfo using old serializations)
      Hide
      Hadoop QA added a comment -

      -1 overall. Here are the results of testing the latest attachment
      http://issues.apache.org/jira/secure/attachment/12528129/5453v13.txt
      against trunk revision .

      +1 @author. The patch does not contain any @author tags.

      +1 tests included. The patch appears to include 33 new or modified tests.

      +1 hadoop23. The patch compiles against the hadoop 0.23.x profile.

      +1 javadoc. The javadoc tool did not generate any warning messages.

      +1 javac. The applied patch does not increase the total number of javac compiler warnings.

      -1 findbugs. The patch appears to introduce 33 new Findbugs (version 1.3.9) warnings.

      +1 release audit. The applied patch does not increase the total number of release audit warnings.

      -1 core tests. The patch failed these unit tests:
      org.apache.hadoop.hbase.regionserver.TestColumnSeeking

      Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/1937//testReport/
      Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/1937//artifact/trunk/patchprocess/newPatchFindbugsWarnings.html
      Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/1937//console

      This message is automatically generated.

      Show
      Hadoop QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12528129/5453v13.txt against trunk revision . +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 33 new or modified tests. +1 hadoop23. The patch compiles against the hadoop 0.23.x profile. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. -1 findbugs. The patch appears to introduce 33 new Findbugs (version 1.3.9) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. -1 core tests. The patch failed these unit tests: org.apache.hadoop.hbase.regionserver.TestColumnSeeking Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/1937//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/1937//artifact/trunk/patchprocess/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/1937//console This message is automatically generated.
      Hide
      stack added a comment -

      The failed test passes locally. Will commit soon unless objection.

      Show
      stack added a comment - The failed test passes locally. Will commit soon unless objection.
      Hide
      Ted Yu added a comment -
      +   * @return This instance serialized with pb with pb magic prefix
      

      Remove redundant 'with pb '.

      +   * @return Convert this instance to a the pb column family type
      

      Remove 'a '

      Show
      Ted Yu added a comment - + * @ return This instance serialized with pb with pb magic prefix Remove redundant 'with pb '. + * @ return Convert this instance to a the pb column family type Remove 'a '
      Hide
      stack added a comment -

      Committed to trunk. Thanks for review Gregory.

      Show
      stack added a comment - Committed to trunk. Thanks for review Gregory.
      Hide
      Ted Yu added a comment - - edited

      Since integration of this JIRA, TestGetClosestAtOrBefore#testUsingMetaAndBinary has been failing since build #2902.

      Replication-related tests have been failing for Hadoop QA since build #1938.

      Show
      Ted Yu added a comment - - edited Since integration of this JIRA, TestGetClosestAtOrBefore#testUsingMetaAndBinary has been failing since build #2902. Replication-related tests have been failing for Hadoop QA since build #1938.
      Hide
      Hudson added a comment -

      Integrated in HBase-TRUNK #2919 (See https://builds.apache.org/job/HBase-TRUNK/2919/)
      HBASE-6063 Replication related failures on trunk after HBASE-5453 (Gregory Channan) (Revision 1342095)

      Result = FAILURE
      larsh :
      Files :

      • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/ClusterId.java
      Show
      Hudson added a comment - Integrated in HBase-TRUNK #2919 (See https://builds.apache.org/job/HBase-TRUNK/2919/ ) HBASE-6063 Replication related failures on trunk after HBASE-5453 (Gregory Channan) (Revision 1342095) Result = FAILURE larsh : Files : /hbase/trunk/src/main/java/org/apache/hadoop/hbase/ClusterId.java
      Hide
      Hudson added a comment -

      Integrated in HBase-TRUNK-on-Hadoop-2.0.0 #16 (See https://builds.apache.org/job/HBase-TRUNK-on-Hadoop-2.0.0/16/)
      HBASE-6063 Replication related failures on trunk after HBASE-5453 (Gregory Channan) (Revision 1342095)

      Result = FAILURE
      larsh :
      Files :

      • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/ClusterId.java
      Show
      Hudson added a comment - Integrated in HBase-TRUNK-on-Hadoop-2.0.0 #16 (See https://builds.apache.org/job/HBase-TRUNK-on-Hadoop-2.0.0/16/ ) HBASE-6063 Replication related failures on trunk after HBASE-5453 (Gregory Channan) (Revision 1342095) Result = FAILURE larsh : Files : /hbase/trunk/src/main/java/org/apache/hadoop/hbase/ClusterId.java
      Hide
      stack added a comment -

      Marking closed.

      Show
      stack added a comment - Marking closed.

        People

        • Assignee:
          stack
          Reporter:
          Todd Lipcon
        • Votes:
          0 Vote for this issue
          Watchers:
          12 Start watching this issue

          Dates

          • Created:
            Updated:
            Resolved:

            Development