Skip to content
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

gRPC query extension #15982

Merged
merged 94 commits into from
Sep 19, 2024
Merged

Conversation

findingrish
Copy link
Contributor

@findingrish findingrish commented Feb 27, 2024

Revives #14024 and additionally supports,

  • Native queries
  • gRPC health check endpoint

This PR doesn't have the shaded module for packaging gRPC and Guava libraries since grpc-query module uses the same Guava version as that of Druid.

The response is gRPC-specific. It provides the result schema along with the results as a binary "blob". Results can be in CSV, JSON array lines or as an array of Protobuf objects. If using Protobuf, the corresponding class must be installed along with the gRPC query extension so it is available to the Broker at runtime.

Example usage

Sample request,

QueryRequest.newBuilder()
            .setQuery("SELECT * FROM foo")
            .setResultFormat(QueryResultFormat.CSV)
            .setQueryType(QueryOuterClass.QueryType.SQL)
            .build();

When using Protobuf response format, bundle up your Protobuf classes
into a jar file, and place that jar file in the
$DRUID_HOME/extensions/grpc-query directory.
Specify the response Protobuf message name in the request.

QueryRequest.newBuilder()
            .setQuery("SELECT dim1, dim2, dim3, cnt, m1, m2, unique_dim1, __time AS "date" FROM foo")
            .setQueryType(QueryOuterClass.QueryType.SQL)
            .setProtobufMessageName(QueryResult.class.getName())
            .setResultFormat(QueryResultFormat.PROTOBUF_INLINE)
            .build();    
            
Response message 
message QueryResult {
  string dim1 = 1;
  string dim2 = 2;
  string dim3 = 3;
  int64 cnt = 4;
  float m1 = 5;
  double m2 = 6;
  bytes unique_dim1 = 7;
  google.protobuf.Timestamp date = 8;
}          

Release note

This is a "contrib" extension. We don't seem to document such extensions in Druid itself. The README.md can serve as documentation instead.



This PR has:

  • been self-reviewed.
  • added documentation for new or modified features or behaviors.
  • a release note entry in the PR description.
  • added Javadocs for most classes and all non-trivial methods. Linked related entities via Javadoc links.
  • added or updated version, license, or notice information in licenses.yaml
  • added comments explaining the "why" and the intent of the code wherever would not be obvious for an unfamiliar reader.
  • added unit tests or modified existing tests to cover new code paths, ensuring the threshold for code coverage is met.
  • added integration tests.
  • been tested in a test Druid cluster.

paul-rogers and others added 30 commits January 14, 2023 14:36
Report stats & close statement
Simple server + query test works
The gRPC shaded jar module causes issues when loaded
into an IDE. This PR hides the module from Maven (and
hence IDEs) by building it implicitly in the grpc-query
module. It is a hack, but it works.
add empty response and GrpcResponseHandler tests
Allow ITs outside of the 'cases' directory
Refactoring
Build the grpc-query extension archive
Build a jar for the test Protobuf message
Fixes to the extension path mechanism
* Refactor template into cluster directory
* Add verify, setup scripts
* Forbid snapshots from upstream repos
* Basic grpc-query IT test
Retains support for anonymous (test) usage
<parent>
<groupId>org.apache.druid</groupId>
<artifactId>druid</artifactId>
<version>30.0.0-SNAPSHOT</version>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
<version>30.0.0-SNAPSHOT</version>
<version>31.0.0-SNAPSHOT</version>

The master branch has been updated to 31.0.0-SNAPSHOT

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's fix it and then merge it

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated.

Copy link

github-actions bot commented Jul 6, 2024

This pull request has been marked as stale due to 60 days of inactivity.
It will be closed in 4 weeks if no further activity occurs. If you think
that's incorrect or this pull request should instead be reviewed, please simply
write any comment. Even if closed, you can still revive the PR at any time or
discuss it on the dev@druid.apache.org list.
Thank you for your contributions.

@github-actions github-actions bot added the stale label Jul 6, 2024
}

message QueryRequest {
QueryType queryType = 1;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rishabh Could you make that QueryType is the last field in this proto to preserve the original order to avoid breaking changes and also give QueryType a default value of SQL.

@github-actions github-actions bot removed the stale label Jul 17, 2024
@andrisnoko
Copy link

@FrankChen021 could you please take a look at this PR, it has been open for a while

@abhishekagarwal87
Copy link
Contributor

@findingrish - can you rebase it on master so we can merge it?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what is this file for?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cleaned up.

@abhishekagarwal87
Copy link
Contributor

@findingrish - the test failures look genuine

@abhishekagarwal87 abhishekagarwal87 merged commit 953fe11 into apache:master Sep 19, 2024
91 checks passed
@adarshsanjeev adarshsanjeev added this to the 32.0.0 milestone Jan 16, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants