Details

    • Type: Sub-task Sub-task
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 0.9.0
    • Fix Version/s: 0.9.0
    • Component/s: Metastore
    • Labels:
      None
    • Hadoop Flags:
      Reviewed

      Description

      In HIVE-1219, method retrying for raw store operation are introduced to handle jdo operations more robustly. However, the abstraction for the RawStore operations can be moved to a separate class implementing RawStore, which should clean up the code base for HiveMetaStore.

      1. ASF.LICENSE.NOT.GRANTED--HIVE-2716.D1227.1.patch
        111 kB
        Phabricator
      2. ASF.LICENSE.NOT.GRANTED--HIVE-2716.D1227.2.patch
        111 kB
        Phabricator
      3. ASF.LICENSE.NOT.GRANTED--HIVE-2716.D1227.3.patch
        108 kB
        Phabricator
      4. ASF.LICENSE.NOT.GRANTED--HIVE-2716.D1227.4.patch
        106 kB
        Phabricator
      5. ASF.LICENSE.NOT.GRANTED--HIVE-2716.D2055.1.patch
        23 kB
        Phabricator
      6. HIVE-2716.patch
        111 kB
        Enis Soztutar

        Issue Links

          Activity

          Hide
          Hudson added a comment -

          Integrated in Hive-trunk-hadoop2 #54 (See https://builds.apache.org/job/Hive-trunk-hadoop2/54/)
          HIVE-2833 Fix test failures caused by HIVE-2716
          (Kevin Wilfong via namit) (Revision 1296946)
          HIVE-2716 : Move retry logic in HiveMetaStore to a separe class (enis via hashutosh) (Revision 1292420)

          Result = ABORTED
          namit : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1296946
          Files :

          • /hive/trunk/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
          • /hive/trunk/metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreUtils.java
          • /hive/trunk/metastore/src/java/org/apache/hadoop/hive/metastore/RetryingRawStore.java
          • /hive/trunk/metastore/src/test/org/apache/hadoop/hive/metastore/DummyJdoConnectionUrlHook.java
          • /hive/trunk/metastore/src/test/org/apache/hadoop/hive/metastore/DummyRawStoreForJdoConnection.java
          • /hive/trunk/metastore/src/test/org/apache/hadoop/hive/metastore/TestMetaStoreConnectionUrlHook.java

          hashutosh : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1292420
          Files :

          • /hive/trunk/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
          • /hive/trunk/metastore/src/java/org/apache/hadoop/hive/metastore/RetryingRawStore.java
          • /hive/trunk/metastore/src/java/org/apache/hadoop/hive/metastore/events/EventCleanerTask.java
          Show
          Hudson added a comment - Integrated in Hive-trunk-hadoop2 #54 (See https://builds.apache.org/job/Hive-trunk-hadoop2/54/ ) HIVE-2833 Fix test failures caused by HIVE-2716 (Kevin Wilfong via namit) (Revision 1296946) HIVE-2716 : Move retry logic in HiveMetaStore to a separe class (enis via hashutosh) (Revision 1292420) Result = ABORTED namit : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1296946 Files : /hive/trunk/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java /hive/trunk/metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreUtils.java /hive/trunk/metastore/src/java/org/apache/hadoop/hive/metastore/RetryingRawStore.java /hive/trunk/metastore/src/test/org/apache/hadoop/hive/metastore/DummyJdoConnectionUrlHook.java /hive/trunk/metastore/src/test/org/apache/hadoop/hive/metastore/DummyRawStoreForJdoConnection.java /hive/trunk/metastore/src/test/org/apache/hadoop/hive/metastore/TestMetaStoreConnectionUrlHook.java hashutosh : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1292420 Files : /hive/trunk/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java /hive/trunk/metastore/src/java/org/apache/hadoop/hive/metastore/RetryingRawStore.java /hive/trunk/metastore/src/java/org/apache/hadoop/hive/metastore/events/EventCleanerTask.java
          Hide
          Ashutosh Chauhan added a comment -

          This issue is closed now. It was released with the fix in 0.9.0. If there is a problem, please open a new jira and link this one with that.

          Show
          Ashutosh Chauhan added a comment - This issue is closed now. It was released with the fix in 0.9.0. If there is a problem, please open a new jira and link this one with that.
          Hide
          Hudson added a comment -

          Integrated in Hive-trunk-h0.21 #1290 (See https://builds.apache.org/job/Hive-trunk-h0.21/1290/)
          HIVE-2833 Fix test failures caused by HIVE-2716
          (Kevin Wilfong via namit) (Revision 1296946)

          Result = SUCCESS
          namit : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1296946
          Files :

          • /hive/trunk/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
          • /hive/trunk/metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreUtils.java
          • /hive/trunk/metastore/src/java/org/apache/hadoop/hive/metastore/RetryingRawStore.java
          • /hive/trunk/metastore/src/test/org/apache/hadoop/hive/metastore/DummyJdoConnectionUrlHook.java
          • /hive/trunk/metastore/src/test/org/apache/hadoop/hive/metastore/DummyRawStoreForJdoConnection.java
          • /hive/trunk/metastore/src/test/org/apache/hadoop/hive/metastore/TestMetaStoreConnectionUrlHook.java
          Show
          Hudson added a comment - Integrated in Hive-trunk-h0.21 #1290 (See https://builds.apache.org/job/Hive-trunk-h0.21/1290/ ) HIVE-2833 Fix test failures caused by HIVE-2716 (Kevin Wilfong via namit) (Revision 1296946) Result = SUCCESS namit : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1296946 Files : /hive/trunk/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java /hive/trunk/metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreUtils.java /hive/trunk/metastore/src/java/org/apache/hadoop/hive/metastore/RetryingRawStore.java /hive/trunk/metastore/src/test/org/apache/hadoop/hive/metastore/DummyJdoConnectionUrlHook.java /hive/trunk/metastore/src/test/org/apache/hadoop/hive/metastore/DummyRawStoreForJdoConnection.java /hive/trunk/metastore/src/test/org/apache/hadoop/hive/metastore/TestMetaStoreConnectionUrlHook.java
          Hide
          Kevin Wilfong added a comment -

          Ignore the Phabricator comment and patch for D2055, that was intended for another JIRA.

          Show
          Kevin Wilfong added a comment - Ignore the Phabricator comment and patch for D2055, that was intended for another JIRA.
          Hide
          Phabricator added a comment -

          kevinwilfong requested code review of "HIVE-2716 [jira] Move retry logic in HiveMetaStore to a separe class".
          Reviewers: JIRA

          This fixes the issue where the implementation of RawStore was being initialized in the metastore before the JDOConnectionURLHook was being called. I moved some code around so that the RawStore implementation is initialized in the RetryRawStore initialization after the hook is called. In addition, the URL now needs to be updated in the thread local configuration instead of the non-thread local one, as by the time the hook is now being called, the thread local configuration has already been initialized based on the other.

          In HIVE-1219, method retrying for raw store operation are introduced to handle jdo operations more robustly. However, the abstraction for the RawStore operations can be moved to a separate class implementing RawStore, which should clean up the code base for HiveMetaStore.

          TEST PLAN
          EMPTY

          REVISION DETAIL
          https://reviews.facebook.net/D2055

          AFFECTED FILES
          metastore/src/test/org/apache/hadoop/hive/metastore/DummyRawStoreForJdoConnection.java
          metastore/src/test/org/apache/hadoop/hive/metastore/TestMetaStoreConnectionUrlHook.java
          metastore/src/test/org/apache/hadoop/hive/metastore/DummyJdoConnectionUrlHook.java
          metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
          metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreUtils.java
          metastore/src/java/org/apache/hadoop/hive/metastore/RetryingRawStore.java

          MANAGE HERALD DIFFERENTIAL RULES
          https://reviews.facebook.net/herald/view/differential/

          WHY DID I GET THIS EMAIL?
          https://reviews.facebook.net/herald/transcript/4407/

          Tip: use the X-Herald-Rules header to filter Herald messages in your client.

          Show
          Phabricator added a comment - kevinwilfong requested code review of " HIVE-2716 [jira] Move retry logic in HiveMetaStore to a separe class". Reviewers: JIRA This fixes the issue where the implementation of RawStore was being initialized in the metastore before the JDOConnectionURLHook was being called. I moved some code around so that the RawStore implementation is initialized in the RetryRawStore initialization after the hook is called. In addition, the URL now needs to be updated in the thread local configuration instead of the non-thread local one, as by the time the hook is now being called, the thread local configuration has already been initialized based on the other. In HIVE-1219 , method retrying for raw store operation are introduced to handle jdo operations more robustly. However, the abstraction for the RawStore operations can be moved to a separate class implementing RawStore, which should clean up the code base for HiveMetaStore. TEST PLAN EMPTY REVISION DETAIL https://reviews.facebook.net/D2055 AFFECTED FILES metastore/src/test/org/apache/hadoop/hive/metastore/DummyRawStoreForJdoConnection.java metastore/src/test/org/apache/hadoop/hive/metastore/TestMetaStoreConnectionUrlHook.java metastore/src/test/org/apache/hadoop/hive/metastore/DummyJdoConnectionUrlHook.java metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreUtils.java metastore/src/java/org/apache/hadoop/hive/metastore/RetryingRawStore.java MANAGE HERALD DIFFERENTIAL RULES https://reviews.facebook.net/herald/view/differential/ WHY DID I GET THIS EMAIL? https://reviews.facebook.net/herald/transcript/4407/ Tip: use the X-Herald-Rules header to filter Herald messages in your client.
          Hide
          Carl Steinbach added a comment -

          I created HIVE-2833 to cover the task of fixing the test failures caused by this patch.

          Show
          Carl Steinbach added a comment - I created HIVE-2833 to cover the task of fixing the test failures caused by this patch.
          Hide
          Namit Jain added a comment -

          @Ashutosh, it broke facebook's internal branch.
          Did you test it with mysql db - I haven't debugged it, but reverting this fixes the problem.

          Show
          Namit Jain added a comment - @Ashutosh, it broke facebook's internal branch. Did you test it with mysql db - I haven't debugged it, but reverting this fixes the problem.
          Hide
          Ashutosh Chauhan added a comment -

          I reran the ant test -Dtestcase=TestNegativeCliDriver -Dqfile=script_broken_pipe1.q which failed as reported and it passed. So, looks like that failure is not related to this patch.

          Show
          Ashutosh Chauhan added a comment - I reran the ant test -Dtestcase=TestNegativeCliDriver -Dqfile=script_broken_pipe1.q which failed as reported and it passed. So, looks like that failure is not related to this patch.
          Hide
          Hudson added a comment -

          Integrated in Hive-trunk-h0.21 #1270 (See https://builds.apache.org/job/Hive-trunk-h0.21/1270/)
          HIVE-2716 : Move retry logic in HiveMetaStore to a separe class (enis via hashutosh) (Revision 1292420)

          Result = FAILURE
          hashutosh : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1292420
          Files :

          • /hive/trunk/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
          • /hive/trunk/metastore/src/java/org/apache/hadoop/hive/metastore/RetryingRawStore.java
          • /hive/trunk/metastore/src/java/org/apache/hadoop/hive/metastore/events/EventCleanerTask.java
          Show
          Hudson added a comment - Integrated in Hive-trunk-h0.21 #1270 (See https://builds.apache.org/job/Hive-trunk-h0.21/1270/ ) HIVE-2716 : Move retry logic in HiveMetaStore to a separe class (enis via hashutosh) (Revision 1292420) Result = FAILURE hashutosh : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1292420 Files : /hive/trunk/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java /hive/trunk/metastore/src/java/org/apache/hadoop/hive/metastore/RetryingRawStore.java /hive/trunk/metastore/src/java/org/apache/hadoop/hive/metastore/events/EventCleanerTask.java
          Hide
          Enis Soztutar added a comment -

          Thanks Ashutosh. I'm uploading the final version for license.

          Show
          Enis Soztutar added a comment - Thanks Ashutosh. I'm uploading the final version for license.
          Hide
          Ashutosh Chauhan added a comment -

          All the tests passed. Patch checked-in!

          Show
          Ashutosh Chauhan added a comment - All the tests passed. Patch checked-in!
          Hide
          Phabricator added a comment -

          enis updated the revision "HIVE-2716 [jira] Move retry logic in HiveMetaStore to a separe class".
          Reviewers: JIRA, cwsteinbach, ashutoshc

          The test was failing due to the change in handling checked exceptions. Updated the patch to chang the auth-related methods to throw MetaExceptions, and convert to RuntimeExceptions to keep the current behavior unchanged.
          Running the tests now, will update with the results.

          REVISION DETAIL
          https://reviews.facebook.net/D1227

          AFFECTED FILES
          metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
          metastore/src/java/org/apache/hadoop/hive/metastore/RetryingRawStore.java
          metastore/src/java/org/apache/hadoop/hive/metastore/events/EventCleanerTask.java

          Show
          Phabricator added a comment - enis updated the revision " HIVE-2716 [jira] Move retry logic in HiveMetaStore to a separe class". Reviewers: JIRA, cwsteinbach, ashutoshc The test was failing due to the change in handling checked exceptions. Updated the patch to chang the auth-related methods to throw MetaExceptions, and convert to RuntimeExceptions to keep the current behavior unchanged. Running the tests now, will update with the results. REVISION DETAIL https://reviews.facebook.net/D1227 AFFECTED FILES metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java metastore/src/java/org/apache/hadoop/hive/metastore/RetryingRawStore.java metastore/src/java/org/apache/hadoop/hive/metastore/events/EventCleanerTask.java
          Hide
          Ashutosh Chauhan added a comment -

          ant test -Dtestcase=TestNegativeCliDriver -Dqfile=authorization_fail_1.q failed.

          Show
          Ashutosh Chauhan added a comment - ant test -Dtestcase=TestNegativeCliDriver -Dqfile=authorization_fail_1.q failed.
          Hide
          Phabricator added a comment -

          ashutoshc has accepted the revision "HIVE-2716 [jira] Move retry logic in HiveMetaStore to a separe class".

          +1 will commit if tests pass.

          REVISION DETAIL
          https://reviews.facebook.net/D1227

          BRANCH
          ms-retry-logic

          Show
          Phabricator added a comment - ashutoshc has accepted the revision " HIVE-2716 [jira] Move retry logic in HiveMetaStore to a separe class". +1 will commit if tests pass. REVISION DETAIL https://reviews.facebook.net/D1227 BRANCH ms-retry-logic
          Hide
          Phabricator added a comment -

          enis updated the revision "HIVE-2716 [jira] Move retry logic in HiveMetaStore to a separe class".
          Reviewers: JIRA, cwsteinbach

          Address review comments.

          • Make RetryingRawStore Private instead of LimitedPrivate
          • Reverted changes related to fixing drop_type inconsistency.

          REVISION DETAIL
          https://reviews.facebook.net/D1227

          AFFECTED FILES
          metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
          metastore/src/java/org/apache/hadoop/hive/metastore/RetryingRawStore.java
          metastore/src/java/org/apache/hadoop/hive/metastore/events/EventCleanerTask.java

          Show
          Phabricator added a comment - enis updated the revision " HIVE-2716 [jira] Move retry logic in HiveMetaStore to a separe class". Reviewers: JIRA, cwsteinbach Address review comments. Make RetryingRawStore Private instead of LimitedPrivate Reverted changes related to fixing drop_type inconsistency. REVISION DETAIL https://reviews.facebook.net/D1227 AFFECTED FILES metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java metastore/src/java/org/apache/hadoop/hive/metastore/RetryingRawStore.java metastore/src/java/org/apache/hadoop/hive/metastore/events/EventCleanerTask.java
          Hide
          Phabricator added a comment -

          cwsteinbach has requested changes to the revision "HIVE-2716 [jira] Move retry logic in HiveMetaStore to a separe class".

          INLINE COMMENTS
          metastore/src/test/org/apache/hadoop/hive/metastore/TestHiveMetaStore.java:136 Is this change required, or are you just providing coverage for the new dropType(String, boolean) method? Were exceptions somehow getting masked earlier?
          metastore/src/java/org/apache/hadoop/hive/metastore/RetryingRawStore.java:38 It's not clear to me why HCatalog needs access to this class. Can you please provide more details?

          REVISION DETAIL
          https://reviews.facebook.net/D1227

          BRANCH
          ms-retry-logic

          Show
          Phabricator added a comment - cwsteinbach has requested changes to the revision " HIVE-2716 [jira] Move retry logic in HiveMetaStore to a separe class". INLINE COMMENTS metastore/src/test/org/apache/hadoop/hive/metastore/TestHiveMetaStore.java:136 Is this change required, or are you just providing coverage for the new dropType(String, boolean) method? Were exceptions somehow getting masked earlier? metastore/src/java/org/apache/hadoop/hive/metastore/RetryingRawStore.java:38 It's not clear to me why HCatalog needs access to this class. Can you please provide more details? REVISION DETAIL https://reviews.facebook.net/D1227 BRANCH ms-retry-logic
          Hide
          Phabricator added a comment -

          enis has commented on the revision "HIVE-2716 [jira] Move retry logic in HiveMetaStore to a separe class".

          > What's the motivation for this patch? Is this change required by something on the HCat side?

          I created the issue and the patch while working on the HMS. The patch refactors the retry logic out of HMS. The Command wrapper inside HMS makes the code very hard to follow, and requires some ugly code for handling checked exceptions, and is error prone is someone forgets wrapping a new method in RawStore.

          REVISION DETAIL
          https://reviews.facebook.net/D1227

          Show
          Phabricator added a comment - enis has commented on the revision " HIVE-2716 [jira] Move retry logic in HiveMetaStore to a separe class". > What's the motivation for this patch? Is this change required by something on the HCat side? I created the issue and the patch while working on the HMS. The patch refactors the retry logic out of HMS. The Command wrapper inside HMS makes the code very hard to follow, and requires some ugly code for handling checked exceptions, and is error prone is someone forgets wrapping a new method in RawStore. REVISION DETAIL https://reviews.facebook.net/D1227
          Hide
          Phabricator added a comment -

          ashutoshc has commented on the revision "HIVE-2716 [jira] Move retry logic in HiveMetaStore to a separe class".

          Enis will know more, but AFAIK HCatalog won't get anything out of this change. I like the idea of the patch for refactoring HiveMetaStore.java which is unwieldy in its current form, thus making Hive codebase easier to read and follow. Do you have any specific concerns?

          REVISION DETAIL
          https://reviews.facebook.net/D1227

          Show
          Phabricator added a comment - ashutoshc has commented on the revision " HIVE-2716 [jira] Move retry logic in HiveMetaStore to a separe class". Enis will know more, but AFAIK HCatalog won't get anything out of this change. I like the idea of the patch for refactoring HiveMetaStore.java which is unwieldy in its current form, thus making Hive codebase easier to read and follow. Do you have any specific concerns? REVISION DETAIL https://reviews.facebook.net/D1227
          Hide
          Phabricator added a comment -

          cwsteinbach has commented on the revision "HIVE-2716 [jira] Move retry logic in HiveMetaStore to a separe class".

          What's the motivation for this patch? Is this change required by something on the HCat side?

          REVISION DETAIL
          https://reviews.facebook.net/D1227

          Show
          Phabricator added a comment - cwsteinbach has commented on the revision " HIVE-2716 [jira] Move retry logic in HiveMetaStore to a separe class". What's the motivation for this patch? Is this change required by something on the HCat side? REVISION DETAIL https://reviews.facebook.net/D1227
          Hide
          Phabricator added a comment -

          enis updated the revision "HIVE-2716 [jira] Move retry logic in HiveMetaStore to a separe class".
          Reviewers: JIRA

          Rebased the patch.

          REVISION DETAIL
          https://reviews.facebook.net/D1227

          AFFECTED FILES
          metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
          metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java
          metastore/src/java/org/apache/hadoop/hive/metastore/RetryingRawStore.java
          metastore/src/java/org/apache/hadoop/hive/metastore/events/EventCleanerTask.java
          metastore/src/test/org/apache/hadoop/hive/metastore/TestHiveMetaStore.java

          Show
          Phabricator added a comment - enis updated the revision " HIVE-2716 [jira] Move retry logic in HiveMetaStore to a separe class". Reviewers: JIRA Rebased the patch. REVISION DETAIL https://reviews.facebook.net/D1227 AFFECTED FILES metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java metastore/src/java/org/apache/hadoop/hive/metastore/RetryingRawStore.java metastore/src/java/org/apache/hadoop/hive/metastore/events/EventCleanerTask.java metastore/src/test/org/apache/hadoop/hive/metastore/TestHiveMetaStore.java
          Hide
          Ashutosh Chauhan added a comment -

          @Enis,
          Patch has gone stale. Can you rebase it ?

          Show
          Ashutosh Chauhan added a comment - @Enis, Patch has gone stale. Can you rebase it ?
          Hide
          Phabricator added a comment -

          enis has commented on the revision "HIVE-2716 [jira] Move retry logic in HiveMetaStore to a separe class".

          Some context for the patch:

          • All the retries in HiveMetaStore are actually retying methods of the RawStore interface, which handles the jdbc connection. Thus an abstraction in this layer makes more sense, and removes the clutter in the calling code.
          • The RawStore is proxied to introduce an interceptor to carry out the method invocations. Proxies of this type are used by most of the ORM frameworks for transaction management, or retrying, logging, etc. Performance-wise, there is a lot of layers already, like the thrift call -> jdo -> jdbc -> SQL parse -> db engine that it won't make a major difference to use a dynamic proxy. Besides they are not very slow as believed.
          • HiveMetaStore.Command and executeWithRetry() is limited-provate to HCat, and Hcat does not use these interfaces, so I just removed them. If we need them, we can instead deprecate.

          REVISION DETAIL
          https://reviews.facebook.net/D1227

          Show
          Phabricator added a comment - enis has commented on the revision " HIVE-2716 [jira] Move retry logic in HiveMetaStore to a separe class". Some context for the patch: All the retries in HiveMetaStore are actually retying methods of the RawStore interface, which handles the jdbc connection. Thus an abstraction in this layer makes more sense, and removes the clutter in the calling code. The RawStore is proxied to introduce an interceptor to carry out the method invocations. Proxies of this type are used by most of the ORM frameworks for transaction management, or retrying, logging, etc. Performance-wise, there is a lot of layers already, like the thrift call -> jdo -> jdbc -> SQL parse -> db engine that it won't make a major difference to use a dynamic proxy. Besides they are not very slow as believed. HiveMetaStore.Command and executeWithRetry() is limited-provate to HCat, and Hcat does not use these interfaces, so I just removed them. If we need them, we can instead deprecate. REVISION DETAIL https://reviews.facebook.net/D1227
          Hide
          Phabricator added a comment -

          enis requested code review of "HIVE-2716 [jira] Move retry logic in HiveMetaStore to a separe class".
          Reviewers: JIRA

          First version of the patch for refactoring retry logic to the RawStore interface

          In HIVE-1219, method retrying for raw store operation are introduced to handle jdo operations more robustly. However, the abstraction for the RawStore operations can be moved to a separate class implementing RawStore, which should clean up the code base for HiveMetaStore.

          TEST PLAN
          EMPTY

          REVISION DETAIL
          https://reviews.facebook.net/D1227

          AFFECTED FILES
          metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
          metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java
          metastore/src/java/org/apache/hadoop/hive/metastore/RetryingRawStore.java
          metastore/src/java/org/apache/hadoop/hive/metastore/events/EventCleanerTask.java
          metastore/src/test/org/apache/hadoop/hive/metastore/TestHiveMetaStore.java

          MANAGE HERALD DIFFERENTIAL RULES
          https://reviews.facebook.net/herald/view/differential/

          WHY DID I GET THIS EMAIL?
          https://reviews.facebook.net/herald/transcript/2523/

          Tip: use the X-Herald-Rules header to filter Herald messages in your client.

          Show
          Phabricator added a comment - enis requested code review of " HIVE-2716 [jira] Move retry logic in HiveMetaStore to a separe class". Reviewers: JIRA First version of the patch for refactoring retry logic to the RawStore interface In HIVE-1219 , method retrying for raw store operation are introduced to handle jdo operations more robustly. However, the abstraction for the RawStore operations can be moved to a separate class implementing RawStore, which should clean up the code base for HiveMetaStore. TEST PLAN EMPTY REVISION DETAIL https://reviews.facebook.net/D1227 AFFECTED FILES metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java metastore/src/java/org/apache/hadoop/hive/metastore/RetryingRawStore.java metastore/src/java/org/apache/hadoop/hive/metastore/events/EventCleanerTask.java metastore/src/test/org/apache/hadoop/hive/metastore/TestHiveMetaStore.java MANAGE HERALD DIFFERENTIAL RULES https://reviews.facebook.net/herald/view/differential/ WHY DID I GET THIS EMAIL? https://reviews.facebook.net/herald/transcript/2523/ Tip: use the X-Herald-Rules header to filter Herald messages in your client.

            People

            • Assignee:
              Enis Soztutar
              Reporter:
              Enis Soztutar
            • Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development