Uploaded image for project: 'HBase'
  1. HBase
  2. HBASE-1976

HBaseConfiguration implements hashCode() without implementing equals()

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Major
    • Resolution: Invalid
    • Affects Version/s: 0.20.1
    • Fix Version/s: 0.90.0
    • Component/s: None
    • Labels:
      None
    • Environment:

      HBase

      Description

      I was browsing though the HBase 0.20.1 code in order to learn about the way HBase deals with Configuration and I noticed that HBaseConfiguration overrides hashCode() without implementing equals(). This can cause some tricky, hard to debug problems whenever instances of this class are added to Maps or HashSets.

        Issue Links

          Activity

          Hide
          stack stack added a comment -

          Good one.

          That we make a hashcode on the HBaseConfiguration looks kinda broke to me; that its done w/o a companion equals is doubly broke.

          Hadoop Configuration does not hashcode nor equals. Doing this on a Configuration object, especially a fat one like the Hadoop Configuration would be expensive (might hash the paths of the files read to make up the Configuration). Seems like folks have been able to live w/o wanting to use an Hadoop Configuration as a key in a map or without having to call equals on the config. That seems reasonable.

          Only in hbase, we do use HBaseConfiguration as a key. Its put into a static hash in the client. The HBC instance is used to key the Map of cached regions. The way it does this – keying caches by HBC instance – is broke too.

          Lets REMOVE hashcode from HBC. Having other than object equivalence would be expensive to maintain. Need to fix client so it doesn't use HBC as a key as part of this issue.

          Show
          stack stack added a comment - Good one. That we make a hashcode on the HBaseConfiguration looks kinda broke to me; that its done w/o a companion equals is doubly broke. Hadoop Configuration does not hashcode nor equals. Doing this on a Configuration object, especially a fat one like the Hadoop Configuration would be expensive (might hash the paths of the files read to make up the Configuration). Seems like folks have been able to live w/o wanting to use an Hadoop Configuration as a key in a map or without having to call equals on the config. That seems reasonable. Only in hbase, we do use HBaseConfiguration as a key. Its put into a static hash in the client. The HBC instance is used to key the Map of cached regions. The way it does this – keying caches by HBC instance – is broke too. Lets REMOVE hashcode from HBC. Having other than object equivalence would be expensive to maintain. Need to fix client so it doesn't use HBC as a key as part of this issue.
          Hide
          agemooij Age Mooij added a comment -

          I found HBASE-1251 as the probable reason why the hasCode() was added. It's probably a good idea to use that issue as a regression test when removing the hashCode method since it was obviously added to fix another bug.

          Show
          agemooij Age Mooij added a comment - I found HBASE-1251 as the probable reason why the hasCode() was added. It's probably a good idea to use that issue as a regression test when removing the hashCode method since it was obviously added to fix another bug.
          Hide
          stack stack added a comment -

          Resolving invalid. HBaseConfiguration had an equals added a good while ago which is what this issue is about. Adding equals and a hashcode in the first place was probably a mistake. Will likely be backed out by hbase-2925, but regardless, this issue was 'addressed' a while back. Closing. Thanks for filing Age.

          Show
          stack stack added a comment - Resolving invalid. HBaseConfiguration had an equals added a good while ago which is what this issue is about. Adding equals and a hashcode in the first place was probably a mistake. Will likely be backed out by hbase-2925, but regardless, this issue was 'addressed' a while back. Closing. Thanks for filing Age.
          Hide
          lars_francke Lars Francke added a comment -

          This issue was closed as part of a bulk closing operation on 2015-11-20. All issues that have been resolved and where all fixVersions have been released have been closed (following discussions on the mailing list).

          Show
          lars_francke Lars Francke added a comment - This issue was closed as part of a bulk closing operation on 2015-11-20. All issues that have been resolved and where all fixVersions have been released have been closed (following discussions on the mailing list).

            People

            • Assignee:
              Unassigned
              Reporter:
              agemooij Age Mooij
            • Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development