HBase
  1. HBase
  2. HBASE-10088

SecureClient will hang when access secure-disabled cluster

    Details

    • Type: Improvement Improvement
    • Status: Patch Available
    • Priority: Major Major
    • Resolution: Unresolved
    • Affects Version/s: 0.94.14
    • Fix Version/s: None
    • Component/s: Client, security
    • Labels:
      None

      Description

      When I misuse a secure hbase client to access a secure-disabled hbase server, I found the client will hang. The reason is that client will firstly invoke rpc method "getProtocolVersion", and the response from a secure-disabled server won't contain necessary fields processed by SecureClient. SecureClient will process the response as follows : (from SecureClient.receiveResponse()):

      if (state == Status.SUCCESS.state) {
                Writable value = ReflectionUtils.newInstance(valueClass, conf);
                value.readFields(in);                 // read value
                if (LOG.isDebugEnabled()) {
                  LOG.debug("call #"+id+", response is:\n"+value.toString());
                }
                // it's possible that this call may have been cleaned up due to a RPC
                // timeout, so check if it still exists before setting the value.
                if (call != null) {
                  call.setValue(value);
                }
              } else if (state == Status.ERROR.state) {
                if (call != null) {
                  call.setException(new RemoteException(WritableUtils.readString(in), WritableUtils
                      .readString(in)));
                }
              } else if (state == Status.FATAL.state) {
                RemoteException exception = new RemoteException(WritableUtils.readString(in),
                    WritableUtils.readString(in));
                // the call will be removed from call map, we must set Exception here to notify
                // the thread waited on the call
                if (call != null) {
                  call.setException(exception);
                }
                // Close the connection
                markClosed(exception);
              }
              calls.remove(id);
      

      As the above code, SecureClient need to read 'state' field from response. If the response is from a secure-disabled server, there will no 'state' field in response and SecureClient will get an illegal 'state', then the call will be removed from cached calls without notifying waiting thread. This will make the invoker waiting all the time. Although we should not use secure client to access secure-disabled server, users might encounter this situation because of misusing or error configuration. If the client will hang in this situation, users might not know the error quickly. Maybe, it is better to report an error in this situation so that users will know what happens quickly.

      1. HBASE-10088-0.94-v2.patch
        0.8 kB
        Andrew Purtell
      2. HBASE-10088-0.94-v2.patch
        0.8 kB
        cuijianwei
      3. HBASE-10088-0.94-v1.patch
        0.9 kB
        cuijianwei

        Activity

        Hide
        cuijianwei added a comment -

        The security implementation of trunk branch changes a lot from 0.94. I go through the trunk code, it seems the trunk code won't suffer from this bug?

        Show
        cuijianwei added a comment - The security implementation of trunk branch changes a lot from 0.94. I go through the trunk code, it seems the trunk code won't suffer from this bug?
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12617318/HBASE-10088-0.94-v2.patch
        against trunk revision .
        ATTACHMENT ID: 12617318

        +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 patch. The patch command could not apply the patch.

        Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/8834//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/12617318/HBASE-10088-0.94-v2.patch against trunk revision . ATTACHMENT ID: 12617318 +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 patch . The patch command could not apply the patch. Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/8834//console This message is automatically generated.
        Hide
        Liang Xie added a comment -

        Oh, mind uploading a trunk patch if possible, jianwei ?

        Show
        Liang Xie added a comment - Oh, mind uploading a trunk patch if possible, jianwei ?
        Hide
        Liang Xie added a comment -

        Kick off QA robot

        Show
        Liang Xie added a comment - Kick off QA robot
        Hide
        Andrew Purtell added a comment -

        Attached an update for the v2 patch with the spelling fix.

        Show
        Andrew Purtell added a comment - Attached an update for the v2 patch with the spelling fix.
        Hide
        Andrew Purtell added a comment -

        +1 on v2 modulo a minor spelling nit we can fix on commit. Lars Hofhansl what do you think?

        Show
        Andrew Purtell added a comment - +1 on v2 modulo a minor spelling nit we can fix on commit. Lars Hofhansl what do you think?
        Hide
        cuijianwei added a comment -

        Andrew Purtell Thanks for your comment. I agree that using IOException is enough to show the error to the user. I polish the patch using IOException.

        Show
        cuijianwei added a comment - Andrew Purtell Thanks for your comment. I agree that using IOException is enough to show the error to the user. I polish the patch using IOException.
        Hide
        Andrew Purtell added a comment - - edited

        Patch looks ok cuijianwei except let's not invent new exception names, there is no exception in the Hadoop or HBase hierarchy named UnknownServerResponseStateException, and "unknow" should be "unknown".

        Edit: How about just setException(new IOException(...))

        Show
        Andrew Purtell added a comment - - edited Patch looks ok cuijianwei except let's not invent new exception names, there is no exception in the Hadoop or HBase hierarchy named UnknownServerResponseStateException, and "unknow" should be "unknown". Edit: How about just setException(new IOException(...))
        Hide
        cuijianwei added a comment -

        The path will make SecureClient notify waiting threads with an RemoteException when receiving illegal 'state', then user will know the error quickly.

        Show
        cuijianwei added a comment - The path will make SecureClient notify waiting threads with an RemoteException when receiving illegal 'state', then user will know the error quickly.

          People

          • Assignee:
            cuijianwei
            Reporter:
            cuijianwei
          • Votes:
            0 Vote for this issue
            Watchers:
            6 Start watching this issue

            Dates

            • Created:
              Updated:

              Development