grpc over http/2 poc #26
Open
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
kmrdhruv
reviewed
Jun 26, 2023
| @@ -58,6 +87,8 @@ public void start(Promise<Void> startPromise) { | |||
| HttpServerOptions options = new HttpServerOptions(); | |||
| // TODO: why? | |||
| options.setDecompressionSupported(false); | |||
| options.setAlpnVersions(Arrays.asList(HttpVersion.HTTP_2, HttpVersion.HTTP_1_1)); | |||
There was a problem hiding this comment.
DEFAULT_ALPN_VERSIONS constant ?
kmrdhruv
reviewed
Jun 26, 2023
| String errorMsg = String.format("Fail to load class %s.", className); | ||
| log.error(errorMsg, e); | ||
| throw new InvalidConfigException(e); | ||
| throw new InvalidConfigException(String.format("Fail to load class %s.", className), e); |
There was a problem hiding this comment.
Any thoughts for removing logging ?
There was a problem hiding this comment.
logging error and then throwing the same leads to redundant logs.
For eg:
Before
2023-06-26 12:19:05 ERROR CoreServices:85 - Fail to load class com.flipkart.varadhi.pulsar.PulsarStackProvider.
java.lang.ClassNotFoundException: com.flipkart.varadhi.pulsar.PulsarStackProvider
at jdk.internal.loader.BuiltinClassLoader.loadClass(BuiltinClassLoader.java:641) ~[?:?]
at jdk.internal.loader.ClassLoaders$AppClassLoader.loadClass(ClassLoaders.java:188) ~[?:?]
at java.lang.ClassLoader.loadClass(ClassLoader.java:520) ~[?:?]
at java.lang.Class.forName0(Native Method) ~[?:?]
at java.lang.Class.forName(Class.java:375) ~[?:?]
at com.flipkart.varadhi.CoreServices.loadClass(CoreServices.java:79) [main/:?]
at com.flipkart.varadhi.CoreServices.setupMessagingStackProvider(CoreServices.java:71) [main/:?]
at com.flipkart.varadhi.CoreServices.<init>(CoreServices.java:41) [main/:?]
at com.flipkart.varadhi.Server.main(Server.java:22) [main/:?]
2023-06-26 12:19:05 ERROR Server:27 - Failed to initialise the server.
com.flipkart.varadhi.exceptions.InvalidConfigException: com.flipkart.varadhi.pulsar.PulsarStackProvider
at com.flipkart.varadhi.CoreServices.loadClass(CoreServices.java:86) ~[main/:?]
at com.flipkart.varadhi.CoreServices.setupMessagingStackProvider(CoreServices.java:71) ~[main/:?]
at com.flipkart.varadhi.CoreServices.<init>(CoreServices.java:41) ~[main/:?]
at com.flipkart.varadhi.Server.main(Server.java:22) [main/:?]
Caused by: java.lang.ClassNotFoundException: com.flipkart.varadhi.pulsar.PulsarStackProvider
at jdk.internal.loader.BuiltinClassLoader.loadClass(BuiltinClassLoader.java:641) ~[?:?]
at jdk.internal.loader.ClassLoaders$AppClassLoader.loadClass(ClassLoaders.java:188) ~[?:?]
at java.lang.ClassLoader.loadClass(ClassLoader.java:520) ~[?:?]
at java.lang.Class.forName0(Native Method) ~[?:?]
at java.lang.Class.forName(Class.java:375) ~[?:?]
at com.flipkart.varadhi.CoreServices.loadClass(CoreServices.java:79) ~[main/:?]
... 3 more
Failed to initialise the server:com.flipkart.varadhi.exceptions.InvalidConfigException: com.flipkart.varadhi.pulsar.PulsarStackProvider
After
2023-06-26 12:19:53 ERROR Server:27 - Failed to initialise the server.
com.flipkart.varadhi.exceptions.InvalidConfigException: Fail to load class com.flipkart.varadhi.pulsar.PulsarStackProvider.
at com.flipkart.varadhi.CoreServices.loadClass(CoreServices.java:84) ~[main/:?]
at com.flipkart.varadhi.CoreServices.setupMessagingStackProvider(CoreServices.java:71) ~[main/:?]
at com.flipkart.varadhi.CoreServices.<init>(CoreServices.java:41) ~[main/:?]
at com.flipkart.varadhi.Server.main(Server.java:22) [main/:?]
Caused by: java.lang.ClassNotFoundException: com.flipkart.varadhi.pulsar.PulsarStackProvider
at jdk.internal.loader.BuiltinClassLoader.loadClass(BuiltinClassLoader.java:641) ~[?:?]
at jdk.internal.loader.ClassLoaders$AppClassLoader.loadClass(ClassLoaders.java:188) ~[?:?]
at java.lang.ClassLoader.loadClass(ClassLoader.java:520) ~[?:?]
at java.lang.Class.forName0(Native Method) ~[?:?]
at java.lang.Class.forName(Class.java:375) ~[?:?]
at com.flipkart.varadhi.CoreServices.loadClass(CoreServices.java:79) ~[main/:?]
... 3 more
There was a problem hiding this comment.
I try to follow the rule for error logs which says: Don't log if you are throwing. can log if you are handling.
There was a problem hiding this comment.
generally I tend to follow log at source of the event to capture all data along that leads to exception.
But I guess, we can avoid exception logging from that and keep only data part and leave exception logging to either default exception handler at base or log where it is being handled ?
There was a problem hiding this comment.
We can still capture what we want to capture in the "exception" that we are throwing.
kmrdhruv
reviewed
Jun 26, 2023
| .path("/topics/produce") | ||
| .method(HttpMethod.POST) | ||
| .handler(rc -> Extensions.RoutingContextExtension.endRequestWithResponse(rc, 200, "0001")); | ||
|
|
There was a problem hiding this comment.
Planning to keep it or move it appropriately to ?
There was a problem hiding this comment.
ya. its a TODO, will fix it in this PR.
kmrdhruv
requested changes
Jun 26, 2023
I pulled the refactoring changes into a different PR and merged that in master. This PR now only contains grpc specific additions. |
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.
Just a poc to try out the grpc. Vertx recently rolled out a new support for grpc.
TODO:
./gradlew run not working.
cleanup grpc setup.
setup code to perform additional "handling" for grpc requests as well. Http api and grpc api should behave same.
setup & test grpc / http requests over tls connections.
How to try grpc flow?
Use Postman / insomnia. I tried with insomnia and it seemed to work.