Feature/http2 #32
Feature/http2 #32
Conversation
After SSL handshake and protocol negotiation, the client configure the pipeline based on the negotiation result.
The default setting prefers H2, but fallback to Http/1.1
This make the semantic clearer that we write the buffered messages to channel rather than the context.
| * | ||
| * @return true if "h2" is in the protocols list | ||
| */ | ||
| public boolean useHttp2() { |
There was a problem hiding this comment.
I was thinking that the list would be ordered by preference. While http1.1, http2 doesn't necessarily make sense that would be the logic. Also it doesn't seem that this method should be available to the public. Minimally it should be hidden. Or, just remove it (preferable) and do the logic in the lower level code
There was a problem hiding this comment.
The list is order irrelevant.
In the ALPN case, if we provide both H2 and H1.1 (no matter what order), the server always pick H2 over H1.1 given the server supports H2.
In the clear text case, when we provide both H2 and H1.1 to the config, it's not like we try H2 first for 30 seconds timeout then fall back to H1.1, instead we directly use H2C protocol, so the order of the protocols is not relevant either.
For the visibility, sure I will remove this method, and do the logic in the lower level code.
| @@ -631,6 +656,14 @@ public NoSQLHandleConfig setRequestTimeout(int timeout) { | |||
| return this; | |||
| } | |||
|
|
|||
| public NoSQLHandleConfig setHttpProtocols(String ... protocols) { | |||
| @@ -553,6 +561,23 @@ public int getDefaultRequestTimeout() { | |||
| return timeout == 0 ? DEFAULT_TIMEOUT : timeout; | |||
| } | |||
|
|
|||
| /** | |||
| * | |||
| * @return Http protocol settings | |||
There was a problem hiding this comment.
needs an actual explanation of what's being returned
| * | ||
| * @return true if "h2" is in the protocols list | ||
| */ | ||
| public boolean useHttp2() { |
There was a problem hiding this comment.
again.. shouldn't the list be ordered by preference?
| * | ||
| * Default: prefer H2 but fallback to Http1.1 | ||
| */ | ||
| private List<String> httpProtocols = new ArrayList<>(Arrays.asList(ApplicationProtocolNames.HTTP_2, ApplicationProtocolNames.HTTP_1_1)); |
There was a problem hiding this comment.
While we may need to use ApplicationProtocolNames when talking to Netty there should be no netty class or constant that is part of our own public API. We need to provide our own constants to represent http/2 and 1.1 and map them internally (probably not in this class) to netty when needed
oracle-contributor-agreement
bot
added
the
OCA Required
|
Thank you for your pull request and welcome to our community! To contribute, please sign the Oracle Contributor Agreement (OCA).
To sign the OCA, please create an Oracle account and sign the OCA in Oracle's Contributor Agreement Application. When signing the OCA, please provide your GitHub username. After signing the OCA and getting an OCA approval from Oracle, this PR will be automatically updated. |
liujoey
force-pushed
the
feature/http2
branch
from
4875fc4
to
7495015
Compare
oracle-contributor-agreement
bot
added
OCA Verified
The client side HTTP2 support.
It works in the following modes: