Add example for h2 keep alive #2203
Conversation
...cetalk-concurrent-api/src/main/java/io/servicetalk/concurrent/api/FromIterablePublisher.java
Outdated
Show resolved
Hide resolved
...les/grpc/keepalive/src/main/java/io/servicetalk/examples/grpc/keepalive/KeepAliveClient.java
Show resolved
Hide resolved
...cetalk-concurrent-api/src/main/java/io/servicetalk/concurrent/api/FromIterablePublisher.java
Outdated
Show resolved
Hide resolved
...-concurrent-api/src/main/java/io/servicetalk/concurrent/api/PublisherAsBlockingIterable.java
Outdated
Show resolved
Hide resolved
Scottmitch
force-pushed
the
keepalive_example
branch
from
b9e83a0
to
5dac91c
Compare
|
This PR depends upon #2204 |
| try (BlockingStreamingGreeterClient client = GrpcClients.forAddress("localhost", 8080) | ||
| .initializeHttp(httpBuilder -> httpBuilder.protocols( | ||
| // 4 second timeout is typically much shorter than necessary, but demonstrates PING frame traffic. | ||
| // Using the default value is suitable in most scenarios, but if you want to customize the value |
There was a problem hiding this comment.
The improved comments will really help.
h2().keepAlivePolicy(new KeepAlivePolicyBuilder().build())
is needed to configure the default. Perhaps a constant defaultKeepAlive() or useDefaults() for configuring the default.
There was a problem hiding this comment.
Since we expose the builder type I don't think the static factory helper methods provide much value and add duplication.
There was a problem hiding this comment.
True, but I had to spend a couple minutes figuring out that there was no default object and how to make one (the Builder docs don't mention that default values will be use if not supplied). Oddly, we provide a constant for disabling keep alive which is the default behaviour (not clear why you would ever needed it except to be explicit). Some mechanism or doc suggestion assistance of how to get a default keep alive policy would help IMO.
There was a problem hiding this comment.
Lets consider this for a followup PR to make any changes more explicit. I would prefer to remove the disabled() API and just use null if we need a default (one less API, no special "by reference" comparison required).
There was a problem hiding this comment.
All are fine approaches, whatever ensures users don't have to read the source to figure out how to get the default.
...les/grpc/keepalive/src/main/java/io/servicetalk/examples/grpc/keepalive/KeepAliveClient.java
Outdated
Show resolved
Hide resolved
...les/grpc/keepalive/src/main/java/io/servicetalk/examples/grpc/keepalive/KeepAliveServer.java
Outdated
Show resolved
Hide resolved
Motivation: Add an example demonstrating how the HTTP/2 transport keep alive feature can be used for gRPC clients and servers.
Scottmitch
force-pushed
the
keepalive_example
branch
from
522e744
to
f452fb1
Compare
Motivation:
Add an example demonstrating how the HTTP/2 transport keep alive feature can be used for gRPC clients and servers.