Skip to content

Allow configuring TLS between brokers and servers for the multi-stage engine #14387

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.

Sign up for GitHub

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

Jump to bottom
Merged
gortiz merged 1 commit into from
Nov 7, 2024
Merged

Allow configuring TLS between brokers and servers for the multi-stage engine #14387

gortiz merged 1 commit into from
Nov 7, 2024

Conversation

@yashmayya
Copy link
Contributor

@yashmayya yashmayya commented Nov 5, 2024 

Caught exception executing request 2087751584000000000: SELECT COUNT(*) FROM (SELECT AirlineID FROM mytable WHERE ArrDelay >= 0 LIMIT 1000000), Error dispatching query: 2087751584000000000 to server: localhost@{64489,64490}
org.apache.pinot.query.service.dispatch.QueryDispatcher.execute(QueryDispatcher.java:268)
org.apache.pinot.query.service.dispatch.QueryDispatcher.submit(QueryDispatcher.java:213)
org.apache.pinot.query.service.dispatch.QueryDispatcher.submitAndReduce(QueryDispatcher.java:125)
org.apache.pinot.broker.requesthandler.MultiStageBrokerRequestHandler.handleRequest(MultiStageBrokerRequestHandler.java:231)
UNAVAILABLE: Network closed for unknown reason
io.grpc.Status.asRuntimeException(Status.java:532)
io.grpc.stub.ClientCalls$StreamObserverToCallListenerAdapter.onClose(ClientCalls.java:481)
io.grpc.internal.DelayedClientCall$DelayedListener$3.run(DelayedClientCall.java:489)
io.grpc.internal.DelayedClientCall$DelayedListener.delayOrExecute(DelayedClientCall.java:453)
  • In a future patch, we can also add TLS support to the gRPC mailbox server and the gRPC sending mailbox that are used for data shuffle in the multi-stage engine.

@yashmayya yashmayya added multi-stage Related to the multi-stage query engine security feature labels Nov 5, 2024
@codecov-commenter
Copy link

codecov-commenter commented Nov 5, 2024 •
edited
Loading

Codecov Report

Attention: Patch coverage is 33.33333% with 34 lines in your changes missing coverage. Please review.

Project coverage is 63.77%. Comparing base (59551e4) to head (29fcad5).
Report is 2616 commits behind head on master.

Files with missing lines Patch % Lines
...rg/apache/pinot/server/starter/ServerInstance.java 0.00% 12 Missing ⚠️
...apache/pinot/query/service/server/QueryServer.java 50.00% 6 Missing and 1 partial ⚠️
.../apache/pinot/server/worker/WorkerQueryServer.java 0.00% 7 Missing ⚠️
...e/pinot/query/service/dispatch/DispatchClient.java 50.00% 4 Missing and 1 partial ⚠️
...requesthandler/MultiStageBrokerRequestHandler.java 33.33% 1 Missing and 1 partial ⚠️
.../java/org/apache/pinot/server/conf/ServerConf.java 0.00% 1 Missing ⚠️
Additional details and impacted files 
@@             Coverage Diff              @@
##             master   #14387      +/-   ##
============================================
+ Coverage     61.75%   63.77%   +2.02%     
- Complexity      207     1555    +1348     
============================================
  Files          2436     2660     +224     
  Lines        133233   145962   +12729     
  Branches      20636    22348    +1712     
============================================
+ Hits          82274    93086   +10812     
- Misses        44911    45997    +1086     
- Partials       6048     6879     +831
Flag Coverage Δ
custom-integration1 100.00% <ø> (+99.99%) ⬆️
integration 100.00% <ø> (+99.99%) ⬆️
integration1 100.00% <ø> (+99.99%) ⬆️
integration2 0.00% <ø> (ø)
java-11 63.73% <33.33%> (+2.03%) ⬆️
java-21 63.65% <33.33%> (+2.03%) ⬆️
skip-bytebuffers-false 63.75% <33.33%> (+2.00%) ⬆️
skip-bytebuffers-true 63.63% <33.33%> (+35.91%) ⬆️
temurin 63.77% <33.33%> (+2.02%) ⬆️
unittests 63.77% <33.33%> (+2.02%) ⬆️
unittests1 55.46% <57.14%> (+8.57%) ⬆️
unittests2 34.14% <9.80%> (+6.41%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@yashmayya yashmayya marked this pull request as ready for review November 5, 2024 08:33
@gortiz
Copy link
Contributor

gortiz commented Nov 5, 2024 •
edited
Loading

We are going to need to update GitBook to describe the new config

@gortiz gortiz merged commit 4cffd91 into apache:master Nov 7, 2024
@yashmayya yashmayya mentioned this pull request Nov 18, 2024
Merged
davecromberge pushed a commit to davecromberge/pinot that referenced this pull request Nov 22, 2024
@yashmayya @davecromberge
Allow configuring TLS between brokers and servers for the multi-stage…
618a4d6 
… engine (apache#14387)
@ilamhs ilamhs mentioned this pull request Dec 17, 2024
Closed
@dinoocch dinoocch mentioned this pull request Jul 31, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@gortiz gortiz gortiz approved these changes

@Jackie-Jiang Jackie-Jiang Awaiting requested review from Jackie-Jiang

Assignees

No one assigned

Labels

feature multi-stage Related to the multi-stage query engine security

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@yashmayya @codecov-commenter @gortiz