Uploaded image for project: 'Phoenix'
  1. Phoenix
  2. PHOENIX-3116

Support incompatible HBase 1.1.5 and HBase 1.2.2

    Details

    • Type: Improvement
    • Status: Closed
    • Priority: Minor
    • Resolution: Won't Fix
    • Affects Version/s: 4.8.0
    • Fix Version/s: 4.9.0, 4.8.1
    • Labels:
      None
    • Flags:
      Patch

      Description

      HBase 1.2.2 made a backwards incompatible change in HTableInterface that requires new overrides.

      1. PHOENIX-3116_v2.patch
        3 kB
        James Taylor
      2. upgrade-hbase-to-1.2.2.patch
        2 kB
        Rob Leidle

        Issue Links

          Activity

          Hide
          jamestaylor James Taylor added a comment -

          Were new methods added to Table interface between HBase 1.2.0 and 1.2.2, Sean Busbey, Andrew Purtell? In a patch release? Phoenix 4.8 won't compile with HBase 1.2.2 now. Is that allowed on a public API? Or does the @InterfaceStability.Evolving mean that "Sure it's public, but we can change it at any time"?

          Show
          jamestaylor James Taylor added a comment - Were new methods added to Table interface between HBase 1.2.0 and 1.2.2, Sean Busbey , Andrew Purtell ? In a patch release? Phoenix 4.8 won't compile with HBase 1.2.2 now. Is that allowed on a public API? Or does the @InterfaceStability.Evolving mean that "Sure it's public, but we can change it at any time"?
          Hide
          apurtell Andrew Purtell added a comment -

          That looks like maybe HBASE-15645 .

          Curious why you at-mentioned me James Taylor as I am neither responsible for the commit nor the RM for the branch. General complaints should be directed to the PMC at dev@hbase.apache.org

          Show
          apurtell Andrew Purtell added a comment - That looks like maybe HBASE-15645 . Curious why you at-mentioned me James Taylor as I am neither responsible for the commit nor the RM for the branch. General complaints should be directed to the PMC at dev@hbase.apache.org
          Hide
          jamestaylor James Taylor added a comment -

          I pinged you Andrew Purtell because of your excellent insights. It's not clear to me what's allowed and not allowed in HBase patch releases. Would be great to understand this so we can come up with a solution as I think it's in the interest of both communities to keep Phoenix compatible with the latest HBase patch releases.

          Is it ok to add methods to public interfaces in patch releases?

          Show
          jamestaylor James Taylor added a comment - I pinged you Andrew Purtell because of your excellent insights. It's not clear to me what's allowed and not allowed in HBase patch releases. Would be great to understand this so we can come up with a solution as I think it's in the interest of both communities to keep Phoenix compatible with the latest HBase patch releases. Is it ok to add methods to public interfaces in patch releases?
          Hide
          apurtell Andrew Purtell added a comment - - edited

          Is it ok to add methods to public interfaces in patch releases?

          Needs to be clarified. See http://hbase.apache.org/book.html#hbase.versioning

          "new APIs may be added which will not be available in earlier patch versions"

          "Client code written to APIs available in a given patch release might not run against the old jars from an earlier patch version."

          "A patch upgrade is a drop-in replacement. Any change that is not Java binary compatible would not be allowed.[1]. Downgrading versions within patch releases may not be compatible."

          The above would seem to allow the Table API change under discussion here, because it is binary compatible per the Java binary compat specification. Yet

          "A minor upgrade requires no application/client code modification."

          implies source compatibility even on minor releases, which is not the intent I think.

          The compatibility matrix also suggests Evolving interfaces cannot be changed on patch releases, which I can see as intended.

          The guidelines do not address source compatibility specifically.

          Show
          apurtell Andrew Purtell added a comment - - edited Is it ok to add methods to public interfaces in patch releases? Needs to be clarified. See http://hbase.apache.org/book.html#hbase.versioning "new APIs may be added which will not be available in earlier patch versions" "Client code written to APIs available in a given patch release might not run against the old jars from an earlier patch version." "A patch upgrade is a drop-in replacement. Any change that is not Java binary compatible would not be allowed. [1] . Downgrading versions within patch releases may not be compatible." The above would seem to allow the Table API change under discussion here, because it is binary compatible per the Java binary compat specification. Yet "A minor upgrade requires no application/client code modification." implies source compatibility even on minor releases, which is not the intent I think. The compatibility matrix also suggests Evolving interfaces cannot be changed on patch releases, which I can see as intended. The guidelines do not address source compatibility specifically.
          Hide
          elserj Josh Elser added a comment -

          Argh.

          The above would seem to allow the Table API change under discussion here, because it is binary compatible per the Java binary compat specification. Yet

          This is my general understanding. HBase's compatibility statement is not what is outlined at semver.org. Instead, it's more to do with binary compatibility of the artifacts. As such, adding a method to an interface wouldn't break the binary compatibility (of the caller) which is probably how we got here. I feel like the intent is from a caller, not so much someone extending or implementing the API.

          Let me try to take this up in HBase land and see what others think there. By the current state of the guidelines, I believe this change was allowed; however, I do not like the current state of the guidelines

          Show
          elserj Josh Elser added a comment - Argh. The above would seem to allow the Table API change under discussion here, because it is binary compatible per the Java binary compat specification. Yet This is my general understanding. HBase's compatibility statement is not what is outlined at semver.org. Instead, it's more to do with binary compatibility of the artifacts. As such, adding a method to an interface wouldn't break the binary compatibility (of the caller) which is probably how we got here. I feel like the intent is from a caller, not so much someone extending or implementing the API. Let me try to take this up in HBase land and see what others think there. By the current state of the guidelines, I believe this change was allowed; however, I do not like the current state of the guidelines
          Hide
          apurtell Andrew Purtell added a comment -

          Josh Elser this specific problem would have been prevented by a guideline enforcing source compatibility of Public annotated interfaces on patch releases as determined by the API compat checker

          Show
          apurtell Andrew Purtell added a comment - Josh Elser this specific problem would have been prevented by a guideline enforcing source compatibility of Public annotated interfaces on patch releases as determined by the API compat checker
          Hide
          jamestaylor James Taylor added a comment -

          FYI, Poorna Chandra - I believe this breaks TransactionAwareHTable too. You'll need a 1.2 compat module with these methods implemented. Would it be possible to get this in a release within four weeks in time for our 4.8.1 release?

          Show
          jamestaylor James Taylor added a comment - FYI, Poorna Chandra - I believe this breaks TransactionAwareHTable too. You'll need a 1.2 compat module with these methods implemented. Would it be possible to get this in a release within four weeks in time for our 4.8.1 release?
          Show
          elserj Josh Elser added a comment - https://lists.apache.org/thread.html/ab2346f94e758bdb83a180ff44c72968354f872f1db1804ca545f1be@%3Cdev.hbase.apache.org%3E
          Hide
          elserj Josh Elser added a comment -

          this specific problem would have been prevented by a guideline enforcing source compatibility of Public annotated interfaces on patch releases as determined by the API compat checker

          Yes, agreed. That is my main problem with the compatibility guidelines HBase has on patch releases. We'll see what the broader community has to say on addressing this problem (for this specific issue and going forward).

          Show
          elserj Josh Elser added a comment - this specific problem would have been prevented by a guideline enforcing source compatibility of Public annotated interfaces on patch releases as determined by the API compat checker Yes, agreed. That is my main problem with the compatibility guidelines HBase has on patch releases. We'll see what the broader community has to say on addressing this problem (for this specific issue and going forward).
          Hide
          poornachandra Poorna Chandra added a comment -

          James Taylor We have noticed the same thing in CDAP too (https://issues.cask.co/browse/CDAP-6937). We should have an HBase 1.2 compat module for Apache Tephra in a couple of weeks. Can you file a JIRA for this?

          Show
          poornachandra Poorna Chandra added a comment - James Taylor We have noticed the same thing in CDAP too ( https://issues.cask.co/browse/CDAP-6937 ). We should have an HBase 1.2 compat module for Apache Tephra in a couple of weeks. Can you file a JIRA for this?
          Hide
          jamestaylor James Taylor added a comment -
          Show
          jamestaylor James Taylor added a comment - Created TEPHRA-178 , Poorna Chandra .
          Hide
          elserj Josh Elser added a comment -

          FYI, this was committed in HBASE-16420 to land in HBase 1.1.6 and 1.2.3.

          Show
          elserj Josh Elser added a comment - FYI, this was committed in HBASE-16420 to land in HBase 1.1.6 and 1.2.3.
          Hide
          jamestaylor James Taylor added a comment -

          Thanks for following up, Josh Elser. IMHO, we should still pursue this patch and TEPHRA-178 (without the @Override tags), so that from 4.8.1 onward Phoenix continues to work with all HBase 1.1 and 1.2 versions.

          Show
          jamestaylor James Taylor added a comment - Thanks for following up, Josh Elser . IMHO, we should still pursue this patch and TEPHRA-178 (without the @Override tags), so that from 4.8.1 onward Phoenix continues to work with all HBase 1.1 and 1.2 versions.
          Hide
          elserj Josh Elser added a comment -

          Thanks for following up, Josh Elser. IMHO, we should still pursue this patch and TEPHRA-178 (without the @Override tags), so that from 4.8.1 onward Phoenix continues to work with all HBase 1.1 and 1.2 versions.

          Agreed, consistency across all HBase versions is ideal. Makes the user's life much easier.

          Show
          elserj Josh Elser added a comment - Thanks for following up, Josh Elser. IMHO, we should still pursue this patch and TEPHRA-178 (without the @Override tags), so that from 4.8.1 onward Phoenix continues to work with all HBase 1.1 and 1.2 versions. Agreed, consistency across all HBase versions is ideal. Makes the user's life much easier.
          Hide
          jamestaylor James Taylor added a comment -

          Adjusting patch to remove @Override tags and use reflection to call delegate method since these methods will only exist in HBase 1.1.5 and HBase 1.2.2. Will need a similar change in Tephra if we want to support these HBase versions. WDYT, Poorna Chandra?

          The alternative would be to just say we don't support HBase 1.1.5 and HBase 1.2.2.

          Show
          jamestaylor James Taylor added a comment - Adjusting patch to remove @Override tags and use reflection to call delegate method since these methods will only exist in HBase 1.1.5 and HBase 1.2.2. Will need a similar change in Tephra if we want to support these HBase versions. WDYT, Poorna Chandra ? The alternative would be to just say we don't support HBase 1.1.5 and HBase 1.2.2.
          Hide
          hadoopqa Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12826212/PHOENIX-3116_v2.patch
          against master branch at commit d873c2ffd1e539ecd56858c82f0ba2d23e877cf9.
          ATTACHMENT ID: 12826212

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

          -1 tests included. The patch doesn't appear to include any new or modified tests.
          Please justify why no new tests are needed for this patch.
          Also please list what manual steps were performed to verify this patch.

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

          -1 javadoc. The javadoc tool appears to have generated 34 warning messages.

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

          -1 lineLengths. The patch introduces the following lines longer than 100:
          + // No @Override tag because it's only declared in HBase 1.2.2 and subsequently removed (see HBASE-16420).
          + // No @Override tag because it's only declared in HBase 1.2.2 and subsequently removed (see HBASE-16420).
          + // No @Override tag because it's only declared in HBase 1.2.2 and subsequently removed (see HBASE-16420).
          + // No @Override tag because it's only declared in HBase 1.2.2 and subsequently removed (see HBASE-16420).

          -1 core tests. The patch failed these unit tests:

          Test results: https://builds.apache.org/job/PreCommit-PHOENIX-Build/546//testReport/
          Javadoc warnings: https://builds.apache.org/job/PreCommit-PHOENIX-Build/546//artifact/patchprocess/patchJavadocWarnings.txt
          Console output: https://builds.apache.org/job/PreCommit-PHOENIX-Build/546//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/12826212/PHOENIX-3116_v2.patch against master branch at commit d873c2ffd1e539ecd56858c82f0ba2d23e877cf9. ATTACHMENT ID: 12826212 +1 @author . The patch does not contain any @author tags. -1 tests included . The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. +1 javac . The applied patch does not increase the total number of javac compiler warnings. -1 javadoc . The javadoc tool appears to have generated 34 warning messages. +1 release audit . The applied patch does not increase the total number of release audit warnings. -1 lineLengths . The patch introduces the following lines longer than 100: + // No @Override tag because it's only declared in HBase 1.2.2 and subsequently removed (see HBASE-16420 ). + // No @Override tag because it's only declared in HBase 1.2.2 and subsequently removed (see HBASE-16420 ). + // No @Override tag because it's only declared in HBase 1.2.2 and subsequently removed (see HBASE-16420 ). + // No @Override tag because it's only declared in HBase 1.2.2 and subsequently removed (see HBASE-16420 ). -1 core tests . The patch failed these unit tests: Test results: https://builds.apache.org/job/PreCommit-PHOENIX-Build/546//testReport/ Javadoc warnings: https://builds.apache.org/job/PreCommit-PHOENIX-Build/546//artifact/patchprocess/patchJavadocWarnings.txt Console output: https://builds.apache.org/job/PreCommit-PHOENIX-Build/546//console This message is automatically generated.
          Hide
          poornachandra Poorna Chandra added a comment -

          James Taylor Do you think those two versions of HBase will have widespread adoption? If none of the major distros ship them, then it should be okay not to support those two HBase versions. Even if one of the major distros ship them then we'll have to support it.

          Show
          poornachandra Poorna Chandra added a comment - James Taylor Do you think those two versions of HBase will have widespread adoption? If none of the major distros ship them, then it should be okay not to support those two HBase versions. Even if one of the major distros ship them then we'll have to support it.
          Hide
          jamestaylor James Taylor added a comment -

          Not sure, Poorna Chandra. Hard to know what the vendors will do. There's also folks who don't rely on the vendors, like us at SFDC - we won't use those patch versions because we know about the issue, but others may pick it up and expect it to work.

          Show
          jamestaylor James Taylor added a comment - Not sure, Poorna Chandra . Hard to know what the vendors will do. There's also folks who don't rely on the vendors, like us at SFDC - we won't use those patch versions because we know about the issue, but others may pick it up and expect it to work.
          Hide
          poornachandra Poorna Chandra added a comment -

          In that case we will have to support it.

          Should we mark the newly introduced methods in Tephra and Phoenix as deprecated as well? This is so that the users know that these methods are not supported, and don't use them. Even if we use reflection in this case, once we introduce a new method users can use the same method in other non-supported HBase versions too (if we don't introduce a new compat module for this change in the case of Tephra for instance).

          Show
          poornachandra Poorna Chandra added a comment - In that case we will have to support it. Should we mark the newly introduced methods in Tephra and Phoenix as deprecated as well? This is so that the users know that these methods are not supported, and don't use them. Even if we use reflection in this case, once we introduce a new method users can use the same method in other non-supported HBase versions too (if we don't introduce a new compat module for this change in the case of Tephra for instance).
          Hide
          jamestaylor James Taylor added a comment -

          The methods were removed in 1.1.6 and 1.2.3, so they can't be called in newer versions. Maybe we just say "Don't use 1.1.5 and 1.2.2 versions" and leave it at that?

          Show
          jamestaylor James Taylor added a comment - The methods were removed in 1.1.6 and 1.2.3, so they can't be called in newer versions. Maybe we just say "Don't use 1.1.5 and 1.2.2 versions" and leave it at that?
          Hide
          poornachandra Poorna Chandra added a comment -

          If we add reflection for these methods in Tephra compat module for HBase 1.2 instead of adding a new compat module for HBase 1.2.2, then these new methods will be visible for other patch versions of HBase 1.2 too. I think adding two separate compat modules, for HBase 1.1.5 and 1.2.2, for just four methods is an overkill.

          Alternatively, as you suggested we could just document that we don't support these two versions. If later there is demand for these versions, then we can revisit this decision.

          Show
          poornachandra Poorna Chandra added a comment - If we add reflection for these methods in Tephra compat module for HBase 1.2 instead of adding a new compat module for HBase 1.2.2, then these new methods will be visible for other patch versions of HBase 1.2 too. I think adding two separate compat modules, for HBase 1.1.5 and 1.2.2, for just four methods is an overkill. Alternatively, as you suggested we could just document that we don't support these two versions. If later there is demand for these versions, then we can revisit this decision.
          Hide
          jamestaylor James Taylor added a comment -

          Agreed. We just won't support these HBase versions.

          Show
          jamestaylor James Taylor added a comment - Agreed. We just won't support these HBase versions.
          Hide
          apurtell Andrew Purtell added a comment -

          Would it help if we add errata to our downloads page indicating 1.1.5 and 1.1.2 should not be used?

          Show
          apurtell Andrew Purtell added a comment - Would it help if we add errata to our downloads page indicating 1.1.5 and 1.1.2 should not be used?
          Hide
          poornachandra Poorna Chandra added a comment -

          Andrew Purtell Yes, having an errata will help a lot. Thanks for the offer.

          What do you think James Taylor?

          Show
          poornachandra Poorna Chandra added a comment - Andrew Purtell Yes, having an errata will help a lot. Thanks for the offer. What do you think James Taylor ?
          Hide
          jamestaylor James Taylor added a comment -

          Agreed. The unsupported versions can be added to this[1] page which lives in the site/markdown/download.md file and can be updated as described here[2].

          [1] https://phoenix.apache.org/download.html
          [2] https://phoenix.apache.org/building_website.html

          Show
          jamestaylor James Taylor added a comment - Agreed. The unsupported versions can be added to this [1] page which lives in the site/markdown/download.md file and can be updated as described here [2] . [1] https://phoenix.apache.org/download.html [2] https://phoenix.apache.org/building_website.html

            People

            • Assignee:
              jamestaylor James Taylor
              Reporter:
              rleidle Rob Leidle
            • Votes:
              0 Vote for this issue
              Watchers:
              7 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Time Tracking

                Estimated:
                Original Estimate - 24h
                24h
                Remaining:
                Remaining Estimate - 24h
                24h
                Logged:
                Time Spent - Not Specified
                Not Specified

                  Development