-
Notifications
You must be signed in to change notification settings - Fork 3.8k
Avatica protobuf #10543
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Avatica protobuf #10543
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -26,6 +26,10 @@ | |
| import com.google.common.collect.ImmutableMap; | ||
| import com.google.inject.Inject; | ||
| import com.google.inject.Provider; | ||
| import org.apache.calcite.avatica.remote.ProtobufTranslation; | ||
| import org.apache.calcite.avatica.remote.ProtobufTranslationImpl; | ||
| import org.apache.calcite.avatica.remote.Service; | ||
| import org.apache.commons.io.IOUtils; | ||
| import org.apache.druid.client.selector.Server; | ||
| import org.apache.druid.guice.annotations.Json; | ||
| import org.apache.druid.guice.annotations.Smile; | ||
|
|
@@ -118,6 +122,7 @@ private static void handleException(HttpServletResponse response, ObjectMapper o | |
| private final RequestLogger requestLogger; | ||
| private final GenericQueryMetricsFactory queryMetricsFactory; | ||
| private final AuthenticatorMapper authenticatorMapper; | ||
| private final ProtobufTranslation protobufTranslation; | ||
|
|
||
| private HttpClient broadcastClient; | ||
|
|
||
|
|
@@ -145,6 +150,7 @@ public AsyncQueryForwardingServlet( | |
| this.requestLogger = requestLogger; | ||
| this.queryMetricsFactory = queryMetricsFactory; | ||
| this.authenticatorMapper = authenticatorMapper; | ||
| this.protobufTranslation = new ProtobufTranslationImpl(); | ||
| } | ||
|
|
||
| @Override | ||
|
|
@@ -191,9 +197,16 @@ protected void service(HttpServletRequest request, HttpServletResponse response) | |
| // them as a generic request. | ||
| final boolean isQueryEndpoint = requestURI.startsWith("/druid/v2") && !requestURI.startsWith("/druid/v2/sql"); | ||
|
|
||
| final boolean isAvatica = requestURI.startsWith("/druid/v2/sql/avatica"); | ||
| final boolean isAvaticaJson = requestURI.startsWith("/druid/v2/sql/avatica"); | ||
| final boolean isAvaticaPb = requestURI.startsWith("/druid/v2/sql/avatica-protobuf"); | ||
|
|
||
| if (isAvatica) { | ||
| if (isAvaticaPb) { | ||
| byte[] requestBytes = IOUtils.toByteArray(request.getInputStream()); | ||
| Service.Request protobufRequest = this.protobufTranslation.parseRequest(requestBytes); | ||
| String connectionId = getAvaticaProtobufConnectionId(protobufRequest); | ||
| targetServer = hostFinder.findServerAvatica(connectionId); | ||
| request.setAttribute(AVATICA_QUERY_ATTRIBUTE, requestBytes); | ||
| } else if (isAvaticaJson) { | ||
| Map<String, Object> requestMap = objectMapper.readValue( | ||
| request.getInputStream(), | ||
| JacksonUtils.TYPE_REFERENCE_MAP_STRING_OBJECT | ||
|
|
@@ -456,6 +469,95 @@ static String getAvaticaConnectionId(Map<String, Object> requestMap) | |
| return (String) connectionIdObj; | ||
| } | ||
|
|
||
| static String getAvaticaProtobufConnectionId(Service.Request request) | ||
| { | ||
| if (request instanceof Service.CatalogsRequest) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agreed, it's a bit fragile, however, that, I'm afraid, is the nature of protobufs. I'm not sure how likely they are to add new request types without significant overhaul to Calcite though |
||
| return ((Service.CatalogsRequest) request).connectionId; | ||
| } | ||
|
|
||
| if (request instanceof Service.SchemasRequest) { | ||
| return ((Service.SchemasRequest) request).connectionId; | ||
| } | ||
|
|
||
| if (request instanceof Service.TablesRequest) { | ||
| return ((Service.TablesRequest) request).connectionId; | ||
| } | ||
|
|
||
| if (request instanceof Service.TypeInfoRequest) { | ||
| return ((Service.TypeInfoRequest) request).connectionId; | ||
| } | ||
|
|
||
| if (request instanceof Service.ColumnsRequest) { | ||
| return ((Service.ColumnsRequest) request).connectionId; | ||
| } | ||
|
|
||
| if (request instanceof Service.ExecuteRequest) { | ||
| return ((Service.ExecuteRequest) request).statementHandle.connectionId; | ||
| } | ||
|
|
||
| if (request instanceof Service.TableTypesRequest) { | ||
| return ((Service.TableTypesRequest) request).connectionId; | ||
| } | ||
|
|
||
| if (request instanceof Service.PrepareRequest) { | ||
| return ((Service.PrepareRequest) request).connectionId; | ||
| } | ||
|
|
||
| if (request instanceof Service.PrepareAndExecuteRequest) { | ||
| return ((Service.PrepareAndExecuteRequest) request).connectionId; | ||
| } | ||
|
|
||
| if (request instanceof Service.FetchRequest) { | ||
| return ((Service.FetchRequest) request).connectionId; | ||
| } | ||
|
|
||
| if (request instanceof Service.CreateStatementRequest) { | ||
| return ((Service.CreateStatementRequest) request).connectionId; | ||
| } | ||
|
|
||
| if (request instanceof Service.CloseStatementRequest) { | ||
| return ((Service.CloseStatementRequest) request).connectionId; | ||
| } | ||
|
|
||
| if (request instanceof Service.OpenConnectionRequest) { | ||
| return ((Service.OpenConnectionRequest) request).connectionId; | ||
| } | ||
|
|
||
| if (request instanceof Service.CloseConnectionRequest) { | ||
| return ((Service.CloseConnectionRequest) request).connectionId; | ||
| } | ||
|
|
||
| if (request instanceof Service.ConnectionSyncRequest) { | ||
| return ((Service.ConnectionSyncRequest) request).connectionId; | ||
| } | ||
|
|
||
| if (request instanceof Service.DatabasePropertyRequest) { | ||
| return ((Service.DatabasePropertyRequest) request).connectionId; | ||
| } | ||
|
|
||
| if (request instanceof Service.SyncResultsRequest) { | ||
| return ((Service.SyncResultsRequest) request).connectionId; | ||
| } | ||
|
|
||
| if (request instanceof Service.CommitRequest) { | ||
| return ((Service.CommitRequest) request).connectionId; | ||
| } | ||
|
|
||
| if (request instanceof Service.RollbackRequest) { | ||
| return ((Service.RollbackRequest) request).connectionId; | ||
| } | ||
|
|
||
| if (request instanceof Service.PrepareAndExecuteBatchRequest) { | ||
| return ((Service.PrepareAndExecuteBatchRequest) request).connectionId; | ||
| } | ||
|
|
||
| if (request instanceof Service.ExecuteBatchRequest) { | ||
| return ((Service.ExecuteBatchRequest) request).connectionId; | ||
| } | ||
|
|
||
| throw new IAE("Received an unknown Avatica protobuf request"); | ||
| } | ||
|
|
||
| private class MetricsEmittingProxyResponseListener<T> extends ProxyResponseListener | ||
| { | ||
| private final HttpServletRequest req; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,62 @@ | ||
| /* | ||
| * 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.druid.sql.avatica; | ||
|
|
||
| import com.google.inject.Inject; | ||
| import org.apache.calcite.avatica.remote.LocalService; | ||
| import org.apache.calcite.avatica.remote.Service; | ||
| import org.apache.calcite.avatica.server.AvaticaProtobufHandler; | ||
| import org.apache.druid.guice.annotations.Self; | ||
| import org.apache.druid.server.DruidNode; | ||
| import org.eclipse.jetty.server.Request; | ||
|
|
||
| import javax.servlet.ServletException; | ||
| import javax.servlet.http.HttpServletRequest; | ||
| import javax.servlet.http.HttpServletResponse; | ||
| import java.io.IOException; | ||
|
|
||
| public class DruidAvaticaProtobufHandler extends AvaticaProtobufHandler | ||
| { | ||
| public static final String AVATICA_PATH = "/druid/v2/sql/avatica-protobuf/"; | ||
|
|
||
| @Inject | ||
| public DruidAvaticaProtobufHandler( | ||
| final DruidMeta druidMeta, | ||
| @Self final DruidNode druidNode, | ||
| final AvaticaMonitor avaticaMonitor | ||
| ) | ||
| { | ||
| super(new LocalService(druidMeta), avaticaMonitor); | ||
| setServerRpcMetadata(new Service.RpcMetadataResponse(druidNode.getHostAndPortToUse())); | ||
| } | ||
|
|
||
| @Override | ||
| public void handle( | ||
| final String target, | ||
| final Request baseRequest, | ||
| final HttpServletRequest request, | ||
| final HttpServletResponse response | ||
| ) throws IOException, ServletException | ||
| { | ||
| if (request.getRequestURI().equals(AVATICA_PATH)) { | ||
| super.handle(target, baseRequest, request, response); | ||
| } | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -104,8 +104,10 @@ public void openConnection(final ConnectionHandle ch, final Map<String, String> | |
| { | ||
| // Build connection context. | ||
| final ImmutableMap.Builder<String, Object> context = ImmutableMap.builder(); | ||
| for (Map.Entry<String, String> entry : info.entrySet()) { | ||
| context.put(entry); | ||
| if (info != null) { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i wonder if this was causing some problem before... looking at other implementations some do check for null on this field, 👍 |
||
| for (Map.Entry<String, String> entry : info.entrySet()) { | ||
| context.put(entry); | ||
| } | ||
| } | ||
| openDruidConnection(ch.id, context.build()); | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
side note, since we now have avatica stuff as dependencies to this project, it probably makes sense in a follow-up to modify the JSON version to use the types like this is doing instead of deserializing to a map.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure how much there is to gain from that, what are you thinking would be the advantage? Also, looking through it I can't find an easy to use class like ProtobufTranslationImpl for json.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
BTW, I guess the reason would be defensive for the most part, to more strongly type the check so that whenever we upgrade the library, stuff would fail at compile time instead of just fail in strange ways at run time if any of the requests ever change, however unlikely.
Some of the unit tests for the json path are using the avatica types for json to test this area already, so was just thinking since its used as a runtime depend now we could use the expected types for non tests too. But yeah, its definitely not necessary and doesn't provide a lot of gain, nor do I think we should do it in this PR or anything.