Wrong minimal value for keepAliveTimeout

[dpasek-senacor]

The ConnectionSettingsBuilder performs validation on the keepAliveInterval and keepAliveTimeout values.

If the value is below the configured defaults the given values or ignored and the internal defaults defined in Consts:

    public static long DEFAULT_KEEP_ALIVE_TIMEOUT_IN_MS = 10000; // 10secs
    public static long DEFAULT_KEEP_ALIVE_INTERVAL_IN_MS = 10000; // 10secs

The default value for the keepAliveInterval of 10s makes sense and is in line with the internal defaults used by the gRPC library. The value for keepAliveTimeout on the other hand is very long (waiting for a minimum 10s for the PING response) and not in line with the values proposed by the gRPC library. In the gRPC library the defaults for keepAliveInterval und keepAliveTimeout are managed inside the KeepAliveManager class: https://github.com/grpc/grpc-java/blob/master/core/src/main/java/io/grpc/internal/KeepAliveManager.java

public class KeepAliveManager {
  private static final long MIN_KEEPALIVE_TIME_NANOS = TimeUnit.SECONDS.toNanos(10);
  private static final long MIN_KEEPALIVE_TIMEOUT_NANOS = TimeUnit.MILLISECONDS.toNanos(10L);
....
}

As you can see the minimum value for the keepAliveTimeout is defined as 10 _milli_seconds which seems muhc more pratical for detecting connection outages as the server response to PING should be matters of milliseconds, not seconds.

To better detect connection issues during idle times the minimum keepAliveTimeout value should be adapted to the default of the gRPC library.

[hayley-jean]

Thanks for bringing this to our attention. The behaviour in the ConnectionSettingsBuilder is incorrect, it should log a warning about the value being changed, but should still allow you to set it to a lower value.

This is handled correctly when using the Connection String.