Hive
  1. Hive
  2. HIVE-2746

Metastore client doesn't log properly in case of connection failure to server

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 0.8.0
    • Fix Version/s: 0.9.0
    • Component/s: Metastore
    • Labels:
      None

      Description

      LOG.error(e.getStackTrace()) in current code prints memory location of StackTraceElement[] instead of message.

      1. ASF.LICENSE.NOT.GRANTED--HIVE-2746.D1395.1.patch
        2 kB
        Phabricator
      2. ASF.LICENSE.NOT.GRANTED--HIVE-2746.D1395.2.patch
        8 kB
        Phabricator
      3. hive-2746.patch
        8 kB
        Ashutosh Chauhan

        Issue Links

          Activity

          Hide
          Phabricator added a comment -

          ashutoshc requested code review of "HIVE-2746 [jira] Metastore client doesn't log properly in case of connection failure to server".
          Reviewers: JIRA

          https://issues.apache.org/jira/browse/HIVE-2746

          Corrected log message to print exception message instead of reference address.

          LOG.error(e.getStackTrace()) in current code prints memory location of StackTraceElement[] instead of message.

          TEST PLAN
          EMPTY

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

          AFFECTED FILES
          metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java

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

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

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

          Show
          Phabricator added a comment - ashutoshc requested code review of " HIVE-2746 [jira] Metastore client doesn't log properly in case of connection failure to server". Reviewers: JIRA https://issues.apache.org/jira/browse/HIVE-2746 Corrected log message to print exception message instead of reference address. LOG.error(e.getStackTrace()) in current code prints memory location of StackTraceElement[] instead of message. TEST PLAN EMPTY REVISION DETAIL https://reviews.facebook.net/D1395 AFFECTED FILES metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java MANAGE HERALD DIFFERENTIAL RULES https://reviews.facebook.net/herald/view/differential/ WHY DID I GET THIS EMAIL? https://reviews.facebook.net/herald/transcript/2913/ Tip: use the X-Herald-Rules header to filter Herald messages in your client.
          Hide
          Phabricator added a comment -

          cwsteinbach has requested changes to the revision "HIVE-2746 [jira] Metastore client doesn't log properly in case of connection failure to server".

          INLINE COMMENTS
          metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java:292 Is there any point in trying to set the ugi if the connection failed? Maybe this should be raised directly from the catch block above?

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

          Show
          Phabricator added a comment - cwsteinbach has requested changes to the revision " HIVE-2746 [jira] Metastore client doesn't log properly in case of connection failure to server". INLINE COMMENTS metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java:292 Is there any point in trying to set the ugi if the connection failed? Maybe this should be raised directly from the catch block above? REVISION DETAIL https://reviews.facebook.net/D1395
          Hide
          Phabricator added a comment -

          ashutoshc has commented on the revision "HIVE-2746 [jira] Metastore client doesn't log properly in case of connection failure to server".

          INLINE COMMENTS
          metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java:292 No, in the catch block above exception is only logged, not thrown, because thats within a for loop of retries. Connection may succeed in a retry. We can't throw exception there.

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

          Show
          Phabricator added a comment - ashutoshc has commented on the revision " HIVE-2746 [jira] Metastore client doesn't log properly in case of connection failure to server". INLINE COMMENTS metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java:292 No, in the catch block above exception is only logged, not thrown, because thats within a for loop of retries. Connection may succeed in a retry. We can't throw exception there. REVISION DETAIL https://reviews.facebook.net/D1395
          Hide
          Phabricator added a comment -

          cwsteinbach has commented on the revision "HIVE-2746 [jira] Metastore client doesn't log properly in case of connection failure to server".

          INLINE COMMENTS
          metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java:292 Ok, but then shouldn't this block be moved above the preceding one where we try to set the ugi? If we're not connected won't the set_ugi operation fail anyway with a LoginException?

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

          Show
          Phabricator added a comment - cwsteinbach has commented on the revision " HIVE-2746 [jira] Metastore client doesn't log properly in case of connection failure to server". INLINE COMMENTS metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java:292 Ok, but then shouldn't this block be moved above the preceding one where we try to set the ugi? If we're not connected won't the set_ugi operation fail anyway with a LoginException? REVISION DETAIL https://reviews.facebook.net/D1395
          Hide
          Phabricator added a comment -

          cwsteinbach has commented on the revision "HIVE-2746 [jira] Metastore client doesn't log properly in case of connection failure to server".

          INLINE COMMENTS
          metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java:292 Ok, I think I'm getting confused by the inconsistent indentation in this method (can you fix that?). It looks to me like the block of code that attempts to set the ugi should be taken out of the for-loop and moved below the !isConnected check. Does that make sense? Otherwise we're just polluting the log with noise every time a connection attempt fails.

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

          Show
          Phabricator added a comment - cwsteinbach has commented on the revision " HIVE-2746 [jira] Metastore client doesn't log properly in case of connection failure to server". INLINE COMMENTS metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java:292 Ok, I think I'm getting confused by the inconsistent indentation in this method (can you fix that?). It looks to me like the block of code that attempts to set the ugi should be taken out of the for-loop and moved below the !isConnected check. Does that make sense? Otherwise we're just polluting the log with noise every time a connection attempt fails. REVISION DETAIL https://reviews.facebook.net/D1395
          Hide
          Phabricator added a comment -

          ashutoshc updated the revision "HIVE-2746 [jira] Metastore client doesn't log properly in case of connection failure to server".
          Reviewers: JIRA, cwsteinbach

          Incorporated Carl's suggestions. Fixed indentation. Moved both setugi functionality as well as secure transport setup outside of for loop, since retrials are for transport.open() and that code doesn't belong in for loop.

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

          AFFECTED FILES
          metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java

          Show
          Phabricator added a comment - ashutoshc updated the revision " HIVE-2746 [jira] Metastore client doesn't log properly in case of connection failure to server". Reviewers: JIRA, cwsteinbach Incorporated Carl's suggestions. Fixed indentation. Moved both setugi functionality as well as secure transport setup outside of for loop, since retrials are for transport.open() and that code doesn't belong in for loop. REVISION DETAIL https://reviews.facebook.net/D1395 AFFECTED FILES metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java
          Hide
          Ashutosh Chauhan added a comment -

          All the tests passed with latest patch except for insert2_overwrite_partitions.q which seems unrelated to this patch and randomly fails on trunk.

          Show
          Ashutosh Chauhan added a comment - All the tests passed with latest patch except for insert2_overwrite_partitions.q which seems unrelated to this patch and randomly fails on trunk.
          Hide
          Phabricator added a comment -

          cwsteinbach has accepted the revision "HIVE-2746 [jira] Metastore client doesn't log properly in case of connection failure to server".

          +1

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

          Show
          Phabricator added a comment - cwsteinbach has accepted the revision " HIVE-2746 [jira] Metastore client doesn't log properly in case of connection failure to server". +1 REVISION DETAIL https://reviews.facebook.net/D1395
          Hide
          Ashutosh Chauhan added a comment -

          Patch with appropriate appropriate grant perms. Going to commit soon.

          Show
          Ashutosh Chauhan added a comment - Patch with appropriate appropriate grant perms. Going to commit soon.
          Hide
          Ashutosh Chauhan added a comment -

          Committed to trunk.

          Show
          Ashutosh Chauhan added a comment - Committed to trunk.
          Hide
          Hudson added a comment -

          Integrated in Hive-trunk-h0.21 #1221 (See https://builds.apache.org/job/Hive-trunk-h0.21/1221/)
          HIVE-2746 : Metastore client doesn't log properly in case of connection failure to server (hashutosh)

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

          • /hive/trunk/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java
          Show
          Hudson added a comment - Integrated in Hive-trunk-h0.21 #1221 (See https://builds.apache.org/job/Hive-trunk-h0.21/1221/ ) HIVE-2746 : Metastore client doesn't log properly in case of connection failure to server (hashutosh) hashutosh : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1236025 Files : /hive/trunk/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.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-hadoop2 #54 (See https://builds.apache.org/job/Hive-trunk-hadoop2/54/)
          HIVE-2746 : Metastore client doesn't log properly in case of connection failure to server (hashutosh) (Revision 1236025)

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

          • /hive/trunk/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java
          Show
          Hudson added a comment - Integrated in Hive-trunk-hadoop2 #54 (See https://builds.apache.org/job/Hive-trunk-hadoop2/54/ ) HIVE-2746 : Metastore client doesn't log properly in case of connection failure to server (hashutosh) (Revision 1236025) Result = ABORTED hashutosh : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1236025 Files : /hive/trunk/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java

            People

            • Assignee:
              Ashutosh Chauhan
              Reporter:
              Ashutosh Chauhan
            • Votes:
              0 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development