GRPC broker request handler #7838
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
walterddr
changed the title
Codecov Report
@@ Coverage Diff @@
## master #7838 +/- ##
============================================
+ Coverage 65.07% 71.26% +6.18%
Complexity 4082 4082
============================================
Files 1538 1587 +49
Lines 79993 82032 +2039
Branches 12035 12259 +224
============================================
+ Hits 52057 58460 +6403
+ Misses 24219 19603 -4616
- Partials 3717 3969 +252
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
walterddr
commented
Dec 1, 2021
pinot-broker/src/main/java/org/apache/pinot/broker/requesthandler/GrpcBrokerRequestHandler.java
Outdated
Show resolved
Hide resolved
Jackie-Jiang
reviewed
Dec 2, 2021
pinot-broker/src/main/java/org/apache/pinot/broker/broker/helix/BaseBrokerStarter.java
Outdated
Show resolved
Hide resolved
pinot-spi/src/main/java/org/apache/pinot/spi/utils/CommonConstants.java
Outdated
Show resolved
Hide resolved
pinot-broker/src/main/java/org/apache/pinot/broker/requesthandler/GrpcBrokerRequestHandler.java
Outdated
Show resolved
Hide resolved
pinot-broker/src/main/java/org/apache/pinot/broker/requesthandler/GrpcBrokerRequestHandler.java
Outdated
Show resolved
Hide resolved
pinot-broker/src/main/java/org/apache/pinot/broker/requesthandler/GrpcBrokerRequestHandler.java
Outdated
Show resolved
Hide resolved
pinot-broker/src/main/java/org/apache/pinot/broker/requesthandler/GrpcBrokerRequestHandler.java
Outdated
Show resolved
Hide resolved
pinot-broker/src/main/java/org/apache/pinot/broker/requesthandler/GrpcBrokerRequestHandler.java
Outdated
Show resolved
Hide resolved
pinot-core/src/main/java/org/apache/pinot/core/query/reduce/SelectionOnlyStreamingReducer.java
Outdated
Show resolved
Hide resolved
pinot-core/src/main/java/org/apache/pinot/core/query/reduce/StreamingReduceService.java
Outdated
Show resolved
Hide resolved
pinot-core/src/main/java/org/apache/pinot/core/query/reduce/StreamingReduceService.java
Outdated
Show resolved
Hide resolved
xiangfu0
reviewed
Dec 2, 2021
| @@ -235,14 +236,21 @@ public void start() | |||
| // Configure TLS for netty connection to server | |||
| TlsConfig tlsDefaults = TlsUtils.extractTlsConfig(_brokerConf, Broker.BROKER_TLS_PREFIX); | |||
|
|
|||
| if (_brokerConf.getProperty(Broker.BROKER_NETTYTLS_ENABLED, false)) { | |||
| if (_brokerConf.getProperty(Broker.BROKER_REQUEST_HANDLER_TYPE, Broker.DEFAULT_BROKER_REQUEST_HANDLER_TYPE) | |||
There was a problem hiding this comment.
Shall we consider have both handlers created and have a query runtime switch to pick the non-default request handler?
There was a problem hiding this comment.
yes I will enable both once I have all other reducer service up and running (probably in the next 1-2 PRs)
walterddr
force-pushed
the
add_broker_grpc
branch
2 times, most recently
from
503f6f5 to
5e85a04
Compare
Jackie-Jiang
approved these changes
Dec 7, 2021
pinot-broker/src/main/java/org/apache/pinot/broker/requesthandler/GrpcBrokerRequestHandler.java
Outdated
Show resolved
Hide resolved
pinot-core/src/main/java/org/apache/pinot/core/query/reduce/BaseReduceService.java
Outdated
Show resolved
Hide resolved
pinot-core/src/main/java/org/apache/pinot/core/query/reduce/BaseReduceService.java
Outdated
Show resolved
Hide resolved
pinot-core/src/main/java/org/apache/pinot/core/query/reduce/BaseReduceService.java
Outdated
Show resolved
Hide resolved
pinot-core/src/main/java/org/apache/pinot/core/query/reduce/BaseReduceService.java
Outdated
Show resolved
Hide resolved
pinot-core/src/main/java/org/apache/pinot/core/query/reduce/BaseReduceService.java
Outdated
Show resolved
Hide resolved
pinot-core/src/main/java/org/apache/pinot/core/query/reduce/SelectionOnlyStreamingReducer.java
Outdated
Show resolved
Hide resolved
|
I'd just like to go through this PR once. Can we please hold off mergin for a day or so ? |
walterddr
force-pushed
the
add_broker_grpc
branch
from
ae12837 to
de122a5
Compare
|
addressed diff comments, any additionl feedback @siddharthteotia @Jackie-Jiang |
added 7 commits
December 9, 2021 21:00
walterddr
force-pushed
the
add_broker_grpc
branch
from
ed1c3c1 to
f4c122a
Compare
kriti-sc
pushed a commit
to kriti-sc/incubator-pinot
that referenced
this pull request
Dec 12, 2021
Adding a framework of GRPCBrokerResponseHandler parallel to the SingleConnectionBrokerRequestHandler This handler handles data streaming back from server and process reduce in a streaming fashion.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Currently there's no way to use the GRPC server request from broker side.
Adding a framework of
GRPCBrokerResponseHandlerparallel to theSingleConnectionBrokerRequestHandlerThis handler handles data streaming back from server and process reduce in a streaming fashion.