Skip to content

Commit fb44092

Browse files
committed
netty: Allow disabling server keep-alive enforcement via permitKeepAliveTime
This change effectively makes the permitKeepAliveTime change consistent, similar to keepAliveTime. We can now disable this mechanism. Currently, we can't do this, because even if we pass Long.MAX_VALUE, KeepAliveEnforcer will substitute its own value.
1 parent ef35313 commit fb44092

File tree

4 files changed

+36
-2
lines changed

4 files changed

+36
-2
lines changed

core/src/main/java/io/grpc/internal/GrpcUtil.java

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -241,6 +241,17 @@ public byte[] parseAsciiString(byte[] serialized) {
241241
*/
242242
public static final long DEFAULT_SERVER_KEEPALIVE_TIMEOUT_NANOS = TimeUnit.SECONDS.toNanos(20L);
243243

244+
/**
245+
* The default minimum time between client keepalive pings permitted by server.
246+
*/
247+
public static final long DEFAULT_SERVER_PERMIT_KEEPALIVE_TIME_NANOS =
248+
TimeUnit.MINUTES.toNanos(5);
249+
250+
/**
251+
* The magic permit keepalive time value that disables server keepalive enforcement.
252+
*/
253+
public static final long SERVER_PERMIT_KEEPALIVE_TIME_NANOS_DISABLED = Long.MAX_VALUE;
254+
244255
/**
245256
* The magic keepalive time value that disables keepalive.
246257
*/

core/src/main/java/io/grpc/internal/KeepAliveEnforcer.java

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,8 @@ public KeepAliveEnforcer(boolean permitWithoutCalls, long minTime, TimeUnit unit
4646
Preconditions.checkArgument(minTime >= 0, "minTime must be non-negative: %s", minTime);
4747

4848
this.permitWithoutCalls = permitWithoutCalls;
49-
this.minTimeNanos = Math.min(unit.toNanos(minTime), IMPLICIT_PERMIT_TIME_NANOS);
49+
this.minTimeNanos = minTime == Long.MAX_VALUE ? Long.MAX_VALUE
50+
: Math.min(unit.toNanos(minTime),IMPLICIT_PERMIT_TIME_NANOS);
5051
this.ticker = ticker;
5152
this.epoch = ticker.nanoTime();
5253
lastValidPingTime = epoch;
@@ -55,6 +56,9 @@ public KeepAliveEnforcer(boolean permitWithoutCalls, long minTime, TimeUnit unit
5556
/** Returns {@code false} when client is misbehaving and should be disconnected. */
5657
@CheckReturnValue
5758
public boolean pingAcceptable() {
59+
if (minTimeNanos == Long.MAX_VALUE) {
60+
return true;
61+
}
5862
long now = ticker.nanoTime();
5963
boolean valid;
6064
if (!hasOutstandingCalls && !permitWithoutCalls) {

core/src/test/java/io/grpc/internal/KeepAliveEnforcerTest.java

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -165,6 +165,20 @@ public void permitsWhenTimeOverflows() {
165165
assertThat(enforcer.pingAcceptable()).isTrue();
166166
}
167167

168+
@Test
169+
public void permitAllWhenDisabled() {
170+
KeepAliveEnforcer enforcer = new KeepAliveEnforcer(
171+
false, Long.MAX_VALUE, TimeUnit.NANOSECONDS, ticker);
172+
enforcer.onTransportIdle();
173+
for (int i = 0; i < LARGE_NUMBER; i++) {
174+
assertThat(enforcer.pingAcceptable()).isTrue();
175+
}
176+
enforcer.onTransportActive();
177+
for (int i = 0; i < LARGE_NUMBER; i++) {
178+
assertThat(enforcer.pingAcceptable()).isTrue();
179+
}
180+
}
181+
168182
@Test
169183
public void resetCounters_resetsStrikes() {
170184
KeepAliveEnforcer enforcer = new KeepAliveEnforcer(false, 1, TimeUnit.NANOSECONDS, ticker);

netty/src/main/java/io/grpc/netty/NettyServerBuilder.java

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,9 @@
2222
import static io.grpc.internal.GrpcUtil.DEFAULT_MAX_MESSAGE_SIZE;
2323
import static io.grpc.internal.GrpcUtil.DEFAULT_SERVER_KEEPALIVE_TIMEOUT_NANOS;
2424
import static io.grpc.internal.GrpcUtil.DEFAULT_SERVER_KEEPALIVE_TIME_NANOS;
25+
import static io.grpc.internal.GrpcUtil.DEFAULT_SERVER_PERMIT_KEEPALIVE_TIME_NANOS;
2526
import static io.grpc.internal.GrpcUtil.SERVER_KEEPALIVE_TIME_NANOS_DISABLED;
27+
import static io.grpc.internal.GrpcUtil.SERVER_PERMIT_KEEPALIVE_TIME_NANOS_DISABLED;
2628

2729
import com.google.common.annotations.VisibleForTesting;
2830
import com.google.errorprone.annotations.CanIgnoreReturnValue;
@@ -113,7 +115,7 @@ public final class NettyServerBuilder extends ForwardingServerBuilder<NettyServe
113115
private long maxConnectionAgeInNanos = MAX_CONNECTION_AGE_NANOS_DISABLED;
114116
private long maxConnectionAgeGraceInNanos = MAX_CONNECTION_AGE_GRACE_NANOS_INFINITE;
115117
private boolean permitKeepAliveWithoutCalls;
116-
private long permitKeepAliveTimeInNanos = TimeUnit.MINUTES.toNanos(5);
118+
private long permitKeepAliveTimeInNanos = DEFAULT_SERVER_PERMIT_KEEPALIVE_TIME_NANOS;
117119
private int maxRstCount;
118120
private long maxRstPeriodNanos;
119121
private Attributes eagAttributes = Attributes.EMPTY;
@@ -656,6 +658,9 @@ public NettyServerBuilder permitKeepAliveTime(long keepAliveTime, TimeUnit timeU
656658
checkArgument(keepAliveTime >= 0, "permit keepalive time must be non-negative: %s",
657659
keepAliveTime);
658660
permitKeepAliveTimeInNanos = timeUnit.toNanos(keepAliveTime);
661+
if (permitKeepAliveTimeInNanos >= AS_LARGE_AS_INFINITE) {
662+
permitKeepAliveTimeInNanos = SERVER_PERMIT_KEEPALIVE_TIME_NANOS_DISABLED;
663+
}
659664
return this;
660665
}
661666

0 commit comments

Comments
 (0)