HBase
  1. HBase
  2. HBASE-1989

Admin (et al.) not accurate with Column vs. Column-Family usage

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 0.20.1, 0.90.1
    • Fix Version/s: 2.0.0
    • Component/s: Client
    • Labels:
      None
    • Hadoop Flags:
      Reviewed

      Description

      Consider the classes Admin and HColumnDescriptor.

      HColumnDescriptor is really referring to a "column family" and not a "column" (i.e., family:qualifer).

      Likewise, in Admin there is a method called "addColumn" that takes an HColumnDescriptor instance.

      I labeled this a bug in the sense that it produces conceptual confusion because there is a big difference between a column and column-family in HBase and these terms should be used consistently. The code works, though.

      1. hbase1989.patch
        4 kB
        Lars Francke
      2. HBASE-1989.patch
        38 kB
        Lars Francke
      3. HBASE-1989-v1.patch
        38 kB
        Lars Francke

        Issue Links

          Activity

          Hide
          Hudson added a comment -

          FAILURE: Integrated in HBase-TRUNK #6473 (See https://builds.apache.org/job/HBase-TRUNK/6473/)
          HBASE-1989 Admin (et al.) not accurate with Column vs. Column-Family (stack: rev ec51d7b2e600fab2c3490e794dd13e27ec1cee01)

          • hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestRestoreSnapshotFromClient.java
          • hbase-client/src/main/java/org/apache/hadoop/hbase/client/HBaseAdmin.java
          • hbase-client/src/main/java/org/apache/hadoop/hbase/client/Admin.java
          • hbase-server/src/test/java/org/apache/hadoop/hbase/master/handler/TestTableDescriptorModification.java
          • hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestSnapshotCloneIndependence.java
          • hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestTableLockManager.java
          • hbase-it/src/test/java/org/apache/hadoop/hbase/IntegrationTestIngestWithEncryption.java
          • hbase-server/src/test/java/org/apache/hadoop/hbase/util/LoadTestTool.java
          • hbase-server/src/test/java/org/apache/hadoop/hbase/io/encoding/TestChangingEncoding.java
          • hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestEncryptionKeyRotation.java
          • hbase-server/src/test/java/org/apache/hadoop/hbase/security/visibility/TestVisibilityLabels.java
          • hbase-rest/src/main/java/org/apache/hadoop/hbase/rest/SchemaResource.java
          • hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestAdmin1.java
          • hbase-server/src/test/java/org/apache/hadoop/hbase/backup/TestHFileArchiving.java
          • hbase-server/src/test/java/org/apache/hadoop/hbase/coprocessor/TestMasterObserver.java
          • hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestSnapshotMetadata.java
          • hbase-server/src/test/java/org/apache/hadoop/hbase/master/handler/TestTableDeleteFamilyHandler.java
          • hbase-server/src/test/java/org/apache/hadoop/hbase/io/encoding/TestLoadAndSwitchEncodeOnDisk.java
          Show
          Hudson added a comment - FAILURE: Integrated in HBase-TRUNK #6473 (See https://builds.apache.org/job/HBase-TRUNK/6473/ ) HBASE-1989 Admin (et al.) not accurate with Column vs. Column-Family (stack: rev ec51d7b2e600fab2c3490e794dd13e27ec1cee01) hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestRestoreSnapshotFromClient.java hbase-client/src/main/java/org/apache/hadoop/hbase/client/HBaseAdmin.java hbase-client/src/main/java/org/apache/hadoop/hbase/client/Admin.java hbase-server/src/test/java/org/apache/hadoop/hbase/master/handler/TestTableDescriptorModification.java hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestSnapshotCloneIndependence.java hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestTableLockManager.java hbase-it/src/test/java/org/apache/hadoop/hbase/IntegrationTestIngestWithEncryption.java hbase-server/src/test/java/org/apache/hadoop/hbase/util/LoadTestTool.java hbase-server/src/test/java/org/apache/hadoop/hbase/io/encoding/TestChangingEncoding.java hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestEncryptionKeyRotation.java hbase-server/src/test/java/org/apache/hadoop/hbase/security/visibility/TestVisibilityLabels.java hbase-rest/src/main/java/org/apache/hadoop/hbase/rest/SchemaResource.java hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestAdmin1.java hbase-server/src/test/java/org/apache/hadoop/hbase/backup/TestHFileArchiving.java hbase-server/src/test/java/org/apache/hadoop/hbase/coprocessor/TestMasterObserver.java hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestSnapshotMetadata.java hbase-server/src/test/java/org/apache/hadoop/hbase/master/handler/TestTableDeleteFamilyHandler.java hbase-server/src/test/java/org/apache/hadoop/hbase/io/encoding/TestLoadAndSwitchEncodeOnDisk.java
          Hide
          stack added a comment -

          Reverted because of bad commit message (Thanks again Sean Busbey) and then reapplied.

          Show
          stack added a comment - Reverted because of bad commit message (Thanks again Sean Busbey ) and then reapplied.
          Hide
          Lars Francke added a comment -

          Thanks stack!

          Sean Busbey I do not object, thank you for catching this.

          Show
          Lars Francke added a comment - Thanks stack! Sean Busbey I do not object, thank you for catching this.
          Hide
          Sean Busbey added a comment -

          this got pushed without a proper commit message (missing jira reference). any objection to revert + reapply with commit?

          Show
          Sean Busbey added a comment - this got pushed without a proper commit message (missing jira reference). any objection to revert + reapply with commit?
          Hide
          stack added a comment -

          Pushed to master. Thanks Lars Francke Nice.

          Show
          stack added a comment - Pushed to master. Thanks Lars Francke Nice.
          Hide
          Lars Francke added a comment -

          Anoop Sam John Thanks for taking a look. Jenkins agrees with you. I think this is good to commit if there are no further comments.

          Show
          Lars Francke added a comment - Anoop Sam John Thanks for taking a look. Jenkins agrees with you. I think this is good to commit if there are no further comments.
          Hide
          Hadoop QA added a comment -

          +1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12730830/HBASE-1989-v1.patch
          against master branch at commit 652929c0ff8c8cec1e86ded834f3e770422b2ace.
          ATTACHMENT ID: 12730830

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

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

          +1 hadoop versions. The patch compiles with all supported hadoop versions (2.4.1 2.5.2 2.6.0)

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

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

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

          +1 checkstyle. The applied patch does not increase the total number of checkstyle errors

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

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

          +1 lineLengths. The patch does not introduce lines longer than 100

          +1 site. The mvn site goal succeeds with this patch.

          +1 core tests. The patch passed unit tests in .

          Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/13962//testReport/
          Release Findbugs (version 2.0.3) warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13962//artifact/patchprocess/newFindbugsWarnings.html
          Checkstyle Errors: https://builds.apache.org/job/PreCommit-HBASE-Build/13962//artifact/patchprocess/checkstyle-aggregate.html

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

          This message is automatically generated.

          Show
          Hadoop QA added a comment - +1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12730830/HBASE-1989-v1.patch against master branch at commit 652929c0ff8c8cec1e86ded834f3e770422b2ace. ATTACHMENT ID: 12730830 +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 58 new or modified tests. +1 hadoop versions . The patch compiles with all supported hadoop versions (2.4.1 2.5.2 2.6.0) +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 protoc . The applied patch does not increase the total number of protoc compiler warnings. +1 javadoc . The javadoc tool did not generate any warning messages. +1 checkstyle . The applied patch does not increase the total number of checkstyle errors +1 findbugs . The patch does not introduce any new Findbugs (version 2.0.3) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. +1 lineLengths . The patch does not introduce lines longer than 100 +1 site . The mvn site goal succeeds with this patch. +1 core tests . The patch passed unit tests in . Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/13962//testReport/ Release Findbugs (version 2.0.3) warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13962//artifact/patchprocess/newFindbugsWarnings.html Checkstyle Errors: https://builds.apache.org/job/PreCommit-HBASE-Build/13962//artifact/patchprocess/checkstyle-aggregate.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/13962//console This message is automatically generated.
          Hide
          Anoop Sam John added a comment -

          Latest patch LGTM

          Show
          Anoop Sam John added a comment - Latest patch LGTM
          Hide
          Lars Francke added a comment -

          Thanks for taking a look and the comments.

          I've attached v1 of the patch that should fix the Checkstyle warning and addresses your comments. I've renamed all parameters I could find in those two classes to columnFamily and I've removed those extra methods I added.

          Show
          Lars Francke added a comment - Thanks for taking a look and the comments. I've attached v1 of the patch that should fix the Checkstyle warning and addresses your comments. I've renamed all parameters I could find in those two classes to columnFamily and I've removed those extra methods I added.
          Hide
          Anoop Sam John added a comment -

          void deleteColumnFamily(final TableName tableName, final byte[] columnName) throws IOException

          Can you make the param name as 'columnFamily '

          HBaseAdmin.java

          addColumnFamily(final byte[] tableName, HColumnDescriptor columnFamily)
          addColumnFamily(final String tableName, HColumnDescriptor columnFamily)

          May be no need to add newly. Just deprecate its counterparts with replacement as
          addColumnFamily(final TableName tableName, final HColumnDescriptor columnFamily)

          Same case applicable for delete/modify

          Show
          Anoop Sam John added a comment - void deleteColumnFamily(final TableName tableName, final byte[] columnName) throws IOException Can you make the param name as 'columnFamily ' HBaseAdmin.java addColumnFamily(final byte[] tableName, HColumnDescriptor columnFamily) addColumnFamily(final String tableName, HColumnDescriptor columnFamily) May be no need to add newly. Just deprecate its counterparts with replacement as addColumnFamily(final TableName tableName, final HColumnDescriptor columnFamily) Same case applicable for delete/modify
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12730594/HBASE-1989.patch
          against master branch at commit 977f867439e960c668ee6311e47c904efc40f219.
          ATTACHMENT ID: 12730594

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

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

          +1 hadoop versions. The patch compiles with all supported hadoop versions (2.4.1 2.5.2 2.6.0)

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

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

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

          -1 checkstyle. The applied patch generated 1900 checkstyle errors (more than the master's current 1896 errors).

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

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

          +1 lineLengths. The patch does not introduce lines longer than 100

          +1 site. The mvn site goal succeeds with this patch.

          +1 core tests. The patch passed unit tests in .

          Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/13946//testReport/
          Release Findbugs (version 2.0.3) warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13946//artifact/patchprocess/newFindbugsWarnings.html
          Checkstyle Errors: https://builds.apache.org/job/PreCommit-HBASE-Build/13946//artifact/patchprocess/checkstyle-aggregate.html

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

          This message is automatically generated.

          Show
          Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12730594/HBASE-1989.patch against master branch at commit 977f867439e960c668ee6311e47c904efc40f219. ATTACHMENT ID: 12730594 +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 58 new or modified tests. +1 hadoop versions . The patch compiles with all supported hadoop versions (2.4.1 2.5.2 2.6.0) +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 protoc . The applied patch does not increase the total number of protoc compiler warnings. +1 javadoc . The javadoc tool did not generate any warning messages. -1 checkstyle . The applied patch generated 1900 checkstyle errors (more than the master's current 1896 errors). +1 findbugs . The patch does not introduce any new Findbugs (version 2.0.3) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. +1 lineLengths . The patch does not introduce lines longer than 100 +1 site . The mvn site goal succeeds with this patch. +1 core tests . The patch passed unit tests in . Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/13946//testReport/ Release Findbugs (version 2.0.3) warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13946//artifact/patchprocess/newFindbugsWarnings.html Checkstyle Errors: https://builds.apache.org/job/PreCommit-HBASE-Build/13946//artifact/patchprocess/checkstyle-aggregate.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/13946//console This message is automatically generated.
          Hide
          Lars Francke added a comment -

          Attaching a patch that fixes all methods in Admin and HBaseAdmin.

          This does not change HColumnDescriptor as that'd take too long for now. If I have time I'll do that in another issue.

          Show
          Lars Francke added a comment - Attaching a patch that fixes all methods in Admin and HBaseAdmin . This does not change HColumnDescriptor as that'd take too long for now. If I have time I'll do that in another issue.
          Hide
          Lars Francke added a comment -

          I'd like to work on this again because the naming is still confusing. I'll reopen and attach a patch that'll deprecate the "old" versions in 2.0.0 so they can be removed in 3.0.0.

          Show
          Lars Francke added a comment - I'd like to work on this again because the naming is still confusing. I'll reopen and attach a patch that'll deprecate the "old" versions in 2.0.0 so they can be removed in 3.0.0.
          Hide
          Andrew Purtell added a comment -

          Stale issue. Reopen if still relevant (if there's new activity)

          Show
          Andrew Purtell added a comment - Stale issue. Reopen if still relevant (if there's new activity)
          Hide
          Doug Meil added a comment -

          Wow, this is from the way-back machine (2009). But I still think it's a good idea from a terminology standpoint to clear this up in the API

          Show
          Doug Meil added a comment - Wow, this is from the way-back machine (2009). But I still think it's a good idea from a terminology standpoint to clear this up in the API
          Hide
          stack added a comment -

          Stale

          Show
          stack added a comment - Stale
          Hide
          stack added a comment -

          Marking as patch available. Has work started by LarsF.

          Show
          stack added a comment - Marking as patch available. Has work started by LarsF.
          Hide
          stack added a comment -

          This is a start. Will I commit it as a start in on this issue, as a part 1? I'm working on the other part over in master rewrite where HColumnDescriptor becomes instead ColumnFamilyDefinition, etc., throughout the code base.

          Show
          stack added a comment - This is a start. Will I commit it as a start in on this issue, as a part 1? I'm working on the other part over in master rewrite where HColumnDescriptor becomes instead ColumnFamilyDefinition, etc., throughout the code base.
          Hide
          Lars Francke added a comment -

          I did a small part of this before I found this issue. I only changed HBaseAdmin - this patch deprecates the old methods and changes their usage.

          But as I said: This is only a small part of the renaming process but perhaps it is useful anyway

          Show
          Lars Francke added a comment - I did a small part of this before I found this issue. I only changed HBaseAdmin - this patch deprecates the old methods and changes their usage. But as I said: This is only a small part of the renaming process but perhaps it is useful anyway
          Hide
          stack added a comment -

          I'll take this one. Redoing HCD any ways to go against ZK instead of as a Writable so can change it then.

          Show
          stack added a comment - I'll take this one. Redoing HCD any ways to go against ZK instead of as a Writable so can change it then.

            People

            • Assignee:
              Lars Francke
              Reporter:
              Doug Meil
            • Votes:
              1 Vote for this issue
              Watchers:
              9 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development