[ 
https://issues.apache.org/jira/browse/DRILL-5485?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16025369#comment-16025369
 ] 

ASF GitHub Bot commented on DRILL-5485:
---------------------------------------

Github user sudheeshkatkam commented on a diff in the pull request:

    https://github.com/apache/drill/pull/829#discussion_r118580820
  
    --- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/rpc/AbstractUserClientConnectionWrapper.java
 ---
    @@ -0,0 +1,101 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *    http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +package org.apache.drill.exec.rpc;
    +
    +import com.google.common.base.Preconditions;
    +import org.apache.drill.common.exceptions.UserException;
    +import org.apache.drill.common.exceptions.UserRemoteException;
    +import org.apache.drill.exec.proto.GeneralRPCProtos;
    +import org.apache.drill.exec.proto.UserBitShared.DrillPBError;
    +import org.apache.drill.exec.proto.UserBitShared.QueryId;
    +import org.apache.drill.exec.proto.UserBitShared.QueryResult;
    +import org.apache.drill.exec.proto.helper.QueryIdHelper;
    +
    +import java.util.concurrent.CountDownLatch;
    +import java.util.concurrent.TimeUnit;
    +
    +public abstract class AbstractUserClientConnectionWrapper implements 
UserClientConnection {
    +  private static final org.slf4j.Logger logger =
    +      
org.slf4j.LoggerFactory.getLogger(AbstractUserClientConnectionWrapper.class);
    +
    +  protected final CountDownLatch latch = new CountDownLatch(1);
    +
    +  protected volatile DrillPBError error;
    +
    +  protected volatile UserException exception;
    +
    +  /**
    +   * Wait until the query has completed or timeout is passed.
    +   *
    +   * @throws InterruptedException
    +   */
    +  public boolean await(final long timeoutMillis) throws 
InterruptedException {
    +    return latch.await(timeoutMillis, TimeUnit.MILLISECONDS);
    +  }
    +
    +  /**
    +   * Wait indefinitely until the query is completed. Used only in case of 
WebUser
    +   *
    +   * @throws Exception
    +   */
    +  public void await() throws Exception {
    +    latch.await();
    +    if (exception != null) {
    +      throw exception;
    +    }
    +  }
    +
    +  @Override
    +  public void sendResult(RpcOutcomeListener<GeneralRPCProtos.Ack> 
listener, QueryResult result) {
    +
    +    Preconditions.checkState(result.hasQueryState());
    +
    +    // Release the wait latch if the query is terminated.
    +    final QueryResult.QueryState state = result.getQueryState();
    +    final QueryId queryId = result.getQueryId();
    +
    +    if (logger.isDebugEnabled()) {
    +      logger.debug("Result arrived for QueryId: {} with QueryState: {}", 
QueryIdHelper.getQueryId(queryId), state);
    +    }
    +
    +    switch (state) {
    +      case FAILED:
    +        error = result.getError(0);
    +        exception = new UserRemoteException(error);
    +        latch.countDown();
    +        break;
    +      case CANCELED:
    +      case COMPLETED:
    +        Preconditions.checkState(result.getErrorCount() == 0);
    +        latch.countDown();
    +        break;
    +      default:
    +        logger.error("Query with QueryId: {} is in unexpected state: {}", 
queryId, state);
    --- End diff --
    
    That maybe an issue as well.
    
    AFAIK 
[DRILL-2498](https://github.com/apache/drill/commit/1d9d82b001810605e3f94ab3a5517dc0ed739715#diff-158c887d198393117d3a1bbc42114a8b)
 ensures that only the final state is sent to client using `sendResult`; this 
is the terminal message from server to client for that query. So if that 
message is wrong, the query is in an illegal state.


> Remove WebServer dependency on DrillClient
> ------------------------------------------
>
>                 Key: DRILL-5485
>                 URL: https://issues.apache.org/jira/browse/DRILL-5485
>             Project: Apache Drill
>          Issue Type: Improvement
>          Components: Web Server
>            Reporter: Sorabh Hamirwasia
>             Fix For: 1.11.0
>
>
> With encryption support using SASL, client's won't be able to authenticate 
> using PLAIN mechanism when encryption is enabled on the cluster. Today 
> WebServer which is embedded inside Drillbit creates a DrillClient instance 
> for each WebClient session. And the WebUser is authenticated as part of 
> authentication between DrillClient instance and Drillbit using PLAIN 
> mechanism. But with encryption enabled this will fail since encryption 
> doesn't support authentication using PLAN mechanism, hence no WebClient can 
> connect to a Drillbit. There are below issues as well with this approach:
> 1) Since DrillClient is used per WebUser session this is expensive as it has 
> heavyweight RPC layer for DrillClient and all it's dependencies. 
> 2) If the Foreman for a WebUser is also selected to be a different node then 
> there will be extra hop of transferring data back to WebClient.
> To resolve all the above issue it would be better to authenticate the WebUser 
> locally using the Drillbit on which WebServer is running without creating 
> DrillClient instance. We can use the local PAMAuthenticator to authenticate 
> the user. After authentication is successful the local Drillbit can also 
> serve as the Foreman for all the queries submitted by WebUser. This can be 
> achieved by submitting the query to the local Drillbit Foreman work queue. 
> This will also remove the requirement to encrypt the channel opened between 
> WebServer (DrillClient) and selected Drillbit since with this approach there 
> won't be any physical channel opened between them.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

Reply via email to