Merge pull request #10412 from y-zeng/max_age_jitter

Add max_connection_age jitter

src/core/ext/filters/max_age/max_age_filter.c

@@ -46,6 +46,7 @@
 #define DEFAULT_MAX_CONNECTION_AGE_MS INT_MAX
 #define DEFAULT_MAX_CONNECTION_AGE_GRACE_MS INT_MAX
 #define DEFAULT_MAX_CONNECTION_IDLE_MS INT_MAX
+#define MAX_CONNECTION_AGE_JITTER 0.1
 
 typedef struct channel_data {
   /* We take a reference to the channel stack for the timer callback */
@@ -254,6 +255,21 @@ static void channel_connectivity_changed(grpc_exec_ctx* exec_ctx, void* arg,
   }
 }
 
+/* A random jitter of +/-10% will be added to MAX_CONNECTION_AGE to spread out
+   connection storms. Note that the MAX_CONNECTION_AGE option without jitter
+   would not create connection storms by itself, but if there happened to be a
+   connection storm it could cause it to repeat at a fixed period. */
+static int add_random_max_connection_age_jitter(int value) {
+  /* generate a random number between 1 - MAX_CONNECTION_AGE_JITTER and
+     1 + MAX_CONNECTION_AGE_JITTER */
+  double multiplier = rand() * MAX_CONNECTION_AGE_JITTER * 2.0 / RAND_MAX +
+                      1.0 - MAX_CONNECTION_AGE_JITTER;
+  double result = multiplier * value;
+  /* INT_MAX - 0.5 converts the value to float, so that result will not be
+     cast to int implicitly before the comparison. */
+  return result > INT_MAX - 0.5 ? INT_MAX : (int)result;
+}
+
 /* Constructor for call_data. */
 static grpc_error* init_call_elem(grpc_exec_ctx* exec_ctx,
                                   grpc_call_element* elem,
@@ -283,7 +299,9 @@ static grpc_error* init_channel_elem(grpc_exec_ctx* exec_ctx,
   chand->max_connection_age =
       DEFAULT_MAX_CONNECTION_AGE_MS == INT_MAX
           ? gpr_inf_future(GPR_TIMESPAN)
-          : gpr_time_from_millis(DEFAULT_MAX_CONNECTION_AGE_MS, GPR_TIMESPAN);
+          : gpr_time_from_millis(add_random_max_connection_age_jitter(
+                                     DEFAULT_MAX_CONNECTION_AGE_MS),
+                                 GPR_TIMESPAN);
   chand->max_connection_age_grace =
       DEFAULT_MAX_CONNECTION_AGE_GRACE_MS == INT_MAX
           ? gpr_inf_future(GPR_TIMESPAN)
@@ -300,8 +318,10 @@ static grpc_error* init_channel_elem(grpc_exec_ctx* exec_ctx,
           &args->channel_args->args[i],
           (grpc_integer_options){DEFAULT_MAX_CONNECTION_AGE_MS, 1, INT_MAX});
       chand->max_connection_age =
-          value == INT_MAX ? gpr_inf_future(GPR_TIMESPAN)
-                           : gpr_time_from_millis(value, GPR_TIMESPAN);
+          value == INT_MAX
+              ? gpr_inf_future(GPR_TIMESPAN)
+              : gpr_time_from_millis(
+                    add_random_max_connection_age_jitter(value), GPR_TIMESPAN);
     } else if (0 == strcmp(args->channel_args->args[i].key,
                            GRPC_ARG_MAX_CONNECTION_AGE_GRACE_MS)) {
       const int value = grpc_channel_arg_get_integer(

test/core/end2end/tests/max_connection_age.c

@@ -47,6 +47,7 @@
 #define MAX_CONNECTION_AGE_GRACE_MS 1000
 #define MAX_CONNECTION_IDLE_MS 9999
 
+#define MAX_CONNECTION_AGE_JITTER_MULTIPLIER 1.1
 #define CALL_DEADLINE_S 10
 /* The amount of time we wait for the connection to time out, but after it the
    connection should not use up its grace period. It should be a number between
@@ -169,8 +170,8 @@ static void test_max_age_forcibly_close(grpc_end2end_test_config config) {
   cq_verify(cqv);
 
   gpr_timespec expect_shutdown_time = grpc_timeout_milliseconds_to_deadline(
-      MAX_CONNECTION_AGE_MS + MAX_CONNECTION_AGE_GRACE_MS +
-      IMMEDIATE_SHUTDOWN_GRACE_TIME_MS);
+      (int)(MAX_CONNECTION_AGE_MS * MAX_CONNECTION_AGE_JITTER_MULTIPLIER) +
+      MAX_CONNECTION_AGE_GRACE_MS + IMMEDIATE_SHUTDOWN_GRACE_TIME_MS);
 
   /* Wait for the channel to reach its max age */
   cq_verify_empty_timeout(cqv, CQ_MAX_CONNECTION_AGE_WAIT_TIME_S);