Uploaded image for project: 'HBase'
  1. HBase
  2. HBASE-5305 Improve cross-version compatibility & upgradeability
  3. HBASE-5453

Switch on-disk formats (reference files, HFile meta fields, etc) to PB

        Details

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

          Activity

          Ascending order - Click to sort in descending order
          Hide
          Permalink
          stack 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 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
          Permalink
          eclark Elliott Clark added a comment -

          Are you talking about KeyValue as well ?

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

          no to kv

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

          Move over the hbase.version file.

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

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

          Show
          jxiang Jimmy Xiang added a comment - Good stuff. Are the proto file and the generated java file in a different patch?
          Hide
          Permalink
          stack 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 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
          Permalink
          jxiang Jimmy Xiang added a comment -

          That would be great, I think. Thanks.

          Show
          jxiang Jimmy Xiang added a comment - That would be great, I think. Thanks.
          Hide
          Permalink
          hadoopqa 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
          hadoopqa 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
          Permalink
          tlipcon 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
          tlipcon 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
          Permalink
          jxiang Jimmy Xiang added a comment -

          v2 looks good to me.

          Show
          jxiang Jimmy Xiang added a comment - v2 looks good to me.
          Hide
          Permalink
          stack 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 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
          Permalink
          hadoopqa 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
          hadoopqa 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
          Permalink
          jiraposter@reviews.apache.org 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 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
          Permalink
          jiraposter@reviews.apache.org 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 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
          Permalink
          stack 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 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
          Permalink
          jiraposter@reviews.apache.org 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 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
          Permalink
          stack 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 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
          Permalink
          jiraposter@reviews.apache.org 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 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
          Permalink
          stack stack added a comment -

          Fix bug in my Reference file serialize/deserialize

          Show
          stack stack added a comment - Fix bug in my Reference file serialize/deserialize
          Hide
          Permalink
          jiraposter@reviews.apache.org 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 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
          Permalink
          jiraposter@reviews.apache.org 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 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
          Permalink
          jiraposter@reviews.apache.org 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 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
          Permalink
          jiraposter@reviews.apache.org 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 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
          Permalink
          jiraposter@reviews.apache.org 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 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
          Permalink
          stack stack added a comment -

          v11 addressing Gregory's last review comments.

          Show
          stack stack added a comment - v11 addressing Gregory's last review comments.
          Hide
          Permalink
          jiraposter@reviews.apache.org 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 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
          Permalink
          hadoopqa 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
          hadoopqa 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
          Permalink
          jiraposter@reviews.apache.org 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 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
          Permalink
          stack 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 stack added a comment - Fixed nasty bug. Default value system and deserialization of HColumnDescriptor clashed. Resulting in cryptic odd failed scanner issue. Yuck.
          Hide
          Permalink
          hadoopqa 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
          hadoopqa 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
          Permalink
          stack stack added a comment -

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

          Show
          stack stack added a comment - Fix the hbck failure (was creating a .regioninfo using old serializations)
          Hide
          Permalink
          hadoopqa 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
          hadoopqa 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
          Permalink
          stack stack added a comment -

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

          Show
          stack stack added a comment - The failed test passes locally. Will commit soon unless objection.
          Hide
          Permalink
          zhihyu@ebaysf.com 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
          zhihyu@ebaysf.com 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
          Permalink
          stack stack added a comment -

          Committed to trunk. Thanks for review Gregory.

          Show
          stack stack added a comment - Committed to trunk. Thanks for review Gregory.
          Hide
          Permalink
          zhihyu@ebaysf.com 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
          zhihyu@ebaysf.com 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
          Permalink
          hudson 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 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
          Permalink
          hudson 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 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
          Permalink
          stack stack added a comment -

          Marking closed.

          Show
          stack stack added a comment - Marking closed.

            People

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

              Dates

              • Created:
                Updated:
                Resolved:

                Development